diff --git a/README.md b/README.md index b9955c6..5027e4f 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,11 +332,15 @@ 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: -- `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__/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__/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/__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..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 @@ -69,6 +81,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 +389,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") } @@ -390,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 (-)') } } 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(