diff --git a/.github/skills/vscode-messenger/SKILL.md b/.github/skills/vscode-messenger/SKILL.md index a94bcad..de8079d 100644 --- a/.github/skills/vscode-messenger/SKILL.md +++ b/.github/skills/vscode-messenger/SKILL.md @@ -81,7 +81,7 @@ class ColorsViewProvider implements vscode.WebviewViewProvider { - `new Messenger(options?)` — `MessengerOptions`: `ignoreHiddenViews` (default `true`), `uniqueHandlers` (throws if a handler for the same method is registered twice — incompatible with sender-scoped handlers; see that section below), `debugLog`. - `registerWebviewView(view, options?)` / `registerWebviewPanel(panel, options?)` — returns a `WebviewIdMessageParticipant` with the assigned `webviewId`. Use this returned participant to address that **specific instance** (vs. the `webviewType` string which addresses **all instances** of that type). The library auto-unregisters on `onDidDispose`. -- `onRequest(type, handler, { sender? })` / `onNotification(type, handler, { sender? })` — return a `Disposable`. **Extension side:** for **notifications**, multiple handlers for the same method stack and all fire. For **requests**, registering more than one handler for the same method is invalid — it causes a runtime error response ("Multiple matching request handlers") when that request arrives; use `sender`-scoped handlers instead if you need per-webview logic for the same method (see below). **Webview side:** registering a new handler for the same method **replaces** the previous one (last-write-wins). The optional `sender` (extension side only) filters the handler so it only fires for messages from that participant. +- `onRequest(type, handler, { sender? })` / `onNotification(type, handler, { sender? })` — return a `Disposable`. **Both sides:** for **notifications**, multiple handlers for the same method stack and all fire. For **requests**, only one handler per method is allowed — a second registration for an overlapping scope **throws synchronously** at registration time (webview: any second request handler for the method; extension: a second handler whose `sender` scope overlaps). On the extension side, use non-overlapping `sender`-scoped handlers if you need per-webview request logic for the same method (see below). Disposing a handler removes only that specific registration. The optional `sender` (extension side only) filters the handler so it only fires for messages from that participant. - `sendRequest(type, receiver, params?, cancelable?)` — returns `Promise`. Receiver is a `MessageParticipant` (webview by id, webview by type, or — once supported — another extension). Cannot be `BROADCAST` — throws immediately on both sides. - `sendNotification(type, receiver, params?)` — fire-and-forget. Receiver may be `BROADCAST`. @@ -121,6 +121,48 @@ const vscodeApi = acquireVsCodeApi(); const messenger = new Messenger(vscodeApi); ``` +## React webviews (Vite + StrictMode) + +When the webview UI is a React app, create the `Messenger` **once** outside the component tree, and register handlers inside a `useEffect` that disposes them on cleanup. This is required because React StrictMode (dev) mounts every component twice (`setup → cleanup → setup`), and Vite HMR re-runs effects on every edit. Registering a **request** handler without disposing the previous one throws on the second run (`A request handler is already registered for method ...`); a **notification** handler without cleanup fires twice. + +```tsx +// messenger.ts — module scope, created exactly once per webview +import { Messenger } from 'vscode-messenger-webview'; +export const messenger = new Messenger(); // acquireVsCodeApi() called once here + +// App.tsx +import { useEffect, useState } from 'react'; +import { HOST_EXTENSION } from 'vscode-messenger-common'; +import { messenger } from './messenger'; +import { ColorModify, GetColors } from './shared/message-types'; + +export function App() { + const [colors, setColors] = useState([]); + + useEffect(() => { + // Collect every registration and dispose it on cleanup. + const disposables = [ + messenger.onNotification(ColorModify, action => { + if (action === 'clear') setColors([]); + }), + messenger.onRequest(GetColors, () => colors), + ]; + messenger.start(); // idempotent — safe to call again after a StrictMode remount + + return () => disposables.forEach(d => d.dispose()); + }, []); // run once on mount; StrictMode runs setup→cleanup→setup, which stays balanced + + return null; +} +``` + +Key rules for React: + +- **One `Messenger` per webview, at module scope** — never `new Messenger()` inside a component body (`acquireVsCodeApi()` may be called only once, and a new instance per render leaks listeners). +- **Register in `useEffect`, dispose in its cleanup** — the returned cleanup disposes each `Disposable`, so the StrictMode double-invoke and HMR re-runs stay balanced. +- **`start()` is idempotent** — calling it again after a remount is a no-op (`if (this.started) return`). +- **Avoid registering in the component body or in a `useMemo`** — those run on every render and will throw on the second request-handler registration. + ## Core patterns ### Request / response @@ -210,7 +252,7 @@ If you address by `webviewType` instead and multiple instances of that type are ### Runtime and targeting pitfalls -- **Webview handlers are last-write-wins** — on the webview side, `onRequest`/`onNotification` for the same method **replace** the previous handler. On the extension side, notification handlers stack and all fire, but registering more than one **request** handler for the same method is invalid usage: it results in a runtime error response ("Multiple matching request handlers") rather than being silently accepted. Use `uniqueHandlers: true` to catch accidental duplicate registrations at registration time (throws immediately). If you re-register on the webview during HMR or re-mount, dispose the old `Disposable` first. +- **Duplicate request handlers throw at registration** — registering a second **request** handler for the same method throws synchronously (webview: any duplicate; extension: when the `sender` scopes overlap). This surfaces accidental double-registration immediately instead of failing later at dispatch. **Notification** handlers, by contrast, stack: registering the same method twice means the handler fires twice. Always keep the returned `Disposable` and call `dispose()` before re-registering (during HMR, re-mount, or React StrictMode) — see the React section below. The extension-side `uniqueHandlers: true` option additionally forbids **all** duplicate method registrations (requests and notifications), which is incompatible with sender-scoped handlers. - **`webviewType` with multiple instances races requests** — if you really want to broadcast a question and aggregate, you have to do it yourself (iterate instances by id and `Promise.all`). The library only returns the first response. - **`extensionId` is reserved for future use** — `ExtensionMessageParticipant.extensionId` is in the type but cross-extension messaging isn't implemented. `sendRequest` to `{ type: 'extension', extensionId }` throws. Use `HOST_EXTENSION` (no `extensionId`) for the host extension. - **`webviewId` changes on every register** — the id is generated fresh each time `registerWebviewView` / `registerWebviewPanel` is called. Don't persist it across sessions; capture the returned participant and use it for the lifetime of that view. diff --git a/AGENTS.md b/AGENTS.md index 55af622..a8f3ed1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -7,7 +7,7 @@ Typed RPC library for message-passing between a VS Code extension host and its w ```sh npm install # ~2s warm npm run build # tsc -b (typecheck+compile all packages) + browserify bundles + devtools-ui vite build + lint, ~8.5s -npm test # jest: vscode-messenger + vscode-messenger-webview, 44 tests, ~2.4s +npm test # jest: vscode-messenger + vscode-messenger-webview, 53 tests, ~2.4s npx jest packages/vscode-messenger/tests/messenger.test.ts # single test file npm run lint # eslint --workspaces, ~3.7s npm run watch # tsc -b -w, incremental rebuild across all packages @@ -28,12 +28,16 @@ See [docs/ARCHITECTURE.md](docs/ARCHITECTURE.md) for the full module index. ## Conventions - Keep all `NotificationType`/`RequestType` declarations in one shared module imported by both the extension and webview side — the same `method` string and generic types must match on both ends, and nothing enforces this beyond a shared import. +- A `method` string must be used exclusively for requests **or** for notifications — never both. `onRequest` and `onNotification` throw synchronously at registration time if this rule or a conflicting sender scope is violated; the error message names the method and the conflicting handler kind. +- On the extension-host side, multiple `onRequest` handlers for the same method are allowed only when each uses a distinct, non-overlapping `sender` scope (enforced at registration). Multiple `onNotification` handlers always stack and all fire in parallel. +- On the webview side, at most one `onRequest` handler per method; multiple `onNotification` handlers stack. Always keep the returned `Disposable` and call `dispose()` before re-registering (required in React `useEffect` — see README.md for the pattern, especially under StrictMode). - ESLint already enforces most style (single quotes, semicolons, `no-explicit-any`, `no-var`, arrow/paren spacing — see `eslint.config.js`); don't restate those as review comments. - Package build order follows the project references in `tsconfig.build.json`: `vscode-messenger-common` → `vscode-messenger-webview` / `vscode-messenger` → `vscode-messenger-devtools` → its `webview-ui` → `examples/*`. Don't add an import that runs against this direction (e.g. `vscode-messenger-common` importing from `vscode-messenger`). ## Boundaries and definition of done - Never hand-edit `packages/*/lib/` or `packages/*/webview-ui/build/` — these are build outputs regenerated by `npm run build`. +- Before editing a config file (e.g. `tsconfig.json`, `package.json`), check its `git log` to understand why the lines exist; verify that your change does not alter the published package content — in particular, confirm that `lib/` still receives the same flat output structure after build (incident: #64 — `rootDir` change caused `lib/src/` subdirs and would have broken `"main": "lib/index.js"`). - Never hand-edit `examples/*/media/web-view-bundle-*.js` — generated by the `browserify` step of `npm run build`. - The three publishable packages (`vscode-messenger`, `vscode-messenger-webview`, `vscode-messenger-common`) must keep matching version ranges on each other — see [releasing.md](releasing.md) before touching any `package.json` version or dependency range. - Done means `npm run build`, `npm test`, and `npm run lint` all pass locally; include the final line(s) of each command's output in your response to confirm. diff --git a/README.md b/README.md index 9f7ddfb..69276d7 100644 --- a/README.md +++ b/README.md @@ -95,6 +95,45 @@ messenger.sendNotification(colorSelectType, HOST_EXTENSION, 'a85b20'); > **Note:** `messenger.start()` must be called before any messages can be received. Forgetting it causes all incoming messages to be silently dropped. +## Usage in a React webview + +When the webview is a React app (e.g. Vite + React), create the `Messenger` **once** at module scope and register handlers inside a `useEffect` that disposes them on cleanup. This is required for React **StrictMode** (which mounts each component twice in development: `setup → cleanup → setup`) and for HMR, both of which re-run effects. + +Registering a handler without disposing the previous one has observable consequences: a second **request** handler for the same method throws (`A request handler is already registered for method ...`), and a second **notification** handler stacks and fires twice. The `useEffect` cleanup keeps registrations balanced. + +```tsx +// messenger.ts — created exactly once per webview +import { Messenger } from 'vscode-messenger-webview'; +export const messenger = new Messenger(); + +// App.tsx +import { useEffect, useState } from 'react'; +import { HOST_EXTENSION } from 'vscode-messenger-common'; +import { messenger } from './messenger'; +import { colorModifyType, availableColorsType } from './shared/message-types'; + +export function App() { + const [colors, setColors] = useState([]); + + useEffect(() => { + const disposables = [ + messenger.onNotification(colorModifyType, action => { + if (action === 'clear') setColors([]); + }), + messenger.onRequest(availableColorsType, () => colors), + ]; + messenger.start(); // idempotent — safe after a StrictMode remount + + return () => disposables.forEach(d => d.dispose()); + }, []); + + return null; +} +``` + +**Do:** one `Messenger` at module scope, register in `useEffect`, dispose in its cleanup. +**Don't:** call `new Messenger()` or `onRequest`/`onNotification` in the component body — those run on every render and will throw on the second request-handler registration. + ## Key concepts ### Message participants diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 9a6c956..1261c3a 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -22,7 +22,8 @@ examples/calico-colors (consumes vscode-messenger + vscode-messenger-webview + Runtime-agnostic types shared by both ends of the channel. -- `messages.ts` — `NotificationType

` / `RequestType` type tags, `MessageParticipant` variants, `HOST_EXTENSION`, `BROADCAST`, the wire message shapes. +- `messages.ts` — `NotificationType

` / `RequestType` type tags, `MessageParticipant` variants, `HOST_EXTENSION`, `BROADCAST`, the wire message shapes, `MessengerAPI` interface, handler types. +- `util.ts` — internal utilities shared by both `Messenger` implementations: `HandlerRegistration`, `HandlerKind`, `participantToString`, `wrongHandlerKindMessage`. Not part of the public user-facing API; consumed only by the two `Messenger` classes. - `cancellation.ts` — cancellation message types shared between the extension's `vscode.CancellationToken` and the webview's `AbortSignal` bridge. ### `packages/vscode-messenger/` @@ -51,3 +52,17 @@ Reference extension (two webview types: `calico-colors-view.ts`, `cat-coding-vie - `vscode-messenger-common` has no dependency on `vscode` or DOM APIs — it must stay usable from both the extension host and the webview sandbox. - The devtools introspection contract (`MessengerDiagnostic`) is the only sanctioned way to observe another extension's messenger traffic; it must not require the observed extension to import `vscode-messenger-devtools`. - `webview-ui` is a `noEmit` TypeScript project (typecheck-only via the root `tsc -b` graph); its actual bundle is produced separately by its own `tsc && vite build` script, invoked from the root `npm run build` — see [AGENTS.md](../AGENTS.md#commands). +- `acquireVsCodeApi()` may be called **at most once** per webview lifetime (VS Code restriction). `vscode-messenger-webview`'s `Messenger` constructor calls it internally. Creating more than one `Messenger` instance per webview would call it twice and throw. Keep the `Messenger` at module scope, outside component trees — see the React example in [README.md](../README.md). +- A `method` string must be used for requests **or** for notifications, never both, on a given side. This is enforced at handler registration time (throws on violation). See [ADR-0004](adr/0004-handler-registration-enforcement.md). +- `Disposable.dispose()` on a handler registration removes exactly that registration by identity — not all registrations for the method. This is a prerequisite for independent notification-handler stacking. See [ADR-0003](adr/0003-disposable-return-from-handler-registration.md). + +## Decision records + +Key architectural decisions are in [`docs/adr/`](adr/): + +| ADR | Decision | +|---|---| +| [0001](adr/0001-two-sided-cancellation-bridge.md) | Two-sided cancellation: `vscode.CancellationToken` on host, `AbortSignal` bridge on webview | +| [0002](adr/0002-webview-participant-union-type.md) | `WebviewMessageParticipant` as discriminated union (not optional fields) | +| [0003](adr/0003-disposable-return-from-handler-registration.md) | `onRequest`/`onNotification` return `Disposable`, not `this` | +| [0004](adr/0004-handler-registration-enforcement.md) | Handler registration throws immediately on conflicting scope or mixed kind (v0.7.0) | diff --git a/docs/adr/0001-two-sided-cancellation-bridge.md b/docs/adr/0001-two-sided-cancellation-bridge.md new file mode 100644 index 0000000..8388cca --- /dev/null +++ b/docs/adr/0001-two-sided-cancellation-bridge.md @@ -0,0 +1,29 @@ +--- +status: accepted +date: 2024-12-01 +--- + +# ADR-0001: Two-sided cancellation bridge — `vscode.CancellationToken` on the host, `AbortSignal` on the webview + +## Context + +Request cancellation was requested in [#16](https://github.com/TypeFox/vscode-messenger/issues/16). The library spans two runtime environments that have incompatible native cancellation primitives: the VS Code extension host exposes `vscode.CancellationToken` (and `CancellationTokenSource`) as the ecosystem-standard; browser-side code uses `AbortSignal` / `AbortController` from the Web Platform API. A request handler on the host side receives `CancellationToken` from the VS Code runtime itself (e.g. editor commands pass one in). Forcing the webview side to use `CancellationToken` would require shipping a userland implementation of something VS Code provides natively on the host; forcing the host side to use `AbortSignal` would break the VS Code extension contract. + +## Options considered + +1. **`AbortSignal` on both sides** — avoids a bespoke type; requires a userland `CancellationToken` implementation on the host (reimplements what VS Code provides) and disconnects host handlers from VS Code's own cancellation plumbing. +2. **`vscode.CancellationToken` on both sides** — host stays idiomatic; requires shipping a browser polyfill of `CancellationTokenSource` in the webview package, adding complexity and runtime size. +3. **Native type per side, bridge at the webview boundary (chosen)** — host uses `vscode.CancellationToken` natively; webview calls `createCancellationToken(abortSignal)` to produce a `CancellationToken`-shaped object from any `AbortSignal`. + +## Decision + +We use `vscode.CancellationToken` as the shared cancellation contract on all request handlers (both host and webview) and provide `createCancellationToken(signal: AbortSignal)` in `vscode-messenger-webview` to bridge the Web Platform primitive into that contract. + +The bridge is one-directional (AbortSignal → CancellationToken) and one-line: it keeps the host side idiomatic to VS Code and the webview side idiomatic to the browser without requiring a polyfill in either direction. + +## Consequences + +- **Easier:** host handlers integrate directly with VS Code's built-in cancellation sources (editor commands, task runners); no extra import needed. +- **Easier:** webview consumers use standard `AbortController`/`AbortSignal` which is already familiar and available without imports. +- **Harder:** webview consumers must import and call `createCancellationToken`; the bridge is not automatic. +- **Follow-up:** the internal `CancellationToken` interface in `vscode-messenger-common` must remain a structural (duck-typed) subset of `vscode.CancellationToken` so both the VS Code-provided token and the bridge's output satisfy it — any narrowing of that interface is a breaking change. diff --git a/docs/adr/0002-webview-participant-union-type.md b/docs/adr/0002-webview-participant-union-type.md new file mode 100644 index 0000000..a9e922c --- /dev/null +++ b/docs/adr/0002-webview-participant-union-type.md @@ -0,0 +1,31 @@ +--- +status: accepted +date: 2022-08-04 +--- + +# ADR-0002: `WebviewMessageParticipant` as a discriminated union, not a single interface with optional fields + +## Context + +The original `WebviewMessageParticipant` interface had both `webviewId` and `webviewType` as optional strings ([#5](https://github.com/TypeFox/vscode-messenger/issues/5)): + +```ts +interface WebviewMessageParticipant { type: 'webview'; webviewId?: string; webviewType?: string; } +``` + +This silently allowed constructing `{ type: 'webview' }` — a participant with neither field set — which the library would reject at runtime with a confusing error. The type system gave no help. + +## Options considered + +1. **Single interface, runtime validation** — add a guard that throws immediately when neither field is set; keep the optional fields. Preserves a simpler type structure but loses compile-time safety. +2. **Discriminated union (chosen)** — split into `WebviewIdMessageParticipant` (requires `webviewId: string`) and `WebviewTypeMessageParticipant` (requires `webviewType: string`); `WebviewMessageParticipant` is their union. Illegal states become unrepresentable at the type level. + +## Decision + +We model `WebviewMessageParticipant` as `WebviewIdMessageParticipant | WebviewTypeMessageParticipant` so the compiler rejects participant objects that carry neither field. The type guards `isWebviewIdMessageParticipant` and `isWebviewTypeMessageParticipant` (exported from `vscode-messenger-common`) are the sanctioned way to narrow the union at runtime. + +## Consequences + +- **Easier:** TypeScript narrows correctly; passing `{ type: 'webview' }` is a compile error; the type guards replace manual field checks throughout the library. +- **Harder:** `webviewId` and `webviewType` cannot be accessed directly on the union without narrowing — code that previously read `participant.webviewType` unconditionally must call `isWebviewTypeMessageParticipant` first. +- **Note:** a concrete sender populated by the library (inside `registerViewContainer`) always carries both fields (`webviewId` AND `webviewType`) as the extension knows both; a consumer-constructed participant carries exactly one. `equalParticipants` is designed for the concrete sender case and uses structural equality; it is **not** suitable for comparing two consumer-constructed filter participants against each other — use `sendersOverlap` in `vscode-messenger/src/messenger.ts` for that. diff --git a/docs/adr/0003-disposable-return-from-handler-registration.md b/docs/adr/0003-disposable-return-from-handler-registration.md new file mode 100644 index 0000000..06f00ca --- /dev/null +++ b/docs/adr/0003-disposable-return-from-handler-registration.md @@ -0,0 +1,26 @@ +--- +status: accepted +date: 2026-01-15 +--- + +# ADR-0003: `onRequest`/`onNotification` return `Disposable`, not `this` + +## Context + +The original webview-side `onRequest` and `onNotification` returned `this` (fluent chaining). This was raised as a gap in [#51](https://github.com/TypeFox/vscode-messenger/issues/51): there was no public way to unregister a handler. The workaround required accessing the private `handlerRegistry` directly. The problem is acute in React webviews: `useEffect` hooks must return a cleanup function, and stale closures from un-disposed handlers are a common source of bugs. The `this`-return pattern is also fundamentally incompatible with a design where `dispose()` targets a specific registration rather than the whole method entry. + +## Options considered + +1. **Add `unregisterHandler(method)` as a separate method, keep `this`-return** — public unregistration, but `this`-return means the returned value is not the disposable, so the React `useEffect` pattern requires an extra call; chaining `messenger.onRequest(...).onNotification(...)` is also order-sensitive and hard to reason about. +2. **Return `Disposable` and drop fluent chaining (chosen)** — align with VS Code's own `Disposable` pattern; the caller captures the return value and calls `dispose()` on cleanup; pairs naturally with `vscode.ExtensionContext.subscriptions.push(...)` and React `useEffect` return. + +## Decision + +We return a `Disposable` from `onRequest` and `onNotification` on both the extension-host and webview sides. Fluent chaining is removed. The `Disposable.dispose()` method removes exactly the registration it was issued for (by identity, not by method name), so disposing one notification handler does not affect other handlers registered for the same method. + +## Consequences + +- **Easier:** React `useEffect` cleanup, VS Code subscription management, and any pattern that needs to register/unregister handlers at different lifecycle points all work directly with the returned `Disposable`. +- **Easier:** multiple notification handlers for the same method can each be independently disposed. +- **Harder:** existing code using fluent chaining (`messenger.onRequest(...).onNotification(...)`) breaks — this was a breaking change shipped in v0.6.0. +- **Follow-up:** the `Disposable` return is the prerequisite for the kind-homogeneity enforcement in ADR-0004 — without per-registration identity, a `dispose()` that deleted the whole map entry would interact badly with stacked notification handlers. diff --git a/docs/adr/0004-handler-registration-enforcement.md b/docs/adr/0004-handler-registration-enforcement.md new file mode 100644 index 0000000..4ad47f6 --- /dev/null +++ b/docs/adr/0004-handler-registration-enforcement.md @@ -0,0 +1,39 @@ +--- +status: accepted +date: 2026-08-28 +--- + +# ADR-0004: Handler registration throws immediately on conflicting scope or mixed kind + +## Context + +Raised in [#63](https://github.com/TypeFox/vscode-messenger/issues/63). Before v0.7.0: + +- **Webview side:** registering a second `onRequest`/`onNotification` for the same method silently overwrote the previous handler (last-write-wins). The `Disposable` returned by the first registration would, when disposed, delete the whole map entry — removing the second handler instead of the first. No error was ever raised. +- **Host side:** a second overlapping `onRequest` registration succeeded silently; the conflict surfaced only at dispatch time as an error response ("Multiple matching request handlers"). A notification handler and a request handler could also coexist under the same method name, causing the wrong handler type to be invoked depending on which arrived first in the dispatch loop. + +Three observable defects: silent overwrite (wrong handler fires), mis-targeted dispose (wrong registration removed), and runtime dispatch error (instead of an early registration error). + +## Options considered + +1. **Status quo — runtime errors only** — cheap; defects surface at dispatch time (potentially long after the bad registration), with no information about where the duplicate registration happened. +2. **`uniqueHandlers: true` option** — opt-in strict mode. Already existed but gated on a flag, blocked sender-scoped multi-handler patterns, and did not distinguish notification stacking from request conflicts. +3. **Typed enforcement at registration time (chosen)** — throw synchronously from `onRequest` when a conflicting scope exists; throw from both `onRequest` and `onNotification` when the method already has handlers of the opposite kind. Notification handlers stack freely (all fire). Request handlers allow multiple registrations only with provably non-overlapping sender scopes. + +## Decision + +We enforce handler invariants at registration time on both sides: + +- **Kind homogeneity:** a method may be used for requests or for notifications, never both. Any registration attempt for the opposite kind throws immediately with the method name and both kinds in the message. +- **Request uniqueness per scope:** on the host side, a second `onRequest` for the same method throws if its `sender` scope overlaps an existing request handler's scope. Non-overlapping scopes (e.g. two different `webviewId` values) coexist. On the webview side, at most one request handler per method. +- **Notification stacking:** `onNotification` always stacks — all matching handlers fire in parallel at dispatch. +- **`Disposable.dispose()` removes by identity** (not by method name), so stacked notification handlers can be independently managed. + +The `uniqueHandlers: true` option is retained for teams that want to disallow even notification stacking, but its error message now names the option so the cause is visible. + +## Consequences + +- **Easier:** double-registration bugs surface immediately at the call site, not at the next incoming message; stack traces point directly to the offending `onRequest` call. +- **Easier:** React StrictMode and HMR re-runs are well-defined: the `useEffect` cleanup (calling `dispose()`) removes exactly the stale handler, and the second mount re-registers cleanly. +- **Harder (breaking):** code that (accidentally or intentionally) registered overlapping request handlers now throws on the second registration instead of failing silently at dispatch. This was shipped as a minor version bump (v0.7.0) under `0.x` SemVer conventions, where the minor position carries the breaking-change signal. +- **Follow-up:** the `sendersOverlap` function (host-side, in `messenger.ts`) is conservative about `webviewId`-vs-`webviewType` pairs — it treats them as non-overlapping at registration time and relies on the dispatch-time `length > 1` guard for any cross-scope ambiguity that can't be resolved statically. Tightening this to registry-aware overlap detection is deferred. diff --git a/package-lock.json b/package-lock.json index 35e12db..a16b501 100644 --- a/package-lock.json +++ b/package-lock.json @@ -44,6 +44,30 @@ "vscode": "^1.74.0" } }, + "examples/calico-colors/node_modules/vscode-messenger": { + "version": "0.6.1", + "resolved": "https://registry.npmjs.org/vscode-messenger/-/vscode-messenger-0.6.1.tgz", + "integrity": "sha512-WBfvs6UsoQN3+q9Z5h03Ht/xPriu3ItO9BgZgr9invV0slSexLsM6PFa4ixpXELkvnNlh7mASP/VAf7DvcTeiw==", + "license": "MIT", + "dependencies": { + "vscode-messenger-common": "^0.6" + } + }, + "examples/calico-colors/node_modules/vscode-messenger-common": { + "version": "0.6.1", + "resolved": "https://registry.npmjs.org/vscode-messenger-common/-/vscode-messenger-common-0.6.1.tgz", + "integrity": "sha512-QqF+Rz44n3KRMHh4mk5WXSLJHHl+aCTkzZZ0hGl2tnzPp+F1rNv8K6K3fhwJhjkoqKhko3WgEzR88JgNUCIsCw==", + "license": "MIT" + }, + "examples/calico-colors/node_modules/vscode-messenger-webview": { + "version": "0.6.1", + "resolved": "https://registry.npmjs.org/vscode-messenger-webview/-/vscode-messenger-webview-0.6.1.tgz", + "integrity": "sha512-ynuyazyEjURr1gsNbL5EGmMcHph6Lp5vHzWj9SvVu+6KLbYD6u/HaXezhy0pNVefUvNuBDLqjod9SHlu01t94Q==", + "license": "MIT", + "dependencies": { + "vscode-messenger-common": "^0.6" + } + }, "node_modules/@babel/code-frame": { "version": "7.29.7", "resolved": "https://registry.npmjs.org/@babel/code-frame/-/code-frame-7.29.7.tgz", @@ -1087,9 +1111,9 @@ "license": "MIT" }, "node_modules/@eslint/config-array/node_modules/brace-expansion": { - "version": "1.1.14", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.14.tgz", - "integrity": "sha512-MWPGfDxnyzKU7rNOW9SP/c50vi3xrmrua/+6hfPbCS2ABNWfx24vPidzvC7krjU/RTo235sV776ymlsMtGKj8g==", + "version": "1.1.18", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.18.tgz", + "integrity": "sha512-Edep/X9fGqVNmzKBVsDYIOtD+z1tuezV70LBjdCst9Tqu76lsnvRiZ6oTic1n+/BIwX6QDGAO94PN4N2SADvtw==", "dev": true, "license": "MIT", "dependencies": { @@ -1168,9 +1192,9 @@ "license": "MIT" }, "node_modules/@eslint/eslintrc/node_modules/brace-expansion": { - "version": "1.1.14", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.14.tgz", - "integrity": "sha512-MWPGfDxnyzKU7rNOW9SP/c50vi3xrmrua/+6hfPbCS2ABNWfx24vPidzvC7krjU/RTo235sV776ymlsMtGKj8g==", + "version": "1.1.18", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.18.tgz", + "integrity": "sha512-Edep/X9fGqVNmzKBVsDYIOtD+z1tuezV70LBjdCst9Tqu76lsnvRiZ6oTic1n+/BIwX6QDGAO94PN4N2SADvtw==", "dev": true, "license": "MIT", "dependencies": { @@ -1391,9 +1415,9 @@ } }, "node_modules/@istanbuljs/load-nyc-config/node_modules/js-yaml": { - "version": "3.14.2", - "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-3.14.2.tgz", - "integrity": "sha512-PMSmkqxr106Xa156c2M265Z+FTrPl+oxd/rgOQy2tijQeK5TxQ43psO1ZCwhVOSdnn+RzkzlRz/eY4BgJBYVpg==", + "version": "3.15.2", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-3.15.2.tgz", + "integrity": "sha512-6EuL879VkRA+1Cz578mKMiKvjPNEuk6+r1JaFzoSWejZmtf7xWbIyw1e3KkxlkzTIt9Taw6JBhEppG7utc1P+w==", "dev": true, "license": "MIT", "dependencies": { @@ -3102,16 +3126,16 @@ "license": "MIT" }, "node_modules/brace-expansion": { - "version": "5.0.6", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.6.tgz", - "integrity": "sha512-kLpxurY4Z4r9sgMsyG0Z9uzsBlgiU/EFKhj/h91/8yHu0edo7XuixOIH3VcJ8kkxs6/jPzoI6U9Vj3WqbMQ94g==", + "version": "5.0.9", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.9.tgz", + "integrity": "sha512-ScQ4IuvIEF1TMlP7Zt+vjJ//9zlPb2SDcxWxM3bk8s6t6GGdJ7KO1dCcTidOPJKePW30LE/2cT7wCyPho9/Wxg==", "dev": true, "license": "MIT", "dependencies": { "balanced-match": "^4.0.2" }, "engines": { - "node": "18 || 20 || >=22" + "node": "20 || >=22" } }, "node_modules/braces": { @@ -4562,9 +4586,9 @@ "license": "MIT" }, "node_modules/eslint/node_modules/brace-expansion": { - "version": "1.1.14", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.14.tgz", - "integrity": "sha512-MWPGfDxnyzKU7rNOW9SP/c50vi3xrmrua/+6hfPbCS2ABNWfx24vPidzvC7krjU/RTo235sV776ymlsMtGKj8g==", + "version": "1.1.18", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.18.tgz", + "integrity": "sha512-Edep/X9fGqVNmzKBVsDYIOtD+z1tuezV70LBjdCst9Tqu76lsnvRiZ6oTic1n+/BIwX6QDGAO94PN4N2SADvtw==", "dev": true, "license": "MIT", "dependencies": { @@ -5079,9 +5103,9 @@ "license": "MIT" }, "node_modules/glob/node_modules/brace-expansion": { - "version": "1.1.14", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.14.tgz", - "integrity": "sha512-MWPGfDxnyzKU7rNOW9SP/c50vi3xrmrua/+6hfPbCS2ABNWfx24vPidzvC7krjU/RTo235sV776ymlsMtGKj8g==", + "version": "1.1.18", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.18.tgz", + "integrity": "sha512-Edep/X9fGqVNmzKBVsDYIOtD+z1tuezV70LBjdCst9Tqu76lsnvRiZ6oTic1n+/BIwX6QDGAO94PN4N2SADvtw==", "dev": true, "license": "MIT", "dependencies": { @@ -6554,9 +6578,9 @@ "license": "MIT" }, "node_modules/js-yaml": { - "version": "4.2.0", - "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.2.0.tgz", - "integrity": "sha512-ePWsvanv0DWuDRsW8dnt+R4jQ31SCRCQ7hhNcPXZPsoBZiemuZNYGf7adZdqX2D86j6rvKp3RpCxVTSb8WQlOw==", + "version": "4.3.2", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.3.2.tgz", + "integrity": "sha512-SFNOvSJ+Dgf/9An904Yx+CgSlIPCkIpao4qo51lpee25TIRejdH3rhR4EZMGoNx3/TP3O+wzWuiTFl4sqbltzA==", "dev": true, "funding": [ { @@ -7041,9 +7065,9 @@ "license": "MIT" }, "node_modules/nanoid": { - "version": "3.3.12", - "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.12.tgz", - "integrity": "sha512-ZB9RH/39qpq5Vu6Y+NmUaFhQR6pp+M2Xt76XBnEwDaGcVAqhlvxrl3B2bKS5D3NH3QR76v3aSrKaF/Kiy7lEtQ==", + "version": "3.3.18", + "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.18.tgz", + "integrity": "sha512-DTg4MJbGMWkfi6VZFdNt2/caMbQy4Ou+Op/hJQvGEWcnVfoA1QA+xzRKAzw9jD6+GVOOeYr/mIcuDSdug6F6+w==", "dev": true, "funding": [ { @@ -7505,9 +7529,9 @@ } }, "node_modules/postcss": { - "version": "8.5.15", - "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.15.tgz", - "integrity": "sha512-FfR8sjd4em2T6fb3I2MwAJU7HWVMr9zba+enmQeeWFfCbm+UOC/0X4DS8XtpUTMwWMGbjKYP7xjfNekzyGmB3A==", + "version": "8.5.26", + "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.26.tgz", + "integrity": "sha512-u82N74LFzG8ca+dD8puPnplTXoGH4fTPpVGuIbt36G3qvNlkvfD0lEAZSxaly3KX8TS/L1A1gsCEmvKmBcVbkQ==", "dev": true, "funding": [ { @@ -7525,7 +7549,7 @@ ], "license": "MIT", "dependencies": { - "nanoid": "^3.3.12", + "nanoid": "^3.3.17", "picocolors": "^1.1.1", "source-map-js": "^1.2.1" }, @@ -8136,9 +8160,9 @@ } }, "node_modules/shell-quote": { - "version": "1.8.4", - "resolved": "https://registry.npmjs.org/shell-quote/-/shell-quote-1.8.4.tgz", - "integrity": "sha512-VsC6n6vz1ihYYyZZwX7YZSF5l5x36ca17OC+a69h94YqB7X6XLwf+5MOgynYir2SLFUbl8gIYvBo8K8RoNQ6bQ==", + "version": "1.10.0", + "resolved": "https://registry.npmjs.org/shell-quote/-/shell-quote-1.10.0.tgz", + "integrity": "sha512-w1aiOKwKuRgtwAReIIj89puqg+I7GvX4IbLrvmhXbzQsj1+Zwi4VO3+fa6ZF91TWSjIxoEkKnMeHcLEODK5ZXA==", "dev": true, "license": "MIT", "engines": { @@ -8604,9 +8628,9 @@ "license": "MIT" }, "node_modules/test-exclude/node_modules/brace-expansion": { - "version": "1.1.14", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.14.tgz", - "integrity": "sha512-MWPGfDxnyzKU7rNOW9SP/c50vi3xrmrua/+6hfPbCS2ABNWfx24vPidzvC7krjU/RTo235sV776ymlsMtGKj8g==", + "version": "1.1.18", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.18.tgz", + "integrity": "sha512-Edep/X9fGqVNmzKBVsDYIOtD+z1tuezV70LBjdCst9Tqu76lsnvRiZ6oTic1n+/BIwX6QDGAO94PN4N2SADvtw==", "dev": true, "license": "MIT", "dependencies": { @@ -9497,24 +9521,24 @@ } }, "packages/vscode-messenger": { - "version": "0.6.1", + "version": "0.7.0", "license": "MIT", "dependencies": { - "vscode-messenger-common": "^0.6" + "vscode-messenger-common": "^0.7" }, "devDependencies": { "@types/vscode": "^1.53.0" } }, "packages/vscode-messenger-common": { - "version": "0.6.1", + "version": "0.7.0", "license": "MIT" }, "packages/vscode-messenger-devtools": { "version": "0.7.0", "license": "MIT", "dependencies": { - "vscode-messenger": "^0.6" + "vscode-messenger": "^0.7" }, "devDependencies": { "@types/node": "^26.0.0", @@ -9597,6 +9621,21 @@ "dev": true, "license": "MIT" }, + "packages/vscode-messenger-devtools/node_modules/vscode-messenger-common": { + "version": "0.6.1", + "resolved": "https://registry.npmjs.org/vscode-messenger-common/-/vscode-messenger-common-0.6.1.tgz", + "integrity": "sha512-QqF+Rz44n3KRMHh4mk5WXSLJHHl+aCTkzZZ0hGl2tnzPp+F1rNv8K6K3fhwJhjkoqKhko3WgEzR88JgNUCIsCw==", + "license": "MIT" + }, + "packages/vscode-messenger-devtools/node_modules/vscode-messenger-webview": { + "version": "0.6.1", + "resolved": "https://registry.npmjs.org/vscode-messenger-webview/-/vscode-messenger-webview-0.6.1.tgz", + "integrity": "sha512-ynuyazyEjURr1gsNbL5EGmMcHph6Lp5vHzWj9SvVu+6KLbYD6u/HaXezhy0pNVefUvNuBDLqjod9SHlu01t94Q==", + "license": "MIT", + "dependencies": { + "vscode-messenger-common": "^0.6" + } + }, "packages/vscode-messenger-devtools/webview-ui": { "name": "devtools-ui", "version": "0.6.0", @@ -9618,10 +9657,10 @@ } }, "packages/vscode-messenger-webview": { - "version": "0.6.1", + "version": "0.7.0", "license": "MIT", "dependencies": { - "vscode-messenger-common": "^0.6" + "vscode-messenger-common": "^0.7" }, "devDependencies": { "jest-environment-jsdom": "^28.0", diff --git a/package.json b/package.json index 02fdcd0..9193b29 100644 --- a/package.json +++ b/package.json @@ -7,6 +7,7 @@ }, "scripts": { "clean": "rimraf \"packages/**/lib\" \"packages/**/build\" \"packages/**/*.tsbuildinfo\" \"examples/**/out\" \"examples/**/*.tsbuildinfo\" \"examples/**/media/web-view-bundle*.js\"", + "clean:modules": "rimraf \"node_modules\" \"packages/*/node_modules\" \"packages/*/*/node_modules\" \"examples/*/node_modules\"", "build": "tsc -b tsconfig.build.json && npm run browserify --workspaces --if-present && npm run build --workspace=packages/vscode-messenger-devtools/webview-ui && npm run lint", "watch": "tsc -b tsconfig.build.json -w", "test": "jest", diff --git a/packages/vscode-messenger-common/package.json b/packages/vscode-messenger-common/package.json index 9961fc2..d746175 100644 --- a/packages/vscode-messenger-common/package.json +++ b/packages/vscode-messenger-common/package.json @@ -1,6 +1,6 @@ { "name": "vscode-messenger-common", - "version": "0.6.1", + "version": "0.7.0", "description": "VS Code Messenger: common code shared by extension and webviews", "keywords": [ "vscode", diff --git a/packages/vscode-messenger-common/src/index.ts b/packages/vscode-messenger-common/src/index.ts index 4a5f9ed..c51a9ac 100644 --- a/packages/vscode-messenger-common/src/index.ts +++ b/packages/vscode-messenger-common/src/index.ts @@ -5,4 +5,5 @@ ******************************************************************************/ export * from './messages'; +export * from './util'; export * from './cancellation'; diff --git a/packages/vscode-messenger-common/src/util.ts b/packages/vscode-messenger-common/src/util.ts new file mode 100644 index 0000000..5a48b7c --- /dev/null +++ b/packages/vscode-messenger-common/src/util.ts @@ -0,0 +1,62 @@ +/****************************************************************************** + * Copyright 2022 TypeFox GmbH + * This program and the accompanying materials are made available under the + * terms of the MIT License, which is available in the project root. + ******************************************************************************/ + +import type { MessageParticipant, NotificationHandler, RequestHandler } from './messages'; +import { isWebviewIdMessageParticipant } from './messages'; + +/** + * Discriminates whether a registered handler serves requests or notifications. + */ +export type HandlerKind = 'request' | 'notification'; + +/** + * Internal record tracked per method name to keep track of a registered request/notification handler. + * Shared between the extension host and webview `Messenger` implementations. + */ +export interface HandlerRegistration { + handler: RequestHandler | NotificationHandler + kind: HandlerKind + /** Restricts a request/notification handler to a specific sender. Only used on the extension host side. */ + sender?: MessageParticipant +} + +/** + * Produce a human-readable representation of a message participant for logging and error messages. + */ +export function participantToString(participant: MessageParticipant | undefined): string { + if (!participant) { + return 'undefined'; + } + switch (participant.type) { + case 'extension': + return 'host extension'; + case 'webview': + if (isWebviewIdMessageParticipant(participant)) { + return participant.webviewId; + } else if (participant.webviewType) { + return participant.webviewType; + } else { + return 'unspecified webview'; + } + case 'broadcast': + return 'broadcast'; + } +} + +/** + * Build a diagnostic message for the case where an incoming message's kind does not match the kind + * of the handler registered for its method (e.g. a request arrives but only a notification handler + * is registered). This can only happen if the same method name is used for different kinds on the + * two communication sides. + * + * @param messageKind The kind of the incoming message. + * @param method The method name of the incoming message. + * @param registeredKind The kind of the handler that is actually registered for the method. + */ +export function wrongHandlerKindMessage(messageKind: HandlerKind, method: string, registeredKind: HandlerKind): string { + return `Received a ${messageKind} for method '${method}', but the registered handler is a ${registeredKind} handler. ` + + 'A method must be used exclusively for requests or for notifications on both communication sides.'; +} diff --git a/packages/vscode-messenger-devtools/package.json b/packages/vscode-messenger-devtools/package.json index 6baafbb..0ab7a7c 100644 --- a/packages/vscode-messenger-devtools/package.json +++ b/packages/vscode-messenger-devtools/package.json @@ -51,7 +51,7 @@ "test": "node ./lib/test/runTest.js" }, "dependencies": { - "vscode-messenger": "^0.6" + "vscode-messenger": "^0.7" }, "devDependencies": { "@types/node": "^26.0.0", diff --git a/packages/vscode-messenger-webview/CHANGELOG.md b/packages/vscode-messenger-webview/CHANGELOG.md index 3f56f5a..856fe48 100644 --- a/packages/vscode-messenger-webview/CHANGELOG.md +++ b/packages/vscode-messenger-webview/CHANGELOG.md @@ -1,5 +1,17 @@ # Change Log of `vscode-messenger-webview` +# Change Log of `vscode-messenger-webview` + +## v0.7.0 + +### Breaking Changes + +* **BREAKING: `onRequest()` now throws synchronously if a request handler is already registered for the same method** - Previously, registering a second `onRequest()` handler for the same method silently replaced the first one without any warning. + * Only one request handler is allowed per method. Dispose the existing handler first if you need to replace it. + * `onNotification()` is unaffected: multiple notification handlers can still be registered for the same method, and all of them are now correctly invoked (previously, only the most recently registered one was called). + * Disposing a handler now only removes that specific registration instead of clearing all handlers registered for the method. +* **BREAKING: A method name can no longer be used for both a request and a notification handler** - Registering a handler whose kind (request/notification) differs from an already registered handler for the same method now throws. A method must be used exclusively for requests or for notifications. + ## v0.6.0 (Jan. 2026) ### Breaking Changes diff --git a/packages/vscode-messenger-webview/jest.config.json b/packages/vscode-messenger-webview/jest.config.json index 0c86e21..a8c4c25 100644 --- a/packages/vscode-messenger-webview/jest.config.json +++ b/packages/vscode-messenger-webview/jest.config.json @@ -1,4 +1,9 @@ { "preset": "ts-jest", - "testEnvironment": "jsdom" + "testEnvironment": "jsdom", + "globals": { + "ts-jest": { + "tsconfig": "/tests/tsconfig.json" + } + } } diff --git a/packages/vscode-messenger-webview/package.json b/packages/vscode-messenger-webview/package.json index 783a481..05457cf 100644 --- a/packages/vscode-messenger-webview/package.json +++ b/packages/vscode-messenger-webview/package.json @@ -1,6 +1,6 @@ { "name": "vscode-messenger-webview", - "version": "0.6.1", + "version": "0.7.0", "description": "VS Code Messenger: webview integration", "keywords": [ "vscode", @@ -26,7 +26,7 @@ "publish:latest": "npm publish --tag latest" }, "dependencies": { - "vscode-messenger-common": "^0.6" + "vscode-messenger-common": "^0.7" }, "devDependencies": { "jsdom": "^17.0", diff --git a/packages/vscode-messenger-webview/src/messenger.ts b/packages/vscode-messenger-webview/src/messenger.ts index 3a6d6ea..cc7084a 100644 --- a/packages/vscode-messenger-webview/src/messenger.ts +++ b/packages/vscode-messenger-webview/src/messenger.ts @@ -6,7 +6,7 @@ import type { CancellationToken, Disposable, - JsonAny, Message, MessageParticipant, MessengerAPI, + HandlerRegistration, JsonAny, Message, MessageParticipant, MessengerAPI, NotificationHandler, NotificationMessage, NotificationType, RequestHandler, RequestMessage, RequestType, ResponseError, ResponseMessage } from 'vscode-messenger-common'; @@ -16,13 +16,14 @@ import { createCancelRequestMessage, isCancelRequestNotification, isMessage, - isNotificationMessage, isRequestMessage, isResponseMessage, isWebviewIdMessageParticipant + isNotificationMessage, isRequestMessage, isResponseMessage, + participantToString, wrongHandlerKindMessage } from 'vscode-messenger-common'; import type { VsCodeApi } from './vscode-api'; export class Messenger implements MessengerAPI { - protected readonly handlerRegistry: Map | NotificationHandler> = new Map(); + protected readonly handlerRegistry: Map = new Map(); // eslint-disable-next-line @typescript-eslint/no-explicit-any protected readonly requests: Map> = new Map(); protected readonly pendingHandlers: Map = new Map(); @@ -65,14 +66,12 @@ export class Messenger implements MessengerAPI { * // Or use the disposable for automatic cleanup * requestDisposable.dispose(); // Clean up when done * ``` + * + * @throws {Error} If a request handler is already registered for this method. Only one request handler + * is allowed per method; dispose the existing handler first if you need to replace it. */ onRequest(type: RequestType, handler: RequestHandler): Disposable { - this.handlerRegistry.set(type.method, handler as RequestHandler); - return { - dispose: () => { - this.unregisterHandler(type.method); - } - }; + return this.registerHandler(type.method, handler as RequestHandler, 'request'); } /** @@ -99,12 +98,44 @@ export class Messenger implements MessengerAPI { * // Or use the disposable for automatic cleanup * notificationDisposable.dispose(); // Clean up when done * ``` + * + * Multiple notification handlers can be registered for the same method; all of them are invoked + * for every received notification. */ onNotification

(type: NotificationType

, handler: NotificationHandler

): Disposable { - this.handlerRegistry.set(type.method, handler as NotificationHandler); + return this.registerHandler(type.method, handler as NotificationHandler, 'notification'); + } + + protected registerHandler( + method: string, + handler: RequestHandler | NotificationHandler, + kind: 'request' | 'notification' + ): Disposable { + const handlers = this.handlerRegistry.get(method) ?? []; + const existingKind = handlers[0]?.kind; + if (existingKind && existingKind !== kind) { + throw new Error(`Cannot register a ${kind} handler for method '${method}': a ${existingKind} handler is already registered for the same method. ` + + 'A method must be used exclusively for requests or for notifications.'); + } + if (kind === 'request' && handlers.length > 0) { + throw new Error(`A request handler is already registered for method '${method}'. ` + + 'Only one request handler is allowed per method; dispose the existing handler first if you need to replace it.'); + } + const registration: HandlerRegistration = { handler, kind }; + handlers.push(registration); + this.handlerRegistry.set(method, handlers); return { dispose: () => { - this.unregisterHandler(type.method); + const regs = this.handlerRegistry.get(method); + if (regs) { + const index = regs.indexOf(registration); + if (index >= 0) { + regs.splice(index, 1); + if (regs.length === 0) { + this.handlerRegistry.delete(method); + } + } + } } }; } @@ -175,9 +206,11 @@ export class Messenger implements MessengerAPI { this.log(`Received cancel notification for missing cancelable. ${msg.params}`, 'warn'); } } else { - const handler = this.handlerRegistry.get(msg.method); - if (handler) { - handler(msg.params, msg.sender!, new CancellationTokenImpl()); + const regs = this.handlerRegistry.get(msg.method); + if (regs && regs[0].kind === 'notification') { + await Promise.all(regs.map(reg => reg.handler(msg.params, msg.sender!, new CancellationTokenImpl()))); + } else if (regs) { + this.log(wrongHandlerKindMessage('notification', msg.method, regs[0].kind), 'warn'); } else if (msg.receiver.type !== 'broadcast') { this.log(`Received notification with unknown method: ${msg.method}`, 'warn'); } @@ -186,8 +219,9 @@ export class Messenger implements MessengerAPI { protected async processRequestMessage(msg: RequestMessage) { this.log(`View received Request message: ${msg.method} (id ${msg.id})`); - const handler = this.handlerRegistry.get(msg.method); - if (handler) { + const registration = this.handlerRegistry.get(msg.method)?.[0]; + if (registration?.kind === 'request') { + const handler = registration.handler; const cancelable = new CancellationTokenImpl(); try { this.pendingHandlers.set(msg.id, cancelable); @@ -213,12 +247,15 @@ export class Messenger implements MessengerAPI { this.pendingHandlers.delete(msg.id); } } else { - this.log(`Received request with unknown method: ${msg.method}`, 'warn'); + const message = registration + ? wrongHandlerKindMessage('request', msg.method, registration.kind) + : `Unknown method: ${msg.method}`; + this.log(message, 'warn'); const response: ResponseMessage = { id: msg.id, receiver: msg.sender!, error: { - message: `Unknown method: ${msg.method}` + message } }; this.vscode.postMessage(response); @@ -433,21 +470,3 @@ export function createCancellationToken(signal: AbortSignal): CancellationToken } }; } - -function participantToString(participant: MessageParticipant): string { - switch (participant.type) { - case 'extension': - return 'host extension'; - case 'webview': { - if (isWebviewIdMessageParticipant(participant)) { - return participant.webviewId; - } else if (participant.webviewType) { - return participant.webviewType; - } else { - return 'unspecified webview'; - } - } - case 'broadcast': - return 'broadcast'; - } -} diff --git a/packages/vscode-messenger-webview/tests/messenger-webview.test.ts b/packages/vscode-messenger-webview/tests/messenger-webview.test.ts index 90a6c60..891a59d 100644 --- a/packages/vscode-messenger-webview/tests/messenger-webview.test.ts +++ b/packages/vscode-messenger-webview/tests/messenger-webview.test.ts @@ -648,4 +648,39 @@ describe('Webview Messenger', () => { const notificationAlreadyUnregisteredResult = messenger.unregisterHandler('stringNotification'); expect(notificationAlreadyUnregisteredResult).toBe(false); }); + + test('Registering an onRequest handler for a method already used by onNotification throws', () => { + const messenger = new Messenger(vsCodeApi); + messenger.start(); + messenger.onNotification(stringRequest, () => undefined); + expect(() => messenger.onRequest(stringRequest, (params: string) => 'handled:' + params)) + .toThrow("Cannot register a request handler for method 'stringRequest': a notification handler is already registered for the same method. " + + 'A method must be used exclusively for requests or for notifications.'); + }); + + test('Registering an onNotification handler for a method already used by onRequest throws', () => { + const messenger = new Messenger(vsCodeApi); + messenger.start(); + messenger.onRequest(stringRequest, (params: string) => 'handled:' + params); + expect(() => messenger.onNotification(stringRequest, () => undefined)) + .toThrow("Cannot register a notification handler for method 'stringRequest': a request handler is already registered for the same method. " + + 'A method must be used exclusively for requests or for notifications.'); + }); + + test('Registering a second request handler for the same method throws', () => { + const messenger = new Messenger(vsCodeApi); + messenger.start(); + messenger.onRequest(stringRequest, (params: string) => 'handled1:' + params); + expect(() => messenger.onRequest(stringRequest, (params: string) => 'handled2:' + params)) + .toThrow("A request handler is already registered for method 'stringRequest'. " + + 'Only one request handler is allowed per method; dispose the existing handler first if you need to replace it.'); + }); + + test('Disposing a request handler allows re-registering a new one for the same method', () => { + const messenger = new Messenger(vsCodeApi); + messenger.start(); + const disposable = messenger.onRequest(stringRequest, (params: string) => 'handled1:' + params); + disposable.dispose(); + expect(() => messenger.onRequest(stringRequest, (params: string) => 'handled2:' + params)).not.toThrow(); + }); }); diff --git a/packages/vscode-messenger-webview/tests/tsconfig.json b/packages/vscode-messenger-webview/tests/tsconfig.json new file mode 100644 index 0000000..ed68a72 --- /dev/null +++ b/packages/vscode-messenger-webview/tests/tsconfig.json @@ -0,0 +1,16 @@ +{ + "extends": "../tsconfig.json", + "compilerOptions": { + "composite": false, + "noEmit": true, + "rootDir": "..", + "types": [ + "node", + "jest" + ] + }, + "include": [ + "../src/**/*", + "**/*" + ] +} diff --git a/packages/vscode-messenger/jest.config.json b/packages/vscode-messenger/jest.config.json index dfa6f8c..0eaa84e 100644 --- a/packages/vscode-messenger/jest.config.json +++ b/packages/vscode-messenger/jest.config.json @@ -1,4 +1,9 @@ { "preset": "ts-jest", - "testEnvironment": "node" + "testEnvironment": "node", + "globals": { + "ts-jest": { + "tsconfig": "/tests/tsconfig.json" + } + } } diff --git a/packages/vscode-messenger/package.json b/packages/vscode-messenger/package.json index 39f850a..26ce30e 100644 --- a/packages/vscode-messenger/package.json +++ b/packages/vscode-messenger/package.json @@ -1,6 +1,6 @@ { "name": "vscode-messenger", - "version": "0.6.1", + "version": "0.7.0", "description": "VS Code Messenger: extension integration", "keywords": [ "vscode", @@ -26,7 +26,7 @@ "publish:latest": "npm publish --tag latest" }, "dependencies": { - "vscode-messenger-common": "^0.6" + "vscode-messenger-common": "^0.7" }, "devDependencies": { "@types/vscode": "^1.53.0" diff --git a/packages/vscode-messenger/src/messenger.ts b/packages/vscode-messenger/src/messenger.ts index 2543d6f..520ffe1 100644 --- a/packages/vscode-messenger/src/messenger.ts +++ b/packages/vscode-messenger/src/messenger.ts @@ -6,14 +6,15 @@ import type * as vscode from 'vscode'; import type { - CancellationToken, JsonAny, Message, MessageParticipant, MessengerAPI, NotificationHandler, + CancellationToken, HandlerRegistration, JsonAny, Message, MessageParticipant, MessengerAPI, NotificationHandler, NotificationMessage, NotificationType, RequestHandler, RequestMessage, RequestType, ResponseError, ResponseMessage, WebviewIdMessageParticipant } from 'vscode-messenger-common'; -import { CancellationTokenImpl, createCancelRequestMessage, Deferred, +import { + CancellationTokenImpl, createCancelRequestMessage, Deferred, equalParticipants, HOST_EXTENSION, isCancelRequestNotification, isMessage, isNotificationMessage, isRequestMessage, isResponseMessage, - isWebviewIdMessageParticipant + isWebviewIdMessageParticipant, isWebviewTypeMessageParticipant, participantToString, wrongHandlerKindMessage } from 'vscode-messenger-common'; import type { DiagnosticOptions, MessengerDiagnostic, MessengerEvent } from './diagnostic-api'; @@ -84,7 +85,7 @@ export class Messenger implements MessengerAPI { !isWebviewIdMessageParticipant(handler.sender) || handler.sender.webviewId !== viewEntry.id); - if (newHandlers.length === 0 ) { + if (newHandlers.length === 0) { this.handlerRegistry.delete(key); } else { this.handlerRegistry.set(key, newHandlers); @@ -185,6 +186,11 @@ export class Messenger implements MessengerAPI { this.log(`Received request with unknown method: ${msg.method}`, 'warn'); return this.sendErrorResponse(`Unknown method: ${msg.method}`, msg, responseCallback); } + if (regs[0].kind !== 'request') { + const message = wrongHandlerKindMessage('request', msg.method, regs[0].kind); + this.log(message, 'warn'); + return this.sendErrorResponse(message, msg, responseCallback); + } const filtered = regs.filter(reg => !reg.sender || equalParticipants(reg.sender, msg.sender!)); if (filtered.length === 0) { @@ -259,6 +265,10 @@ export class Messenger implements MessengerAPI { } else { const regs = this.handlerRegistry.get(msg.method); if (regs) { + if (regs[0].kind !== 'notification') { + this.log(wrongHandlerKindMessage('notification', msg.method, regs[0].kind), 'warn'); + return; + } const filtered = regs.filter(reg => !reg.sender || equalParticipants(reg.sender, msg.sender!)); if (filtered.length > 0) { // TODO No need to cancel a notification @@ -295,6 +305,11 @@ export class Messenger implements MessengerAPI { * The handler will be called whenever a request with the specified method is received. * The handler should return the response data or throw an error for failed requests. * + * Only one request handler may be registered per method and overlapping sender scope: registering + * a second request handler whose `sender` filter overlaps with an already registered one (e.g. both + * omit `sender`, or use the same sender) throws synchronously instead of failing later at dispatch time. + * Handlers scoped to distinct, non-overlapping senders can coexist. + * * @template P The type of the request parameters * @template R The type of the response data * @param type The request type to handle @@ -302,9 +317,10 @@ export class Messenger implements MessengerAPI { * @param options Additional options for handler registration * @param options.sender Optional sender filter - if provided, only requests from this sender will trigger the handler * @returns A Disposable that can be used to unregister the handler + * @throws {Error} If a request handler with an overlapping sender scope is already registered for this method */ onRequest(type: RequestType, handler: RequestHandler, options: { sender?: MessageParticipant } = {}): vscode.Disposable { - return this.registerHandler(type, handler, options); + return this.registerHandler(type, handler, options, 'request'); } /** @@ -312,6 +328,8 @@ export class Messenger implements MessengerAPI { * * The handler will be called whenever a notification with the specified method is received. * Notification handlers don't return values and should not throw errors for normal operation. + * Multiple notification handlers can be registered for the same method (optionally scoped to different + * senders); all matching handlers are invoked for every received notification. * * @template P The type of the notification parameters * @param type The notification type to handle @@ -321,7 +339,7 @@ export class Messenger implements MessengerAPI { * @returns A Disposable that can be used to unregister the handler */ onNotification

(type: NotificationType

, handler: NotificationHandler

, options: { sender?: MessageParticipant } = {}): vscode.Disposable { - return this.registerHandler(type, handler, options); + return this.registerHandler(type, handler, options, 'notification'); } protected registerHandler( @@ -329,17 +347,33 @@ export class Messenger implements MessengerAPI { type: RequestType | NotificationType, // eslint-disable-next-line @typescript-eslint/no-explicit-any handler: RequestHandler | NotificationHandler, - options: { sender?: MessageParticipant } + options: { sender?: MessageParticipant }, + kind: 'request' | 'notification' ): vscode.Disposable { let handlers = this.handlerRegistry.get(type.method); if (handlers && this.options.uniqueHandlers) { - throw new Error(`A message handler is already registered for method ${type.method}.`); + throw new Error(`A message handler is already registered for method '${type.method}'. Registering more than one handler for the same method is not allowed because the 'uniqueHandlers' option is enabled.`); + } + if (handlers && handlers.length > 0) { + const existingKind = handlers[0].kind; + if (existingKind !== kind) { + throw new Error(`Cannot register a ${kind} handler for method '${type.method}': a ${existingKind} handler is already registered for the same method. ` + + 'A method must be used exclusively for requests or for notifications.'); + } + } + if (kind === 'request' && handlers) { + const conflict = handlers.find(reg => reg.kind === 'request' && sendersOverlap(reg.sender, options.sender)); + if (conflict) { + throw new Error(`A request handler is already registered for method '${type.method}' with an overlapping sender scope ` + + `(existing: ${participantToString(conflict.sender)}, new: ${participantToString(options.sender)}). ` + + 'Only one request handler is allowed per method and sender scope; dispose the existing handler first or use a non-overlapping sender.'); + } } if (!handlers) { handlers = []; this.handlerRegistry.set(type.method, handlers); } - const registration: HandlerRegistration = { handler, sender: options.sender }; + const registration: HandlerRegistration = { handler, sender: options.sender, kind }; handlers.push(registration); // Create a disposable that removes the message handler from the registry @@ -616,9 +650,25 @@ export interface ViewOptions { broadcastMethods?: string[] } -export interface HandlerRegistration { - handler: RequestHandler | NotificationHandler - sender: MessageParticipant | undefined +/** + * Two sender scopes overlap (i.e. could both match the same concrete sender) if either is unspecified + * (matches any sender), or if they are scoped to the same concrete webview (same `webviewId`) or the same + * webview type (same `webviewType`). A `webviewId`-scoped filter and a `webviewType`-scoped filter are + * treated as non-overlapping here, since it cannot be statically decided whether they refer to the same + * webview instance - `equalParticipants` is not suitable for this comparison because it is designed to + * match a concrete (always fully populated) sender against a filter, not to compare two filters directly. + */ +function sendersOverlap(a: MessageParticipant | undefined, b: MessageParticipant | undefined): boolean { + if (!a || !b) { + return true; + } + if (isWebviewIdMessageParticipant(a) && isWebviewIdMessageParticipant(b)) { + return a.webviewId === b.webviewId; + } + if (isWebviewTypeMessageParticipant(a) && isWebviewTypeMessageParticipant(b)) { + return a.webviewType === b.webviewType; + } + return a.type === b.type && a.type !== 'webview'; } class IdProvider { @@ -633,23 +683,3 @@ class IdProvider { return view.viewType + '_' + this.counter++; } } - -function participantToString(participant: MessageParticipant | undefined): string { - if (!participant) { - return 'undefined'; - } - switch (participant.type) { - case 'extension': - return 'host extension'; - case 'webview': - if (isWebviewIdMessageParticipant(participant)) { - return participant.webviewId; - } else if (participant.webviewType) { - return participant.webviewType; - } else { - return 'unspecified webview'; - } - case 'broadcast': - return 'broadcast'; - } -} diff --git a/packages/vscode-messenger/tests/messenger.test.ts b/packages/vscode-messenger/tests/messenger.test.ts index 0503d71..e50a358 100644 --- a/packages/vscode-messenger/tests/messenger.test.ts +++ b/packages/vscode-messenger/tests/messenger.test.ts @@ -252,21 +252,75 @@ describe('Extension Messenger', () => { }); test('Handle request with multiple handlers', async () => { - // suppress "Multiple request handlers" warn logging - const warn = jest.spyOn(console, 'warn').mockImplementation(() => null); - const messenger = new Messenger(); messenger.registerWebviewView(view1); messenger.onRequest(simpleRequest, (params: string) => { return 'handled1:' + params; }); - messenger.onRequest(simpleRequest, (params: string) => { + // Registering a second overlapping request handler for the same method throws immediately. + expect(() => messenger.onRequest(simpleRequest, (params: string) => { return 'handled2:' + params; - }); - // Simulate webview request - await view1.messageCallback({ ...simpleRequest, receiver: HOST_EXTENSION, id: 'fake_req_id', params: 'test' }); - expect(view1.messages[0]).toMatchObject({ id: 'fake_req_id', error: { message: 'Multiple matching request handlers' } }); - warn.mockRestore(); + })).toThrow("A request handler is already registered for method 'request' with an overlapping sender scope " + + '(existing: undefined, new: undefined). ' + + 'Only one request handler is allowed per method and sender scope; dispose the existing handler first or use a non-overlapping sender.'); + }); + + test('Registering an onRequest handler for a method already used by onNotification throws', () => { + const messenger = new Messenger(); + messenger.registerWebviewView(view1); + messenger.onNotification(simpleRequest, () => undefined); + expect(() => messenger.onRequest(simpleRequest, (params: string) => 'handled:' + params)) + .toThrow("Cannot register a request handler for method 'request': a notification handler is already registered for the same method. " + + 'A method must be used exclusively for requests or for notifications.'); + }); + + test('Registering an onNotification handler for a method already used by onRequest throws', () => { + const messenger = new Messenger(); + messenger.registerWebviewView(view1); + messenger.onRequest(simpleRequest, (params: string) => 'handled:' + params); + expect(() => messenger.onNotification(simpleRequest, () => undefined)) + .toThrow("Cannot register a notification handler for method 'request': a request handler is already registered for the same method. " + + 'A method must be used exclusively for requests or for notifications.'); + }); + + test('Registering two request handlers with the same webviewId sender scope throws', () => { + const messenger = new Messenger(); + messenger.registerWebviewView(view1); + const sender: MessageParticipant = { type: 'webview', webviewId: 'asdf' }; + messenger.onRequest(simpleRequest, (params: string) => 'handled1:' + params, { sender }); + expect(() => messenger.onRequest(simpleRequest, (params: string) => 'handled2:' + params, { sender })) + .toThrow("A request handler is already registered for method 'request' with an overlapping sender scope " + + '(existing: asdf, new: asdf). ' + + 'Only one request handler is allowed per method and sender scope; dispose the existing handler first or use a non-overlapping sender.'); + }); + + test('Registering two request handlers with the same webviewType sender scope throws', () => { + const messenger = new Messenger(); + messenger.registerWebviewView(view1); + const sender: MessageParticipant = { type: 'webview', webviewType: 'asdf' }; + messenger.onRequest(simpleRequest, (params: string) => 'handled1:' + params, { sender }); + expect(() => messenger.onRequest(simpleRequest, (params: string) => 'handled2:' + params, { sender })) + .toThrow("A request handler is already registered for method 'request' with an overlapping sender scope " + + '(existing: asdf, new: asdf). ' + + 'Only one request handler is allowed per method and sender scope; dispose the existing handler first or use a non-overlapping sender.'); + }); + + test('With uniqueHandlers enabled, a second handler for the same method always throws', () => { + const messenger = new Messenger({ uniqueHandlers: true }); + messenger.registerWebviewView(view1); + messenger.onNotification(simpleNotification, () => undefined); + expect(() => messenger.onNotification(simpleNotification, () => undefined)) + .toThrow("A message handler is already registered for method 'notification'. Registering more than one handler " + + "for the same method is not allowed because the 'uniqueHandlers' option is enabled."); + }); + + test('Disposing a conflicting request handler allows re-registering an overlapping one', () => { + const messenger = new Messenger(); + messenger.registerWebviewView(view1); + const disposable = messenger.onRequest(simpleRequest, (params: string) => 'handled1:' + params); + disposable.dispose(); + // Same (undefined) sender scope as the disposed handler - should not throw now. + expect(() => messenger.onRequest(simpleRequest, (params: string) => 'handled2:' + params)).not.toThrow(); }); test('Handle request with multiple handlers, but none matching', async () => { diff --git a/packages/vscode-messenger/tests/tsconfig.json b/packages/vscode-messenger/tests/tsconfig.json new file mode 100644 index 0000000..ed68a72 --- /dev/null +++ b/packages/vscode-messenger/tests/tsconfig.json @@ -0,0 +1,16 @@ +{ + "extends": "../tsconfig.json", + "compilerOptions": { + "composite": false, + "noEmit": true, + "rootDir": "..", + "types": [ + "node", + "jest" + ] + }, + "include": [ + "../src/**/*", + "**/*" + ] +}