From 8e6af69083973d438dc6512be69a6b68752fe693 Mon Sep 17 00:00:00 2001 From: skjnldsv Date: Thu, 10 Sep 2026 09:57:01 +0200 Subject: [PATCH] fix(handlers): register the default handlers only when asked Importing the package no longer registers the image, video and audio handlers. The server does it from its `viewer-init` init script on every page, and a page the server does not set up calls `registerDefaultHandlers()` itself, as the playground already did. With several copies of the package on one page, each copy registering on import warned "Handler with id X is already registered." about the others. It also made the entry's side effects invisible from a consumer's import line, which is what the review on nextcloud/server#63954 asked to avoid. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: skjnldsv --- README.md | 12 +++++++++++- __tests__/defaults.spec.ts | 4 +++- __tests__/entry.spec.ts | 13 ++++++++++--- lib/defaults.ts | 8 +++++--- lib/index.ts | 7 ------- vite.config.ts | 4 ++-- 6 files changed, 31 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 41915ab..0b81915 100644 --- a/README.md +++ b/README.md @@ -203,7 +203,17 @@ for what happens between those two sentences. If you are not registering a handler, no server-side setup is needed. A plain `import { getViewer } from '@nextcloud/viewer'` in your regular bundle is enough — -no `\OCP\Util::addInitScript` required, the server always ships a copy of its own. +no `\OCP\Util::addInitScript` required, the server always ships a copy of its own +and registers the handlers for images, video and audio on every page. + +Importing the package registers nothing by itself. Only a page the server does +not set up, such as a standalone playground, needs to ask for those handlers: + +```ts +import { registerDefaultHandlers } from '@nextcloud/viewer' + +registerDefaultHandlers() +``` Only call `open()` in response to an actual user interaction, not eagerly at import or mount time — see [how a page ends up with one diff --git a/__tests__/defaults.spec.ts b/__tests__/defaults.spec.ts index e4d1262..78fe09d 100644 --- a/__tests__/defaults.spec.ts +++ b/__tests__/defaults.spec.ts @@ -26,13 +26,15 @@ describe('default handlers', () => { expect(scope.handlers!.has('images')).toBe(true) }) - it('do not complain about themselves when asked for explicitly', async () => { + it('register once, however often they are asked for', async () => { const { registerDefaultHandlers } = await importPackage() const { logger } = await import('../lib/services/logger.ts') const warn = vi.spyOn(logger, 'warn') + registerDefaultHandlers() registerDefaultHandlers() + expect(scope.handlers!.size).toBe(3) expect(warn).not.toHaveBeenCalled() }) }) diff --git a/__tests__/entry.spec.ts b/__tests__/entry.spec.ts index 4fcebf2..c977f4f 100644 --- a/__tests__/entry.spec.ts +++ b/__tests__/entry.spec.ts @@ -29,8 +29,14 @@ describe('importing @nextcloud/viewer', () => { expect(getViewer()).toBe(scope.service) }) - it('registers the image, video and audio handlers', async () => { - const { getHandlers } = await importEntry() + it('registers no handler until asked, then the image, video and audio ones', async () => { + const { getHandlers, registerDefaultHandlers } = await importEntry() + + // The server asks from an init script; a second copy on the page + // registering on import would only warn about the first + expect(getHandlers().size).toBe(0) + + registerDefaultHandlers() expect([...getHandlers().keys()].sort()).toEqual(['audios', 'images', 'videos']) }) @@ -59,7 +65,8 @@ describe('importing @nextcloud/viewer', () => { describe('the offered implementation', () => { it('mounts the viewer into the page and hands it to the service', async () => { - const { getViewer } = await importEntry() + const { getViewer, registerDefaultHandlers } = await importEntry() + registerDefaultHandlers() await scope.candidates[0]!.load() diff --git a/lib/defaults.ts b/lib/defaults.ts index b5fa006..5603224 100644 --- a/lib/defaults.ts +++ b/lib/defaults.ts @@ -12,9 +12,11 @@ let registered = false * Register the handlers for the file types the viewer shows out of the box: * images, video and audio. * - * Importing the package does this, so there is normally nothing to call. - * It stays exported for a consumer that wants them registered at a point - * of its own choosing, and does nothing on any call after the first. + * The server calls this from an init script on every page, so an app only + * needs to when it hosts the viewer on a page of its own. Importing the + * package deliberately does not: with several copies on a page, each one + * registering would only warn about the others. Any call after the first + * does nothing. */ export function registerDefaultHandlers(): void { if (registered) { diff --git a/lib/index.ts b/lib/index.ts index 54064ca..607ad1b 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -2,7 +2,6 @@ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: AGPL-3.0-or-later */ -import { registerDefaultHandlers } from './defaults.ts' import { registerImplementation } from './scope.ts' import { loadTranslations } from './utils/l10n.ts' import { getViewer } from './viewer.ts' @@ -27,12 +26,6 @@ registerImplementation({ // It is an empty shell until the viewer is mounted. getViewer() -// Images, video and audio are what the viewer is for, so an app gets them by -// importing the package rather than by remembering to ask. It has to happen -// here, at import: the Files list reads the available actions when it first -// renders, and a handler registered after that is a file that does not open. -registerDefaultHandlers() - export { canView, getHandlers, registerHandler } from './handlers.ts' export type { IHandler } from './handlers.ts' export { getViewer, Viewer } from './viewer.ts' diff --git a/vite.config.ts b/vite.config.ts index 8f46334..8e99e81 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -28,8 +28,8 @@ const translations = readdirSync('./l10n') /** * The strings the package can show before the viewer is loaded: the file - * actions it registers on import, and the handler names listed under - * "Open with …". Every other string belongs to the viewer itself and + * actions it registers along with the handlers, and the handler names listed + * under "Open with …". Every other string belongs to the viewer itself and * arrives with it. * * A string used by the entry but missing here is not an error, it just