refactor: redesign webview package as @microsoft/vscode-ext-webview by tnaum-ms · Pull Request #766 · microsoft/vscode-documentdb

tnaum-ms · 2026-06-30T14:32:19Z

Summary

Redesigns the webview RPC package: replaces @microsoft/vscode-ext-react-webview with a new @microsoft/vscode-ext-webview (0.9.0-preview) built around four side-aware subpaths and composable primitives, migrates the extension onto it, and removes the old package.

What changed

New package @microsoft/vscode-ext-webview with four subpaths:
- . (shared, no vscode/react), ./host, ./webview, ./react
New primitives: openWebview factory + options-bag WebviewController, attachTrpc (bring-your-own-panel), connectTrpc, initWebviewTrpc, and telemetry middleware bodies (telemetryMiddlewareBody / loggingMiddlewareBody) wired through a consumer-supplied TelemetryRunner adapter.
Hook split: useTrpcClient() returns the client directly; useRpcEvents() exposes the event channel.
Retired: createMiddleware, TelemetryContext, the old useTrpcClient tuple return, UseTrpcClientOptions.
Extension migration: the _integration layer and panel controllers moved onto the new package; construction-only panels now use an openAppWebview factory preset (the old WebviewControllerBase is gone).
Removed the deprecated @microsoft/vscode-ext-react-webview package and all references.
Docs: package README.md + ADVANCED.md, plus an internal migration manual.

Validation

Whole-repo lint clean, full Jest suite green (2571 tests / 146 suites), and npm run build green.

Draft for review.

….9.0-preview

…Sink, BaseRouterContext)

…act entry

…adapters

…roller

…annel)

…ntroller

…kage-redesign

…onfig references

…ain openAppWebview

…red, testing, and webview directories

…tions in README and migration manual

… [R766-N06] Adds an ADVANCED.md section showing the consumer-side Map<key, controller> + revealToForeground() + onDisposed() eviction pattern for single-instance panels, instead of building a panel registry into the transport package.

tnaum-ms · 2026-07-03T16:23:21Z

[R766-N06] — documented the create-or-reveal pattern (b603affd)

Per the review, this stays consumer scope (a panel registry is consumer state, not transport state). Added a "Create-or-reveal (single-instance panels)" section to ADVANCED.md showing the few-line pattern using the returned controller handle: a Map<key, controller>, revealToForeground() on a hit, and onDisposed(() => map.delete(key)) to evict. No package code — if multiple consumers converge on this, it becomes a candidate to promote later.

…-operation listener [R766-S04] attachTrpc stamps each dispatch log entry with 'concurrent' (in-flight ops at completion) via a logProcedure wrapper; ProcedureLogEntry gains an optional concurrent field. DocumentDB rpcConcurrencyLogger forwards it into callWithAccumulatingTelemetry as a concurrentRpcOps distribution + dispatch counter (batched), wired via openAppWebview. Turns S04's 'revisit on evidence' into a real signal. Tests on both sides.

tnaum-ms · 2026-07-03T16:31:01Z

[R766-S04] — the per-operation listener is now measured, not guessed (dbbf9969)

The design is unchanged (still one window message listener per in-flight operation), but the review's "revisit only on evidence" is now backed by a real signal so we can decide with data — or retire the concern if concurrency stays tiny.

Package: attachTrpc stamps every dispatch log entry with concurrent (in-flight ops at completion, which still includes the completing op) via a small logProcedure wrapper; ProcedureLogEntry gains an optional concurrent field. The standalone loggingMiddlewareBody omits it (no view of the transport's in-flight set).
Consumer: rpcConcurrencyLogger keeps the console line and forwards concurrent into callWithAccumulatingTelemetry('documentDB.webview.rpcConcurrency', …) as a concurrentRpcOps distribution + a dispatch counter; wired via openAppWebview. Batched (20 calls / 30 s), so per-RPC volume is negligible.
Emits: dist_concurrentRpcOps_max (peak — the number that matters), dist_concurrentRpcOps_sum/count (average), dispatch (per-window volume), and dist_auto_duration_ms_* (free latency).
Revisit thresholds (from the O(N) model, in the review doc): _max ≤ 8 in ~all sessions → keep / consider retiring; _max ≥ 32 in ≳1% of sessions or average ≥ 8 sustained → build the single-listener + Map<id, observer> multiplexer.

Tests on both sides (package: concurrent is stamped; consumer: gauge + counter forwarding).

…R766-S04]

