Webview API package preview hardening and consumer reshape#676
Merged
Conversation
Extracts the generic webview API infrastructure into a new local
workspace package alongside the existing @documentdb-js/* packages.
Adopts the simplifications and bug fixes from the
tnaum-ms/vscode-webview-starter-kit reference repository:
- Pluggable telemetry: generic TelemetryContext interface +
createMiddleware factory; default console.log sink. The extension
plugs in its own callWithTelemetryAndErrorHandling-backed middleware.
- WebviewController genericised over <TRouter, TConfiguration,
TContext> and parameterised with appRouter + bundle layout via
constructor options (no static appRouter import; no ext.isBundle
coupling).
- Fix: void mutations now coalesce undefined to null so the client
observable completes — the structured-clone algorithm strips
undefined over postMessage.
- Drop redundant _onDisposed.dispose() after fire() in dispose().
- vscodeLink.ts and useTrpcClient genericised over consumer AppRouter.
- New TrpcClient<TRouter> type alias.
- Auto-recorded durationMs per RPC in the default middleware.
- Ported vscodeLink.test.ts (18 cases) as the link behavioural
contract; wired into the root Jest projects.
Split entry points keep the webview bundle free of Node/vscode deps:
- main entry (.) — webview-client only
- /server subpath — extension-host APIs (WebviewController,
router, publicProcedure, createMiddleware,
BaseRouterContext, TelemetryContext,
WithTelemetry, ...)
The extension does not yet import from this package; the migration
follows in a subsequent commit.
Replaces the in-tree src/webviews/api/{extension-server,webview-client,
configuration}/ folders with thin app-specific wrappers that bind the
generic framework package to DocumentDB:
- src/webviews/api/appRouter.ts — root tRPC tree +
DocumentDB BaseRouterContext (extends the framework base with
dbExperience + webviewName; telemetry typed against the framework's
TelemetryContext)
- src/webviews/api/WebviewRegistry.ts — app-specific webview keys
- src/webviews/api/trpc.ts — DocumentDB telemetry
middleware preserving the existing documentDB.rpc.* event-name
convention; redefines WithTelemetry<T> to surface ITelemetryContext
(suppressAll, etc.)
- src/webviews/api/WebviewController.ts — DocumentDB-tuned base class
pre-filling the framework's WebviewControllerOptions with appRouter,
bundle layout, and dev-server host
- src/webviews/api/useTrpcClient.ts — AppRouter-bound hook wrapper
Webview components import accessibility/configuration helpers directly
from @microsoft/vscode-webview-api; extension-host code imports from
@microsoft/vscode-webview-api/server. The old WebviewContext.tsx and
the three subfolders under src/webviews/api/ are removed.
Updates the in-tree GitHub Copilot skill files to reflect the
relocated symbols after the @microsoft/vscode-webview-api extraction:
src/webviews/api/extension-server/ -> @microsoft/vscode-webview-api/server
src/webviews/api/webview-client/ -> @microsoft/vscode-webview-api
src/webviews/api/configuration/ -> src/webviews/api/{appRouter,WebviewRegistry}.ts
Also regenerates l10n/bundle.l10n.json after the API surface move.
…preview Renames the workspace package to better differentiate it from VS Code core APIs: @microsoft/vscode-webview-api -> @microsoft/vscode-ext-react-webview packages/vscode-webview-api/ -> packages/vscode-ext-react-webview/ - Bumps version to 0.8.0-preview, aligning with the parent vscode-documentdb extension that currently ships it. - Updates the description to: "Webview infrastructure for VS Code extensions with type-safe tRPC RPC over postMessage, React hooks, pluggable telemetry middleware, and accessibility helpers". - Adds a README marking the package as Preview, documenting the two entry points (main + /server), and listing peer dependencies. - Updates every import path, project reference, Jest project entry, and skill doc reference to the new package name and folder.
Adds a "Scope" section to the README to set expectations for what does and does not belong in this package: - In scope: the webview transport (tRPC over postMessage, WebviewController, React hooks) plus minimum-viable WCAG helpers every VS Code webview eventually needs (currently: Announcer). - Out of scope: general-purpose React UI / accessibility toolbox (focus traps, keyboard shortcut managers, skip-links, route announcers, etc.). Consumers should reach for react-aria or their in-house framework for those. New helpers only land here when they are (a) tightly coupled to the VS Code webview lifecycle and (b) the same boilerplate every consumer would otherwise write themselves.
Relocates the Announcer screen-reader live-region component out of the @microsoft/vscode-ext-react-webview package and back into the extension where it is actually used. The package's job is the webview transport (tRPC over postMessage) and the minimum React glue for it; accessibility patterns belong in the consumer. - Adds src/webviews/components/accessibility/Announcer.tsx (and index.ts barrel) — content copied verbatim from the package. - Updates the 3 consumers (MonacoEditor.tsx, CollectionView.tsx, GetPerformanceInsightsCard.tsx) to import from the new local path. - Updates the accessibility-aria-expert skill doc to reference the in-extension path. The package still exports Announcer at this commit so the surface change is non-breaking; the actual deletion follows in a subsequent commit.
Relocates the context-menu-prevention hook out of the @microsoft/vscode-ext-react-webview package and back into the extension where it is used. The hook's allowlist hardcodes Monaco-specific CSS selectors, so it is application-flavoured UX policy rather than generic webview infrastructure. - Adds src/webviews/components/useSelectiveContextMenuPrevention.ts — content copied verbatim from the package. - Updates the 2 consumers (CollectionView.tsx, documentView.tsx) to split the package import and pull the hook from the new local path. The package still exports the hook at this commit so the surface change is non-breaking; the actual deletion follows in a subsequent commit.
Removes the accessibility helper (Announcer) and the Monaco-flavoured
context-menu hook (useSelectiveContextMenuPrevention) from the package.
Both were app-level concerns rather than transport infrastructure:
- Announcer is a generic WCAG-compliant screen-reader live region. A11y
belongs with the consumer or a dedicated a11y library, not bundled
into a webview transport package.
- useSelectiveContextMenuPrevention hardcodes Monaco-specific CSS
selectors in its allowlist. It is DocumentDB's UX policy for one
editor, not generic webview behaviour.
The two hooks live in the extension now (re-homed in the preceding
commits under src/webviews/components/accessibility/ and
src/webviews/components/useSelectiveContextMenuPrevention.ts).
What remains in the package after this commit is the strict transport
surface:
Webview side:
WebviewContext, WithWebviewContext, useConfiguration, useTrpcClient,
vscodeLink, TrpcClient (+ message types)
Extension side (/server):
WebviewController, router, publicProcedure,
publicProcedureWithTelemetry, createMiddleware, createCallerFactory,
BaseRouterContext, TelemetryContext, WithTelemetry
Also:
- package.json description drops "and accessibility helpers".
- README's "What's inside" drops the Accessibility helper bullet and
useSelectiveContextMenuPrevention from React hooks; entry-point
example drops Announcer; "Scope" is rewritten to "only the webview
transport, nothing UX-flavoured".
Marked breaking (!) per Conventional Commits because the package's
exported surface shrinks. Acceptable: the package is preview, not yet
published.
Adds three sections to the package README to match the production-grade shape of the vscode-webview-starter-kit's documentation: - "Architecture" — ASCII diagram of the extension-host / webview boundary, calling out the actual symbols on each side and pointing to the correct entry point (main vs /server). Establishes a shared mental model before the per-symbol "What's inside" rundown. - "Advanced — Sharing a single tRPC client across components" — guidance for views that grow past ~10 components and benefit from a shared client via React context. Includes a complete TrpcProvider example and a pros/cons table vs the per-component default. - "FAQ" — answers common consumer questions: why tRPC over raw postMessage; why two entry points; how to plug in a real telemetry sink (with @microsoft/vscode-azext-utils example); multiple panels; AbortSignal-driven cancellation; React-only scope of the package. Sets the documentation bar for future API iterations: each new section above is structural rather than narrative, so it stays useful while the preview surface evolves.
Telemetry middleware, publicProcedureWithTelemetry, and the WithTelemetry helper now live in appRouter.ts alongside the router tree they configure. Per-view routers import these primitives from appRouter directly. trpc.ts is removed; five files become four.
…eclaring The DocumentDB BaseRouterContext now intersects with the framework's BaseRouterContext (from @microsoft/vscode-ext-react-webview/server) and only adds dbExperience and webviewName. The telemetry? and signal? slots are inherited, so any future framework field lands here automatically.
…ntrollerBase The same-name shadow of the framework WebviewController class made stack traces and Ctrl+Click navigation ambiguous. The DocumentDB-side base class is now WebviewControllerBase (file and class), while the framework class keeps its WebviewController name. View controllers (CollectionViewController, DocumentsViewController) now extend WebviewControllerBase. Breaking change for in-tree consumers; the npm package surface is unchanged.
Lifts every customizable runtime value in src/webviews/api/ into a single WEBVIEW_CONFIG object: telemetry namespace and prefixes, bundle layout, and dev-server host. appRouter.ts and WebviewControllerBase.ts now read from it instead of declaring constants inline. The three telemetry prefixes are derived from one TELEMETRY_NAMESPACE constant, so renaming the namespace is a one-line change.
…Integration/ The folder is the local glue between this extension and @microsoft/vscode-ext-react-webview; "api" was too generic and overloaded with the extension's public API, REST/data API, and the package API. webviewIntegration accurately describes the role and the contents (extension-host controller, router, browser-side hook). All consumer imports are updated. Skill docs (webview-trpc-messaging, react-webview-architecture) are updated to match. Breaking change for in-tree consumers; npm package surface is unchanged.
Renames src/webviews/webviewIntegration/ to src/webviews/_integration/. The underscore prefix is the conventional "infrastructure / not feature code" signal in monorepos, and it sorts before feature folders (components/, documentdb/, ...) in VS Code's file explorer. With api/ the folder was first by alphabetical accident; with webviewIntegration/ it sorted at the bottom. _integration restores the desired top placement while staying honest about the role (local glue between this extension and @microsoft/vscode-ext-react-webview). Breaking change for in-tree consumers; the npm package surface is unchanged. All imports across src/ and the skill docs under .github/ are updated.
…riptions
Adds a small typed async-iterable utility under the /server entry that
bridges push-style domain events (event emitters, driver callbacks,
completion notifiers) into tRPC subscription procedures. The shape is:
- producer side: imperative `emit(event)` or `emit(type, payload)`
with discriminated-union narrowing on the second overload
- consumer side: `for await (const event of sink)` from inside a
subscription generator
- `close()` to complete the iterator on panel disposal
The class is single-consumer by design and matches tRPC's 1-to-1
subscription model. Events emitted before a consumer attaches are
buffered; events emitted after close() are silently dropped.
Exports added to @microsoft/vscode-ext-react-webview/server:
- TypedEventSink (class)
- DiscriminatedEvent, EventOfType<T, K>, UntypedEventEmitter (helper types)
README gets a new Advanced subsection ("Push events from the extension
host to the webview") that shows the full pattern: event union, events
router file, producer call site, and webview-side subscribe. The
events-router file convention is recommended as the place to put
push-event procedures.
Includes a 7-case Jest suite covering buffering, parked consumer
delivery, close semantics, and single-consumer enforcement.
Adds an opt-in webview-side error observer to
@microsoft/vscode-ext-react-webview. New public surface (default entry):
- errorLink<TRouter>(onError): TRPCLink<TRouter>
- ErrorHandler type
- UseTrpcClientOptions interface
- useTrpcClient<TRouter>(options?) - existing zero-arg call sites
continue to work; passing { onError } installs the link
Semantics:
- The handler fires for query and mutation errors, in addition to the
normal tRPC error flow. Call-site .catch handlers still run and
still receive the error - the link observes, it does not swallow.
- Subscription errors are intentionally not forwarded. Subscriptions
have their own .subscribe({ onError }) hook that gives the call
site enough control; forwarding here would surface errors twice.
- Non-Error rejections are normalised to Error instances before being
handed to the consumer.
Use it for single-place webview-side error UX: an ARIA Announcer, a
FluentUI Toaster, a telemetry sink.
README gets a new Advanced subsection ("Webview-side error observer")
covering both call paths (option on useTrpcClient, or composing
errorLink manually). errorLink is also added to "What's inside".
Includes a 6-case Jest suite covering query/mutation forwarding,
subscription pass-through, non-Error normalisation, and the happy path.
…ntroller
Adds a private safePostMessage(message) helper on the framework
WebviewController and routes every postMessage call site in the tRPC
dispatcher through it. The helper:
- returns early if the controller is already disposed
- wraps the postMessage call in try/catch to absorb synchronous throws
- attaches a no-op catch to the returned Thenable so a late rejection
does not surface as an unhandled promise rejection
The race this addresses: a subscription generator can yield one or two
more values after the panel was closed, because the per-subscription
AbortController only flags signal.aborted between yields. Without the
guard, those late yields hit postMessage on a disposed Webview, which
either throws synchronously or rejects the returned Thenable depending
on the VS Code version. The result was noisy uncaught exceptions in
the extension host. They were never real bugs, but they confused
debugging.
dispose() already aborts all _activeOperations and _activeSubscriptions
so server-side procedures stop early; this change is purely defensive
for the window between abort() and the next signal.aborted check.
No behaviour change for valid messages. Six call sites updated:
subscription value yield, subscription complete, subscription error,
subscription outer-catch, default result, default error.
Restructures the package README to be self-contained:
- new "Quick start" section walks through a four-file minimum (router,
controller, webview entry, view component) so a first-time consumer
can read the package surface in one pass without bouncing between
repos
- new "Starter kit and reference consumers" section makes the
tnaum-ms/vscode-webview-starter-kit link prominent and lists what
the kit covers that the package intentionally does not
(build configuration, accessibility helpers, Monaco wiring, demo
view), plus a freshness caveat that the kit may lag a release
while the surface stabilises
- stale `../api/appRouter` import path in the shared-client example
updated to the current `../_integration/appRouter` shape
- all 16 em-dashes replaced with plain hyphens, colons, or periods
README-only, no code changes.
Contributor
There was a problem hiding this comment.
Pull request overview
Hardens the preview surface of the new @microsoft/vscode-ext-react-webview workspace package (typed push-event bridge, optional webview-side error observer, safer panel messaging) while reshaping this repo’s consumer integration layer under src/webviews/_integration/ to keep the in-tree usage aligned with the package’s evolving API.
Changes:
- Added new package utilities and entrypoints (
TypedEventSink,errorLink,/serversplit) plus saferpostMessagehandling in the frameworkWebviewController. - Reshaped DocumentDB’s consumer glue layer into
src/webviews/_integration/(router + controller base + config + typed client hook) and rewired webviews to use it. - Re-homed a11y/helpers (e.g.,
Announcer, selective context-menu prevention) intosrc/webviews/components/and updated imports/docs.
Reviewed changes
Copilot reviewed 52 out of 58 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Adds workspace reference for new package |
| src/webviews/index.tsx | Uses package WithWebviewContext; updates registry import |
| src/webviews/documentdb/documentView/documentView.tsx | Switches to _integration client + component util paths |
| src/webviews/documentdb/documentView/documentsViewRouter.ts | Imports router primitives/context from _integration |
| src/webviews/documentdb/documentView/documentsViewController.ts | Moves to WebviewControllerBase |
| src/webviews/documentdb/collectionView/components/toolbar/ToolbarViewNavigation.tsx | Updates useTrpcClient import path |
| src/webviews/documentdb/collectionView/components/toolbar/ToolbarTableNavigation.tsx | Updates useTrpcClient import path |
| src/webviews/documentdb/collectionView/components/toolbar/ToolbarMainView.tsx | Uses package useConfiguration + _integration tRPC |
| src/webviews/documentdb/collectionView/components/queryInsightsTab/QueryInsightsTab.tsx | Uses package useConfiguration + _integration tRPC |
| src/webviews/documentdb/collectionView/components/queryInsightsTab/components/queryPlanSummary/QueryPlanSummary.tsx | Updates useTrpcClient import path |
| src/webviews/documentdb/collectionView/components/queryInsightsTab/components/optimizationCards/TipsCard.tsx | Updates useTrpcClient import path |
| src/webviews/documentdb/collectionView/components/queryInsightsTab/components/optimizationCards/custom/GetPerformanceInsightsCard.tsx | Moves Announcer import to components |
| src/webviews/documentdb/collectionView/components/queryEditor/QueryEditor.tsx | Uses package useConfiguration + _integration tRPC |
| src/webviews/documentdb/collectionView/collectionViewRouter.ts | Imports router primitives/context from _integration |
| src/webviews/documentdb/collectionView/collectionViewController.ts | Moves to WebviewControllerBase |
| src/webviews/documentdb/collectionView/CollectionView.tsx | Uses package useConfiguration; moves a11y/util imports |
| src/webviews/components/useSelectiveContextMenuPrevention.ts | New context-menu prevention hook (Monaco allowlist) |
| src/webviews/components/MonacoEditor.tsx | Moves Announcer import to components |
| src/webviews/components/accessibility/index.ts | Consolidates value+type export style |
| src/webviews/components/accessibility/Announcer.tsx | New ARIA live-region announcer component |
| src/webviews/api/webview-client/useTrpcClient.ts | Removed (replaced by package + _integration) |
| src/webviews/api/extension-server/trpc.ts | Removed (moved/merged into _integration/appRouter.ts) |
| src/webviews/api/configuration/appRouter.ts | Removed (replaced by _integration/appRouter.ts) |
| src/webviews/_integration/WebviewRegistry.ts | Updated relative imports after folder move |
| src/webviews/_integration/WebviewControllerBase.ts | New DocumentDB-tuned controller base over package controller |
| src/webviews/_integration/useTrpcClient.ts | New typed wrapper hook for package useTrpcClient |
| src/webviews/_integration/README.md | New signpost for integration layer responsibilities |
| src/webviews/_integration/configuration.ts | New consumer-owned knobs (telemetry/bundle/devhost) |
| src/webviews/_integration/appRouter.ts | New root router + telemetry middleware + shared procedures |
| packages/vscode-ext-react-webview/tsconfig.json | New composite TS project for workspace package |
| packages/vscode-ext-react-webview/src/webview-client/WebviewContext.tsx | New shared webview context/provider |
| packages/vscode-ext-react-webview/src/webview-client/vscodeLink.ts | Makes vscodeLink generic + exports options type |
| packages/vscode-ext-react-webview/src/webview-client/vscodeLink.test.ts | Updates tests for new generic vscodeLink |
| packages/vscode-ext-react-webview/src/webview-client/useTrpcClient.ts | New hook with optional onError observer |
| packages/vscode-ext-react-webview/src/webview-client/useConfiguration.ts | Docstring cleanup |
| packages/vscode-ext-react-webview/src/webview-client/index.ts | New browser-surface barrel export |
| packages/vscode-ext-react-webview/src/webview-client/errorLink.ts | New link to observe query/mutation errors |
| packages/vscode-ext-react-webview/src/webview-client/errorLink.test.ts | Tests for errorLink behavior |
| packages/vscode-ext-react-webview/src/server.ts | New server entrypoint for extension-host surface |
| packages/vscode-ext-react-webview/src/index.ts | New browser-only main entrypoint |
| packages/vscode-ext-react-webview/src/extension-server/WebviewController.ts | Adds options injection + safe postMessage routing |
| packages/vscode-ext-react-webview/src/extension-server/TypedEventSink.ts | New async-iterable event sink for subscriptions |
| packages/vscode-ext-react-webview/src/extension-server/TypedEventSink.test.ts | Tests for TypedEventSink |
| packages/vscode-ext-react-webview/src/extension-server/trpc.ts | New tRPC init + middleware factory + default telemetry |
| packages/vscode-ext-react-webview/src/extension-server/index.ts | New server-surface barrel export |
| packages/vscode-ext-react-webview/src/extension-server/BaseRouterContext.ts | New base ctx contract (telemetry/signal) |
| packages/vscode-ext-react-webview/README.md | New package docs + quick start + advanced usage |
| packages/vscode-ext-react-webview/package.json | New workspace package manifest + exports map |
| packages/vscode-ext-react-webview/jest.config.js | New package Jest config |
| package.json | Adds workspace dependency on new package |
| package-lock.json | Adds workspace link for new package |
| l10n/bundle.l10n.json | Removes unused localization key |
| jest.config.js | Adds new package to Jest projects |
| docs/ai-and-plans/PRs/webview-api-package-preview-hardening.md | Adds PR design/decision doc |
| .github/skills/webview-trpc-messaging/SKILL.md | Updates skill doc to new integration shape |
| .github/skills/react-webview-architecture/SKILL.md | Updates architecture doc references |
| .github/skills/react-webview-architecture/references/REACT_ARCHITECTURE_GUIDELINES.md | Updates guideline import paths |
| .github/skills/accessibility-aria-expert/SKILL.md | Updates Announcer import guidance |
Comments suppressed due to low confidence (2)
packages/vscode-ext-react-webview/src/extension-server/WebviewController.ts:508
dispose()firesthis._onDisposedbut no longer disposes theEventEmitteritself. Since_onDisposedisn't in_disposables, this can retain listeners longer than necessary. Consider callingthis._onDisposed.dispose()(typically after.fire()) to release resources.
packages/vscode-ext-react-webview/src/extension-server/WebviewController.ts:211- In
handleSubscriptionMessage,_activeSubscriptions.set(message.id, abortController)happens before looking up/awaiting the subscription procedure. If procedure lookup fails orawait procedure(...)throws before the async loop starts, the entry is never removed (cleanup currently only happens in the inner IIFE’sfinally). Add cleanup in the outercatch/finallypath (or defer inserting into the map until after the subscription is successfully started) to avoid leaking AbortControllers and stale subscription IDs.
If a subscription procedure was misnamed or its initial await threw, the abort controller had already been inserted into _activeSubscriptions and never removed (the inner finally only runs after the consumer task starts). Repeated invalid subscription calls on a long-lived panel could therefore leak stale (id, AbortController) entries. Defer the Map.set until after the iterator is obtained so an early failure falls through to the outer catch without ever inserting an entry. The inner finally remains the sole cleanup site for the successful path. Refs PR #676 review (MEDIUM-2).
Collaborator
Author
Fix: MEDIUM-2 — Subscription leak on early failure
Fix: defer Commit: 4d9cdaa |
The only runtime use of @vscode/l10n inside the package was a single
'Procedure not found: {name}' message in WebviewController, used for a
framework-internal protocol error that consumers cannot translate. The
bundle key was already removed from l10n/bundle.l10n.json earlier in
this PR, and l10n/scripts/build.l10n.mjs only scans ./src so it would
never be regenerated from the package source.
Make the message a plain Error string and remove @vscode/l10n from the
package's peer dependencies, the README peer-dep table, and the lockfile
entry. The HTML template in WebviewController still injects
vscode.l10n.bundle from the host into the webview — that uses VS Code's
already-loaded bundle, not the peer.
Refs PR #676 review (MEDIUM-3).
The install paragraph previously listed react-dom alongside the real peers (react, @trpc/*). The package itself never imports react-dom; only consumer app shells do (the README quick-start example uses createRoot from react-dom/client because that is how a typical React app boots a webview, not because this package depends on it). The peer-dep table and package.json have always been consistent with that. Drop react-dom from the install sentence and add a one-liner explaining that react-dom is a transitive concern of the consumer's DOM shell, not a peer of this package. Refs PR #676 review (LOW-1).
…disposed The _onDisposed EventEmitter was created at construction time and fired during dispose(), but its own .dispose() was never called, so its internal listener array survived for the lifetime of the extension host. Low-impact (controllers don't outlive their panels), but easy to make the lifecycle complete. Register the emitter in _disposables at construction so the existing _disposables.forEach(d => d.dispose()) loop in dispose() tears it down right after _onDisposed.fire(). Order matters: fire() runs first, then the disposables array (including the emitter) is disposed, so subscribers still see the notification before the emitter goes away. Refs PR #676 review (LOW-2).
Collaborator
Author
Fix: LOW-2 —
|
The createMiddleware example imported createMiddleware, publicProcedure, and BaseRouterContext from '@microsoft/vscode-ext-react-webview', but the package's main entry intentionally exports only the browser/webview- client surface. These server symbols live under the '/server' subpath (matching the README example and the actual exports map in package.json). Refs PR #676 review (LOW-4).
Collaborator
Author
Fix: LOW-4 — JSDoc imported server symbols from the main entryThe Fix: changed the JSDoc import to Commit: edd1274 |
The PR that renamed the workspace package from @microsoft/vscode-webview-api to @microsoft/vscode-ext-react-webview left a stale 'extraneous' entry for packages/vscode-webview-api in the lockfile. Regenerate cleanly with 'rm -rf node_modules package-lock.json && npm install' so the lockfile reflects only the workspace packages that actually exist. The large diff is mostly alphabetical reordering of the resolved package entries — the dependency graph itself is unchanged. Verified: build, lint, prettier, and jest (96 suites, 1948 tests) all green after the regenerate. Refs PR #676 review (LOW-3).
Collaborator
Author
Fix: LOW-3 — Stale workspace entry in lockfileThe earlier rename of the workspace package ( Fix: regenerated the lockfile cleanly: rm -rf node_modules package-lock.json
npm installThe diff is large (~12k/~10k lines) but almost entirely alphabetical reordering of resolved package entries; the dependency graph itself is unchanged. Verified post-regen:
Commit: 00f3664 |
Until now only 'npm run build' and 'npm run jesttest' ran the workspace build chain ahead of time (via prebuild / prejesttest). Webpack-driven scripts — webpack-dev, webpack-prod, watch:ext, watch:views — went straight to the bundler, which resolves @microsoft/vscode-ext-react-webview to packages/vscode-ext-react-webview/dist per its package.json 'exports' field. On a clean checkout or after the package source changed without 'npm run build', that resolved to missing or stale output and silently bundled the wrong code. Add pre* hooks so every bundle/watch path runs 'npm run build --workspaces --if-present' first: prewebpack-dev, prewebpack-prod, prewatch:ext, prewatch:views 'npm run package' and 'npm run package-prerelease' inherit the safety transitively because they call 'npm run webpack-prod', which now has its own pre-hook. Verified by removing packages/vscode-ext-react-webview/dist and running 'npm run webpack-prod' — the dist was recreated by the pre- hook before webpack started. Aligns in-repo dev with the npm-consumer reality: every consumer of the package resolves it through 'dist', so always build dist first. When the package leaves this repo, these hooks can be removed in one commit. Refs PR #676 review (HIGH-1).
Before, both subscription.stop and panel disposal only aborted the per-
operation AbortController. That signal is cooperative: it cannot unblock
an iterator parked on next(), which is exactly the shape used by
TypedEventSink — consumers wait on 'new Promise((resolve) => …)' for the
next emit() or close(). When a client unsubscribed mid-stream while the
panel stayed alive, the procedure's generator stayed parked indefinitely.
vscode-cosmosdb hits the same shape and works around it by having every
panel call sink.close() in its own dispose(); we want the framework to
handle this instead of relying on producer discipline.
Two changes:
1. TypedEventSink iterator gains a real return() implementation. It
marks the sink closed, clears any buffered events, resolves a pending
next() with { done: true }, and releases the single-consumer guard
so a fresh for-await can iterate again (and will see done: true).
This makes 'break' inside a for-await over a sink release the
pending promise per the JS iterator protocol — no host cooperation
needed for the natural case either.
2. WebviewController now tracks both the AbortController and the live
AsyncIterator per subscription. handleSubscriptionMessage normalizes
the procedure result (AsyncIterable | AsyncIterator) to a single
iterator, drives it via an explicit next() loop instead of for-await,
and stores both in _activeSubscriptions. handleSubscriptionStopMessage
and dispose() now abort *and* call iterator.return(); return()
propagates through the procedure's async generator into any inner
for-await over an event sink, which settles the parked promise and
ends the streaming task naturally.
README 'Can I cancel a long-running query or subscription?' now states
the real lifecycle (abort + iterator.return propagation). The event-sink
guidance is updated: producers should still call sink.close() when their
own lifetime ends, but the framework no longer depends on it for
unsubscribe cleanup.
Tests: four new TypedEventSink tests for return() — releases a parked
next(), completes a for-await on break, drops buffered events on
return(), and idempotency. Full suite 1952 passing.
Refs PR #676 review (MEDIUM-1).
Collaborator
Author
Fix: HIGH-1 — Bundle scripts now always rebuild workspace packages firstUntil now only Fix: added
Verified by deleting Commit: 5e67c14 |
Collaborator
Author
Fix: MEDIUM-1 — Subscriptions now release iterators parked on next()
Fix (two parts):
README "Can I cancel a long-running query or subscription?" now states the real lifecycle (abort + iterator.return propagation). The event-sink guidance is updated: producers should still call Tests: four new I'll file a follow-up on Commit: d21d55b |
5 tasks
The package-lock.json regeneration in 00f3664 was done with npm 10.9.4, which resolves @swc/cli's optional peer dependency on chokidar@^5.0.0 differently than npm 10.9.3. The result was a lockfile that npm install accepts on either version, but npm ci on 10.9.3 (which is what CI uses, bundled with Node 22.18) rejects with: Invalid: lock file's chokidar@3.6.0 does not satisfy chokidar@5.0.0 Missing: chokidar@3.6.0 from lock file Restore the pre-00f36644 lockfile, which has the chokidar@5.0.0 entry at the root marked as optional/peer (resolved from @swc/cli) and a nested chokidar@3.6.0 under @vscode/test-cli. This is the shape npm 10.9.3 expects. The stale 'extraneous: true' entry for the renamed packages/vscode-webview-api workspace returns as a side effect; it is cosmetic and does not affect install behaviour. A clean regen requires running npm install with the same npm version CI uses, which is npm 10.9.3 bundled with Node 22.18.0 (per .nvmrc). Verified with Node 22.18.0 / npm 10.9.3: - npm ci passes (1792 packages) - npm run build passes - npm run lint clean - npm run prettier-fix no changes - npx jest --no-coverage: 96 suites / 1952 tests passing
Collaborator
Author
Fix: CI npm ci failure caused by 00f3664 lockfile regenThe CI build for this PR started failing after commit 00f3664 (the LOW-3 lockfile cleanup) with: Root cause: I regenerated the lockfile locally with
Fix (commit 28f38a2): restore the pre-00f36644 lockfile. That lockfile has the Lesson: when regenerating fnm use 22.18.0 # or: nvm use
rm -rf node_modules package-lock.json
npm installVerified locally with Node 22.18.0 / npm 10.9.3:
|
sajeetharan
approved these changes
May 26, 2026
Contributor
✅ Code Quality Checks
This comment is updated automatically on each push. |
Contributor
📦 Build Size Report
Download artifact · updated automatically on each push. |
github-project-automation
Bot
moved this to Done
in DocumentDB for VSCode: Release Plan
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Preview hardening for
@microsoft/vscode-ext-react-webviewplus a reshape of the consumer-side integration layer insrc/webviews/. Both halves land together so the package's preview surface and the in-tree consumer that exercises it stay in lock step.Branch:
dev/tnaum/webview-api-package(34 commits, all atomic).What changed
Package (
packages/vscode-ext-react-webview/)TypedEventSink<T>under/server: typed async-iterable for bridging push-style domain events into tRPC subscriptions (7-test suite).errorLinkunder the default entry plus an optionalonErrorargument onuseTrpcClient: opt-in webview-side error observer for queries and mutations (6-test suite). Subscriptions intentionally skipped so errors are not double-reported.WebviewControllernow routes everypostMessagecall through a privatesafePostMessagehelper that guards against disposed panels (both synchronous throws and late Thenable rejections).Package hardening (added during review)
TypedEventSinkiterator now implementsreturn(): marks the sink closed, drops buffered events, resolves a pendingnext()with{ done: true }, and releases the single-consumer guard.breakinside afor awaitover a sink now releases the parked promise per the JS iterator protocol with no producer cooperation.WebviewControllertracks both theAbortControllerand the liveAsyncIteratorper subscription.handleSubscriptionStopMessageanddispose()now abort and calliterator.return(), propagating cleanup through the procedure's async generator into any innerfor await._onDisposedemitter registered in_disposablesso it is torn down on panel dispose._activeSubscriptions.setdeferred until after the iterator is in hand; early failures fall through to the outer catch without inserting a stale entry.@vscode/l10nfrompeerDependencies, the README peer-dep table, and the package surface.pre*hooks added so every bundle/watch script runsnpm run build --workspaces --if-presentfirst.Consumer (
src/webviews/_integration/, wassrc/webviews/api/)_integration/so it sorts above feature folders in the explorer (the conventional "infrastructure, not feature code" signal).trpc.tsmerged intoappRouter.ts(5 files -> 4).BaseRouterContextnow intersects the framework type (no more duplicatedtelemetry?/signal?).WebviewControllerrenamed toWebviewControllerBaseto stop shadowing the framework class.configuration.tsholds the consumer-owned knobs (telemetry namespace, bundle layout, dev-server host). Three telemetry prefixes derived from one constant.webview-trpc-messaging,react-webview-architecture) updated to the new shape.Final consumer shape:
Verification
npm run l10nno-op.npm run prettier-fixno rewrites.npm run lintclean (only the pre-existingwebpack.config.views.jsESLint env warning).npx jest --no-coverage96 suites, 1952 tests, 4 snapshots green.npm run buildclean across all workspaces.Why this matters (and what was decided against)
Full rationale, alternatives considered, and decisions taken (folder naming, file boundaries, package scope, what we did not adopt from the partner extension's tRPC migration) are tracked in
docs/ai-and-plans/PRs/676-webview-api-package-preview-hardening.mdin this PR.Highlights:
TypedEventSink,errorLink,safePostMessage, README) are all pure transport plumbing or composable utilities; none import a UI library._integration/chosen overwebviewIntegration/. Sort order + brevity won; documented underscore-prefix convention.TypedEventSinkpattern was adopted (it is the migration carrot when they move to this package); their centralised routers/schemas, generic command-dispatch, and mutable-state-in-ctx patterns were not.Follow-up (out of scope for this PR)
0.8.0-preview.commonRouter.displayErrorMessageandopenUrltopublicProcedureWithTelemetryfor baseline rpc traces. Skipped here to keep PR scope tight.Commits
(Plus 9 pre-existing commits on the branch that built up the package extraction itself: scope tightening, README, helper re-homing, package rename. See
git log 4888c489^..a80e1843.)