From eac8f625c3132eaf0ae8afa29161bd630c72e0cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Molakvo=C3=A6?= <14975046+skjnldsv@users.noreply.github.com> Date: Thu, 1 Oct 2026 15:26:22 +0200 Subject: [PATCH 1/2] feat(handlers): add onInit() to define a handler's element when it is needed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same shape as the files sidebar tabs: the viewer calls onInit() the first time it needs the element, and renders it once the promise resolved and the tag is defined. The view and what it imports stay out of the registration script that runs on every page. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: John Molakvoæ <14975046+skjnldsv@users.noreply.github.com> --- README.md | 32 ++++++++----- __tests__/component/Viewer.spec.ts | 73 ++++++++++++++++++++++++++++++ __tests__/registerHandler.spec.ts | 10 ++++ lib/handlers.ts | 16 +++++++ lib/utils/customElements.ts | 28 ++++++++++++ lib/views/Viewer.vue | 42 +++++++++++++++-- 6 files changed, 187 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index b9955c6..9fc45ca 100644 --- a/README.md +++ b/README.md @@ -87,30 +87,34 @@ element is mounted again for every file, which is why `onMounted` is enough here #### 3. Register it -This script turns the component into a custom element and tells the viewer which -files it takes: +This script tells the viewer which files it takes, and how to define the element +that shows them: ```ts // src/init-viewer.ts import { t } from '@nextcloud/l10n' import { registerHandler } from '@nextcloud/viewer' -import { defineCustomElement } from 'vue' -import NoteView from './views/NoteView.vue' const tagName = 'myapp-note-view' -if (!window.customElements.get(tagName)) { - window.customElements.define(tagName, defineCustomElement(NoteView, { shadowRoot: false })) -} - registerHandler({ id: 'myapp-notes', displayName: t('myapp', 'Notes'), tagName, enabled: (nodes) => nodes.every((node) => node.mime === 'application/x-myapp-note'), + // Only called the first time a note is opened + onInit: async () => { + const { defineCustomElement } = await import('vue') + const { default: NoteView } = await import('./views/NoteView.vue') + window.customElements.define(tagName, defineCustomElement(NoteView, { shadowRoot: false })) + }, }) ``` +`onInit()` keeps your view out of this script. The viewer calls it the first +time it needs the element, and renders the element once the promise has resolved +and the tag is defined. + Add it as an entry of your build, next to your other ones: ```js @@ -122,9 +126,9 @@ export default createAppConfig({ }) ``` -It lands in `js/myapp-init-viewer.mjs`. This script runs on every page, so keep -it small: if your view pulls in something heavy, have the element render a small -wrapper that loads the real view with `defineAsyncComponent`. +It lands in `js/myapp-init-viewer.mjs`. This script runs on every page, which is +why the view itself is only imported in `onInit()`: the script stays a few lines however +much the view imports. #### 4. Load it on every page @@ -309,6 +313,11 @@ registerHandler({ }) ``` +Defining the element up front puts the view in the script that registers the +handler, and that script runs on every page. To keep it out, leave the +definition to `onInit()`, as the [tutorial](#3-register-it) does: the viewer +calls it the first time it needs the element. + The full handler shape (see the `IHandler` interface): | Field | Type | Required | Description | @@ -323,6 +332,7 @@ The full handler shape (see the `IHandler` interface): | `theme` | `'dark' \| 'light' \| 'default'` | no | Viewer modal theme | | `canCompare` | `boolean` | no | Comparing two versions is worth offering, see `canCompare(node)` | | `supportsEndToEndEncryption` | `boolean` | no | Whether the handler supports end-to-end encrypted files | +| `onInit` | `() => Promise` | no | Defines the element for `tagName`, called the first time it is needed | Gotchas: diff --git a/__tests__/component/Viewer.spec.ts b/__tests__/component/Viewer.spec.ts index f229c63..202e7fd 100644 --- a/__tests__/component/Viewer.spec.ts +++ b/__tests__/component/Viewer.spec.ts @@ -1182,3 +1182,76 @@ describe('stepping to another file', () => { expect(modalProps().slideshowPaused).toBe(false) }) }) + +describe('a handler that defines its element in onInit()', () => { + let count = 0 + + /** + * A handler whose onInit() defines its element, under a tag no other test uses. + * + * @param before - What onInit() waits for before defining it + */ + function lazyHandler(before: () => Promise = async () => {}) { + const tagName = `oca-viewer-lazy-${++count}` + const onInit = vi.fn(async () => { + await before() + customElements.define(tagName, class extends HTMLElement {}) + }) + return makeHandler({ id: tagName, tagName, enabled: () => true, onInit }) + } + + it('renders the element only once onInit() has defined it', async () => { + let resolve!: () => void + const handler = lazyHandler(() => new Promise((r) => { + resolve = r + })) + const { vm, renderedTags } = mountViewer([handler]) + const file = makeFile() + await vm.open([file], file) + await flushPromises() + + // Rendered before it is defined, it would get its bindings as + // attributes and lose them on upgrade + expect(renderedTags()).not.toContain(handler.tagName) + expect(customElements.get(handler.tagName)).toBeUndefined() + + resolve() + await flushPromises() + + expect(customElements.get(handler.tagName)).toBeDefined() + expect(renderedTags()).toContain(handler.tagName) + }) + + it('calls onInit() once however many files open with it', async () => { + const handler = lazyHandler() + const { vm, renderedTags } = mountViewer([handler]) + const first = makeFile({ basename: 'first.txt' }) + const second = makeFile({ basename: 'second.txt' }) + await vm.open([first, second], first) + await flushPromises() + await vm.open([first, second], second) + await flushPromises() + + expect(handler.onInit).toHaveBeenCalledOnce() + expect(renderedTags()).toContain(handler.tagName) + }) + + it('shows the error when onInit() fails, and tries again on the next open', async () => { + const handler = lazyHandler() + vi.mocked(handler.onInit!).mockRejectedValueOnce(new Error('Failed to fetch dynamically imported module')) + const { vm, errorText, renderedTags } = mountViewer([handler]) + const file = makeFile() + await vm.open([file], file) + await flushPromises() + + expect(errorText()).toBe('Failed to fetch dynamically imported module') + expect(renderedTags()).not.toContain(handler.tagName) + + vm.close() + await vm.open([file], file) + await flushPromises() + + expect(handler.onInit).toHaveBeenCalledTimes(2) + expect(renderedTags()).toContain(handler.tagName) + }) +}) diff --git a/__tests__/registerHandler.spec.ts b/__tests__/registerHandler.spec.ts index 6b0b68a..fbaa1f1 100644 --- a/__tests__/registerHandler.spec.ts +++ b/__tests__/registerHandler.spec.ts @@ -40,3 +40,13 @@ describe('registering a handler whose id is taken', () => { expect(warn).not.toHaveBeenCalled() }) }) + +describe('a handler that defines its element in onInit()', () => { + it('is refused when onInit is not a function', () => { + expect(() => registerHandler(makeHandler({ + id: 'not-an-init', + tagName: 'oca-viewer-not-an-init', + onInit: 'later' as never, + }))).toThrow('Handler onInit must be a function if provided') + }) +}) diff --git a/lib/handlers.ts b/lib/handlers.ts index 77083b8..44b51ef 100644 --- a/lib/handlers.ts +++ b/lib/handlers.ts @@ -69,6 +69,18 @@ export interface IHandler { */ preload?: (node: IFile, space?: { width: number, height: number }) => Promise + /** + * Called the first time the viewer needs the element, to define it + * (`customElements.define()` with `tagName`). + * + * The viewer waits for the returned promise and for the element to be + * defined (`customElements.whenDefined()`) before rendering it, so the + * view and everything it imports stay out of the registration script + * that runs on every page. Leave it out when the element is already + * defined by the time the viewer opens. + */ + onInit?: () => Promise + /** * Viewer modal theme (one of 'dark', 'light', 'default') */ @@ -365,6 +377,10 @@ function validateHandler(handler: IHandler): void { throw new Error('Handler preload must be a function if provided') } + if (handler.onInit && typeof handler.onInit !== 'function') { + throw new Error('Handler onInit must be a function if provided') + } + if (handler.theme && !['dark', 'light', 'default'].includes(handler.theme)) { throw new Error("Handler theme must be one of 'dark', 'light', 'default' if provided") } diff --git a/lib/utils/customElements.ts b/lib/utils/customElements.ts index 4a0e779..142c848 100644 --- a/lib/utils/customElements.ts +++ b/lib/utils/customElements.ts @@ -2,6 +2,8 @@ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: AGPL-3.0-or-later */ +import type { IHandler } from '../handlers.ts' + import { logger } from '../services/logger.ts' /** @@ -23,3 +25,29 @@ export function defineCustomElementOnce(tagName: string, constructor: CustomElem window.customElements.define(tagName, constructor) } + +/** The handlers being initialized, by tag, so each one is initialized once however often it is asked for */ +const initializing = new Map>() + +/** + * Make sure a handler's element is defined, initializing the handler first if it has to. + * + * Resolves at once for an element that is defined already, or for a + * handler without `onInit()`, which defines its element itself. An + * `onInit()` that fails is forgotten, so the next open tries again. + * + * @param handler - The handler whose element is about to be rendered + */ +export function initHandlerElement(handler: IHandler): Promise { + if (handler.onInit === undefined || window.customElements.get(handler.tagName) !== undefined) { + return Promise.resolve() + } + + let pending = initializing.get(handler.tagName) + if (pending === undefined) { + pending = handler.onInit().then(() => window.customElements.whenDefined(handler.tagName)).then(() => undefined) + initializing.set(handler.tagName, pending) + pending.catch(() => initializing.delete(handler.tagName)) + } + return pending +} diff --git a/lib/views/Viewer.vue b/lib/views/Viewer.vue index a894a3a..737d0f8 100644 --- a/lib/views/Viewer.vue +++ b/lib/views/Viewer.vue @@ -153,7 +153,7 @@ (offerRotate.value ? 1 : 0) + (offerEdit.value ? 1 : 0) + (isMobile.value ? 0 : 1 + (offerSidebar.value ? 1 : 0))) +// Tags whose element a handler's onInit() has defined. The registry itself +// is not reactive, so this is what tells the template it may render them. +const loadedTags = ref(new Set()) + +/** + * Whether a handler's element can be rendered yet. + * + * Rendered before its tag is defined, the element gets its bindings as + * attributes and loses them when it upgrades, so one that comes from + * `onInit()` waits for it. + * + * @param handler - The handler about to be rendered + */ +function elementReady(handler?: IHandler): boolean { + return handler !== undefined + && (handler.onInit === undefined || loadedTags.value.has(handler.tagName) || window.customElements.get(handler.tagName) !== undefined) +} + +watch([currentHandler, comparisonHandler], (handlers) => { + for (const handler of handlers) { + if (handler === undefined || elementReady(handler)) { + continue + } + initHandlerElement(handler).then(() => { + loadedTags.value = new Set([...loadedTags.value, handler.tagName]) + }, (error: unknown) => { + logger.error('Could not initialize a handler', { handler: handler.id, error }) + onError(error) + }) + } +// Synchronous: closing and reopening with the same handler in one tick is a +// change a deferred watcher would not see, and the retry after a failed +// load would never happen +}, { immediate: true, flush: 'sync' }) + // Files actions rendered in the viewer menu (download, delete, …), linked to // the Files actions and run with the view/folder forwarded by the opener. const { actions: fileActions, enabledSubmenuActions, isValidMenu, actionLabel, actionIcon, execAction } = useViewerActions( From c007bac6c1307c7fd84bf1b82d7ea2f792ef2a0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Molakvo=C3=A6?= <14975046+skjnldsv@users.noreply.github.com> Date: Thu, 1 Oct 2026 21:59:01 +0200 Subject: [PATCH 2/2] feat(handlers): explain the tagName convention, and take app ids with underscores MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Like the files sidebar tabs: how the element gets defined, when the viewer waits for it, and why the name starts with the app id. App ids like files_pdfviewer have an underscore, which the custom element spec allows but the tag name check refused. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: John Molakvoæ <14975046+skjnldsv@users.noreply.github.com> --- README.md | 7 +++++-- __tests__/fileActions.spec.ts | 10 ++++++++++ lib/handlers.ts | 18 +++++++++++++++--- 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 9fc45ca..5027e4f 100644 --- a/README.md +++ b/README.md @@ -336,8 +336,11 @@ The full handler shape (see the `IHandler` interface): Gotchas: -- `tagName` must be lowercase, contain a hyphen, and have no leading, trailing - or consecutive hyphens (e.g. `my-app-viewer`). An invalid one throws. +- `tagName` must be lowercase (letters, digits, `_` and `-`), contain a hyphen, + and have no leading, trailing or consecutive hyphens. An invalid one throws. + Custom elements share one registry for the whole page, so start it with your + app id to avoid clashing with another app's: for the app `your_app`, a good + name is `your_app-viewer-handler`. - `id` must be unique **across every app on the page**, not just your own: it is not namespaced for you. A collision does not throw: the second registration is silently dropped with a console warning, so pick something diff --git a/__tests__/fileActions.spec.ts b/__tests__/fileActions.spec.ts index 620fb3b..87fe79d 100644 --- a/__tests__/fileActions.spec.ts +++ b/__tests__/fileActions.spec.ts @@ -118,6 +118,16 @@ describe('registerHandler validation', () => { .toThrow('Handler tagName must not start or end with a hyphen (-)') }) + it('takes a tagName starting with an app id that has an underscore', () => { + expect(() => registerHandler(makeHandler({ id: 'pdf', tagName: 'files_pdfviewer-viewer-handler' }))) + .not.toThrow() + }) + + it('throws on a tagName with any other character', () => { + expect(() => registerHandler(makeHandler({ tagName: 'oca-viewer.pdf' }))) + .toThrow('Handler tagName must only contain lowercase letters, numbers, underscores (_) and hyphens (-)') + }) + it('throws on a tagName ending with a hyphen', () => { expect(() => registerHandler(makeHandler({ tagName: 'oca-viewer-' }))) .toThrow('Handler tagName must not start or end with a hyphen (-)') diff --git a/lib/handlers.ts b/lib/handlers.ts index 44b51ef..f6c4265 100644 --- a/lib/handlers.ts +++ b/lib/handlers.ts @@ -37,7 +37,19 @@ export interface IHandler { iconSvgInline?: string /** - * The custom element tag name to use for this handler. + * The tag name of the custom element that shows the file. + * + * The element must be defined under this name with + * `CustomElementRegistry.define()`, either when the handler is registered + * or within the `onInit` callback (preferred, as it keeps the view out of + * the script that runs on every page). With `onInit`, the viewer waits for + * the element to be defined (`customElements.whenDefined()`) before + * rendering it. + * + * Custom elements share one registry for the whole page, so to avoid name + * clashes the name has to start with your app id (e.g. `your_app`). In + * addition to the custom element naming rules (lowercase, with a hyphen), + * a good name would be `your_app-viewer-handler`. */ tagName: string @@ -406,7 +418,7 @@ function validateCustomElementName(tagName: string): void { if (tagName.startsWith('-') || tagName.endsWith('-')) { throw new Error('Handler tagName must not start or end with a hyphen (-)') } - if (!/^[a-z][a-z0-9-]*$/.test(tagName)) { - throw new Error('Handler tagName must only contain lowercase letters, numbers, and hyphens (-)') + if (!/^[a-z][a-z0-9_-]*$/.test(tagName)) { + throw new Error('Handler tagName must only contain lowercase letters, numbers, underscores (_) and hyphens (-)') } }