… iteration-3 deferral

tnaum-ms · 2026-07-03T16:39:41Z

Iteration 2 — summary

Six review items shipped as individual commits (each pushed + commented above); one is deferred to Iteration 3.

Item	Commit	Outcome
R766-01 + R766-N01	`ade2ce61`	Transport listeners ignore foreign `postMessage` traffic (guard + throw-safe)
R766-N02	`21b0a2f7`	`trpc` instance option; standalone `createCallerFactory` deprecated
R766-N03	`d5748abe`	Initial data via an inert `application/json` block (break-out class removed)
R766-N06	`b603affd`	Create-or-reveal pattern documented in ADVANCED.md
R766-S04	`dbbf9969`	RPC-concurrency telemetry signal to judge the per-operation listener

Deferred to Iteration 3 — [R766-N05] (observer exceptions → telemetry): per the standing "options only" decision, not implemented. Two things are bundled and wait on one design decision — (1) the cheap correctness fix (isolate a throwing event-channel observer so it can't break the tRPC call it only observes), and (2) the telemetry-visibility hook (leaning: an opt-in onObserverError sink defaulting to console.error, optionally plus reportError). Iteration 3 should pick the hook shape and ship isolation + hook together with a "throwing observer does not break dispatch" test.

Validation (all green): l10n (no drift) · prettier · eslint --quiet (clean) · jest (2655 passed / 158 suites) · tsc build (clean).

Motivation and rationale for every change are recorded inline in the PR review doc (docs/ai-and-plans/PRs/766-webview-ext-package-redesign/webview-ext-review.md, "Iteration 2 — change protocol").

…erverError sink [R766-N05] createEventChannel always isolates a throwing onSuccess/onError/onAborted handler so it cannot break the tRPC call it observes, routing it to an onObserverError sink (default console.error). Threaded through connectTrpc + WithWebviewContext/hooks. Off by default in the happy path; DocumentDB opts in via reportObserverError (structured console.error + reportError() global elevation, no channel re-entry). README + ADVANCED document it; tests cover isolation, sink routing, default, and the DocumentDB sink.

…ion-2 protocol

tnaum-ms · 2026-07-04T22:15:03Z

[R766-N05] — event-channel observers are now isolated + an opt-in onObserverError sink (76227034)

Implements option 1 from the review discussion. Previously a throwing events.onSuccess / onError / onAborted observer propagated back into the tRPC link chain and broke the very call it was only observing.

Correctness fix (always on): createEventChannel wraps each handler call; a throw is caught (upholding the observer-only contract) and routed to an onObserverError sink. This can no longer break dispatch, and a throw from the sink itself is swallowed too.
Structured sink (opt-in): onObserverError(error, { info, phase }) — phase is success | error | aborted, info is the CallInfo. Threaded through connectTrpc({ onObserverError }) and <WithWebviewContext onObserverError={…}> / the hooks. Defaults to console.error.
Off by default in the happy path: the all-in path only gets the console.error default — no telemetry events a generic consumer might not want.
DocumentDB opts in for itself: reportObserverError keeps the structured console.error and elevates to the browser reportError() global (the "general observability" of option 5), without re-entering the tRPC channel — so a throwing observer can't cause a report loop.
Docs: README Observability gains a "Webview-side observer errors" subsection; ADVANCED.md documents the isolation + sink contract.
Tests: isolation (later handlers still run, emit doesn't throw), sink routing with { info, phase }, the console.error default, a throwing sink, and the DocumentDB sink.

Validation: l10n (no drift) · prettier · eslint --quiet (clean) · jest (2661 passed / 159 suites) · tsc build (clean). This closes the last open review item; nothing is deferred to a later iteration.

…observer-error sink [R766-N05] reportError is declared in lib.dom (the webview tsconfig includes 'dom'), so the '(globalThis as { reportError?: ... })' cast was unnecessary. Call globalThis.reportError directly behind the same typeof guard. No behavior change; the DOM reportError() global is unrelated to the app router's reportError tRPC mutation.

Copilot

Pull request overview

Copilot reviewed 92 out of 99 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

src/webviews/_integration/useTrpcClient.ts:20

useTrpcClient is a shared helper used across the webview codebase; giving it an explicit return type makes the public surface clearer and avoids accidental type widening if the framework hook signature changes.

…back as Iteration 4 Iteration 3: R766-N05 (event-observer isolation) was implemented after the iter-2 batch but recorded inconsistently (folded into iter-2, while the options section still said 'deferred to iter-3'). Give it its own chapter: what shipped (option 1 always-on isolation + opt-in onObserverError sink; DocumentDB opts in via reportObserverError), the change protocol (7622703 / 559a9af / f7c9656, jest 2661/159), the reportError DOM-global-vs-tRPC-mutation naming clarification, and the deferred host-telemetry route. Reconcile the iter-2 'Still open' table, protocol table, wrap-up, and N05 options blockquote to point at Iteration 3. Iteration 4: triage the GitHub Copilot reviewer's 2026-07-05 pass (review 4630986353) as R766-C01..C04 with detailed options and recommendations (analysis only, not yet implemented): C01 telemetry middleware stamps error fields on aborted calls; C02 the DocumentDB runner re-stamps them (pairs with C01); C03 tighten the webview onReceive guard to an own string id (refines R766-N01); C04 explicit return type on useTrpcClient (suppressed). Also brings the doc into prettier compliance (the committed version was failing prettier; tables are now column-aligned).

…PC calls [R766-C01][R766-C02] R766-C01 (package): telemetryMiddlewareBody gated the 'Failed' result on !aborted but still stamped error/errorMessage for an aborted call that surfaced as a rejected result, so a cancellation looked like a failure on the error dimension. Gate the whole error-recording block on !aborted (option A): an aborted call is now recorded only as 'Canceled' (aborted='true') with no error* fields. Also export getInvocationSignal from ./host so consumers can share the abort check. Adds a regression test (aborted invocation that also throws -> no error* fields). R766-C02 (documentdb): documentDbTelemetryRunner ran its parseError enrichment (error/errorMessage/errorStack/errorCause) for any !result.ok, on the same telemetry bag, after the body -- so it re-stamped error fields and silently undid C01 for canceled ops. It now reads getInvocationSignal(invocation.ctx)?.aborted and skips enrichment when aborted (option B). Shipped with C01 so both telemetry layers agree. Also marks R766-C01/C02 implemented in the review doc.

…onse guard [R766-C03] The webview onReceive guard added in R766-N01 forwarded any object with an `id` property, including an inherited (prototype) `id` and a non-string `id` -- looser than the host-side isTransportRequestMessage guard (which already checks typeof id === 'string'). Add a shared isTransportResponseMessage type guard to shared/wireProtocol.ts (non-null object with its OWN string `id`, via Object.hasOwn + typeof) and use it in connectTrpc's onReceive, restoring host/webview symmetry and a single source of truth for the response shape (option C). Extend the R766-N01 test with a numeric-id case and an inherited-id case (whose id equals the pending request id, so the old guard would have resolved the query with undefined before the real response arrived). Also marks R766-C03 implemented in the review doc.

…n type [R766-C04] The DocumentDB useTrpcClient wrapper had an inferred return type. Declare it explicitly as TrpcClient<AppRouter> (imported from @microsoft/vscode-ext-webview/react), which locks the public surface, guards against accidental type widening if the framework hook signature changes, and matches the repo TypeScript guideline of always specifying return types. This was the Copilot reviewer's low-confidence (self-suppressed) comment. Also marks R766-C04 implemented in the review doc.

All four Copilot findings are implemented; add the Iteration 4 change-protocol table (C01/C02 -> 553bf4e, C03 -> 3301a70, C04 -> 19b99cb) and flip the chapter intro from "nothing implemented yet" to implemented. Records the green validation run (l10n no drift, prettier, eslint, jest 2662/159, tsc build).

tnaum-ms · 2026-07-05T10:07:21Z

Iteration 4 — Copilot feedback addressed

All four findings from the 2026-07-05 Copilot review are implemented on dev/tnaum/webview-api-refinements:

ID	Fix	Commit
C01	`telemetryMiddlewareBody` no longer stamps `error` / `errorMessage` on aborted calls (the whole error-recording block is gated on the not-aborted path); `getInvocationSignal` is exported from `./host` for reuse	`553bf4e8`
C02	`documentDbTelemetryRunner` reads `getInvocationSignal` and skips its `parseError` enrichment when the call was aborted, so it no longer re-stamps error fields and undoes C01	`553bf4e8`
C03	shared `isTransportResponseMessage` guard (own string `id`) now backs `connectTrpc`'s `onReceive`, mirroring the host-side `isTransportRequestMessage`; the R766-N01 test was extended with numeric-id / inherited-id cases	`3301a709`
C04	the `useTrpcClient` wrapper return type is annotated as `TrpcClient<AppRouter>` (the suppressed low-confidence note)	`19b99cbf`

The three inline threads (C01 / C02 / C03) have been replied to and resolved. C04 was self-suppressed and had no thread, hence this summary note.

C01 and C02 shipped in one commit because C02's enrichment would otherwise undo C01.

Validation (all green): npm run l10n (no drift), prettier, eslint --quiet, jest (2662 passed / 159 suites), and tsc build across all workspaces. Full rationale + change protocol: docs/ai-and-plans/PRs/766-webview-ext-package-redesign/webview-ext-review.md (Iteration 4).

The Iteration 4 chapter ended on the "Suggested batching" recommendations. Add a closing "Iteration 4 — done" section that lists the steps performed (triage, Batch 1 C01+C02 553bf4e, Batch 2 C03 3301a70, Batch 3 C04 19b99cb, doc finalize cb9cd3b, green validation) and a clear completion message, and reword the batching close to past tense.

Copilot

Pull request overview

Copilot reviewed 91 out of 99 changed files in this pull request and generated no new comments.

…mics and onboarding

…ot extension mode The redesign chose the bundled-vs-dev source layout from extensionMode === Production, but the dev server (webpack serve) emits the bundled asset name (views.js). A webpack-bundled extension running in Development (the normal F5 dev flow, IS_BUNDLE=true, DEVSERVER=true) therefore requested the tsc dev file (index.js) from the dev server and got a 404, leaving the webview blank. Restore the pre-redesign contract: a required isBundled option drives the layout; extensionMode stays CSP-only. The DocumentDB consumer passes isBundled: git commit -q -F /tmp/done-msg.txt && git push -q origin HEAD && echo pushed && git --no-pager log -1 --format='%h %s'ext.isBundle. Adds regression tests.

…ubfolder Move rpcConcurrencyLogger (host) and reportObserverError (webview), plus their tests, into src/webviews/_integration/observability/ so the reference integration folder separates the router/transport wiring from the cross-cutting observability adapters. Update imports, the two external importers, the folder README, and add observability/README.md. No runtime behavior change.

…tion 7 (_integration observability regrouping)

github-actions · 2026-07-05T21:02:47Z

✅ Code Quality Checks

Check	Status	How to fix
Localization (`l10n`)	✅ Passed
ESLint	✅ Passed
Prettier formatting	✅ Passed

This comment is updated automatically on each push.

github-actions · 2026-07-05T21:06:00Z

📦 Build Size Report

Metric	Base (`main`)	PR	Delta
VSIX (`vscode-documentdb-0.9.1.vsix`)	8.01 MB	8.01 MB	⬆️ +2 KB (+0.0%)
Webview bundle (`views.js`)	5.88 MB	5.88 MB	⬆️ +2 KB (+0.0%)

Download artifact · updated automatically on each push.

tnaum-ms added 29 commits June 30, 2026 09:03

chore(webview-ext): scaffold vscode-ext-webview as a copy

9e5b787

chore(webview-ext): rename package to @microsoft/vscode-ext-webview@0…

2baed21

….9.0-preview

refactor(webview-ext): introduce shared entry (wire types, TypedEvent…

68d15fb

…Sink, BaseRouterContext)

refactor(webview-ext): introduce host entry

6d2e8f8

refactor(webview-ext): split framework-agnostic webview entry from re…

60e40f4

…act entry

chore(webview-ext): export ./host, ./webview, ./react subpaths

7516e10

feat(webview-ext): add initWebviewTrpc typed-init helper

f88b342

feat(webview-ext)!: replace bound telemetry with middleware bodies + …

f1d9481

…adapters

refactor(webview-ext): extract attachTrpc dispatcher from WebviewCont…

74f75d6

…roller

feat(webview-ext): add createEventChannel and refit errorLink as a shim

f2af464

feat(webview-ext): add connectTrpc webview client factory

e3e723c

feat(webview-ext)!: split useTrpcClient (client) and useRpcEvents (ch…

cca91f3

…annel)

style(webview-ext): apply prettier normalization to phase C files

56adb9f

feat(webview-ext)!: add openWebview factory and options-bag WebviewCo…

e11ad56

…ntroller

docs(webview-ext): quick-start README with advanced signpost

9ba9bcd

docs(webview-ext): add ADVANCED.md

8012f18

chore(documentdb): migrate _integration to @microsoft/vscode-ext-webview

237e9ed

chore(documentdb): point webview consumers at ./react and split hooks

0d7ecb3

style(documentdb): wrap trpc.ts host import per prettier

c73aa4d

chore(documentdb): depend on @microsoft/vscode-ext-webview

6fa0e4a

chore(documentdb): adopt openWebview factory for panel controllers

57c3136

chore: remove deprecated @microsoft/vscode-ext-react-webview package

309e2f0

docs: add internal webview-ext migration manual

4e989d9

docs: archive webview-ext planning docs under PRs/766-webview-ext-pac…

f576f92

…kage-redesign

fix(build): register vscode-ext-webview in root jest projects and tsc…

2294a51

…onfig references

docs(webview-ext): strengthen package description and README framing

cf1a2f4

refactor(documentdb): rename panel factories to open*Webview and expl…

70d43a3

…ain openAppWebview

feat: add detailed README files for src, host, middleware, react, sha…

a5af2e8

…red, testing, and webview directories

refactor(webview): enhance documentation and streamline naming conven…

0f54231

…tions in README and migration manual

tnaum-ms marked this pull request as ready for review July 2, 2026 21:56

tnaum-ms added 2 commits July 3, 2026 16:39

style(vscode-documentdb): prettier-fix rpcConcurrencyLogger imports […

36c8493

…R766-S04]

docs(pr-766): finalize iteration-2 change protocol, statuses, and N05…

953a895

… iteration-3 deferral

tnaum-ms added 2 commits July 4, 2026 22:12

docs(pr-766): record R766-N05 implementation (option 1) in the iterat…

559a9af

…ion-2 protocol

tnaum-ms requested a review from Copilot July 5, 2026 08:09

Copilot started reviewing on behalf of tnaum-ms July 5, 2026 08:09 View session

Copilot AI reviewed Jul 5, 2026

View reviewed changes

Comment thread packages/vscode-ext-webview/src/host/middleware/telemetry.ts Outdated

Comment thread src/webviews/_integration/trpc.ts

Comment thread packages/vscode-ext-webview/src/webview/connectTrpc.ts

tnaum-ms added 5 commits July 5, 2026 09:13

tnaum-ms requested a review from Copilot July 5, 2026 20:02

Copilot started reviewing on behalf of tnaum-ms July 5, 2026 20:03 View session

Copilot AI reviewed Jul 5, 2026

View reviewed changes

tnaum-ms added 4 commits July 5, 2026 20:16

docs(webview-ext-review): add Iteration 5 focusing on consumer ergono…

13ec19e

…mics and onboarding

docs(pr-766): record iteration 6 (R766-07 webview-load fix) and itera…

5478d2e

…tion 7 (_integration observability regrouping)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: redesign webview package as @microsoft/vscode-ext-webview#766

refactor: redesign webview package as @microsoft/vscode-ext-webview#766
tnaum-ms wants to merge 62 commits into
mainfrom
dev/tnaum/webview-api-refinements

tnaum-ms commented Jun 30, 2026

Uh oh!

tnaum-ms commented Jul 3, 2026

Uh oh!

tnaum-ms commented Jul 3, 2026

Uh oh!

tnaum-ms commented Jul 3, 2026

Uh oh!

tnaum-ms commented Jul 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tnaum-ms commented Jul 5, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

github-actions Bot commented Jul 5, 2026

Uh oh!

github-actions Bot commented Jul 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

tnaum-ms commented Jun 30, 2026

Summary

What changed

Validation

Uh oh!

tnaum-ms commented Jul 3, 2026

Uh oh!

tnaum-ms commented Jul 3, 2026

Uh oh!

tnaum-ms commented Jul 3, 2026

Iteration 2 — summary

Uh oh!

tnaum-ms commented Jul 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tnaum-ms commented Jul 5, 2026

Iteration 4 — Copilot feedback addressed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

github-actions Bot commented Jul 5, 2026

✅ Code Quality Checks

Uh oh!

github-actions Bot commented Jul 5, 2026

📦 Build Size Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants