refactor: react porting#2
Open
pmarchini wants to merge 2 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors the VS Code “main” webview UI toward a bundled React + hydration approach, introduces a typed intent/message contract for the main view, and updates packaging/build plumbing to ship dist/ assets with the extension.
Changes:
- Replace the legacy string-template + inline click bridge with React SSR markup plus an optional bundled runtime bootstrap.
- Add a canonical
main-viewmessage contract and update providers/adapters/tests to use it. - Add an esbuild-based
build:vscode-assetspipeline and adjust extension packaging to include builtdist/output.
Reviewed changes
Copilot reviewed 41 out of 42 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Includes scripts/**/*.ts in TypeScript checking to cover new build tooling. |
| test/infra/vscode/ui/main-view/main-view-provider.test.ts | Updates main view provider tests for the new typed contract and bundled asset setup. |
| test/infra/vscode/ui/main-view/main-view-contract.test.ts | Adds tests for canonical message types/builders and message type guard. |
| test/infra/vscode/ui/main-view/main-view-adapter.test.ts | Reworks adapter tests to snapshots + adapter error handling + bundled bootstrap behavior. |
| test/infra/vscode/ui/main-view/snapshots/main-view-adapter/setup.route.html | Updates formatted HTML snapshot for setup route SSR output. |
| test/infra/vscode/ui/main-view/snapshots/main-view-adapter/setup-loading.route.html | Updates formatted HTML snapshot for setup-loading SSR output. |
| test/infra/vscode/ui/main-view/snapshots/main-view-adapter/path.route.html | Updates formatted HTML snapshot for path route SSR output. |
| test/infra/vscode/ui/main-view/snapshots/main-view-adapter/home.route.html | Updates formatted HTML snapshot for home route SSR output. |
| test/infra/vscode/ui/main-view-react/render-smoke.test.ts | Adds a basic React render smoke test for the isolated DOM harness. |
| test/infra/vscode/ui/main-view-react/main-view-browser.test.ts | Adds hydration/runtime wiring test to ensure clicks emit existing intents. |
| test/infra/vscode/ui/main-view-react/main-view-app.test.ts | Adds React component-level interaction test for message emission. |
| test/infra/vscode/ui/main-view-react/dom-test-environment.ts | Introduces a JSDOM-based DOM test environment installer. |
| test/infra/vscode/ui/legacy/navigator/navigator-view-provider.test.ts | Removes legacy navigator provider tests (legacy UI surface cleanup). |
| test/infra/vscode/ui/legacy/navigator/navigator-view-adapter.test.ts | Removes legacy navigator adapter tests (legacy UI surface cleanup). |
| test/infra/vscode/bindings/create-vscode-extension-host.test.ts | Extends host tests for webview asset URI helpers + accessor-backed webviews. |
| test/infra/package-manifest.test.ts | Adds assertions ensuring packaged output uses dist/ and ignores src/. |
| test/infra/build-vscode-assets.test.ts | Adds test coverage for the new esbuild asset bundling output. |
| src/infra/vscode/vscode-api.ts | Extends webview typing with optional asset helper hooks. |
| src/infra/vscode/ui/main-view/routes/setup-route.ts | Adds React route component for Setup view. |
| src/infra/vscode/ui/main-view/routes/path-route.ts | Adds React route component for Path view. |
| src/infra/vscode/ui/main-view/routes/home-route.ts | Adds React route component for Home view. |
| src/infra/vscode/ui/main-view/react-helpers.ts | Adds h alias for React.createElement to reduce JSX-free boilerplate. |
| src/infra/vscode/ui/main-view/modals/setup-install-confirmation.ts | Adds React modal for setup install confirmations. |
| src/infra/vscode/ui/main-view/modals/path-delete-confirmation.ts | Adds React modal for path delete confirmations. |
| src/infra/vscode/ui/main-view/main-view.css | Adds a minimal CSS entry for bundling verification. |
| src/infra/vscode/ui/main-view/main-view-provider.ts | Adds asset resolution (URIs + roots) and switches provider typing to VscodeWebviewView. |
| src/infra/vscode/ui/main-view/main-view-contract.ts | Introduces canonical main-view message types/builders/type guards and view model types. |
| src/infra/vscode/ui/main-view/main-view-browser.ts | Adds bundled browser runtime hydration entrypoint. |
| src/infra/vscode/ui/main-view/main-view-app.ts | Adds React root app composition (shell + route + modals). |
| src/infra/vscode/ui/main-view/main-view-adapter.ts | Switches rendering to React SSR + optional bootstrap payload and typed message dispatching. |
| src/infra/vscode/ui/main-view/components/faro-logo.ts | Extracts the logo to a React component. |
| src/infra/vscode/ui/main-view/components/app-shell.ts | Adds React shell/nav component that emits existing intents. |
| src/infra/vscode/ui/main-view/components/action-props.ts | Centralizes data-action attributes + optional interactive message posting. |
| src/infra/vscode/ui/legacy/navigator/navigator-view-provider.ts | Removes legacy navigator provider implementation. |
| src/infra/vscode/ui/legacy/navigator/navigator-view-adapter.ts | Removes legacy navigator adapter implementation. |
| src/infra/vscode/composition/register-vscode-bindings.ts | Wires extensionPath through to the main provider for asset resolution. |
| src/infra/vscode/bindings/create-vscode-extension-host.ts | Adapts webview APIs to expose resolveWebviewUri + setLocalResourceRoots. |
| src/infra/vscode/README.md | Updates docs to reflect removal of legacy UI surfaces. |
| scripts/build-vscode-assets.ts | Adds esbuild pipeline to generate dist/extension and dist/webviews/main assets. |
| package.json | Switches main to bundled output and adds build + React/testing deps. |
| Makefile | Adds build target and makes dev-host depend on it. |
| .vscodeignore | Updates ignore rules to package dist/ while excluding src/, scripts/, etc. |
| esbuild.build({ | ||
| entryPoints: [webviewEntry], | ||
| bundle: true, | ||
| format: "iife", |
There was a problem hiding this comment.
The webview bundle is built with esbuild format: "iife", but the HTML loader uses <script type="module" ...>. This mismatch can cause subtle runtime differences (e.g., top-level this / scoping) and makes the build intent unclear. Consider switching the bundle output to ESM (format: "esm") to match type="module", or change the script tag to a classic script if you want an IIFE bundle.
Suggested change
| format: "iife", | |
| format: "esm", |
Comment on lines
+1
to
+2
| // @ts-expect-error jsdom is available at runtime but is untyped in this repo. | ||
| import { JSDOM } from "jsdom"; |
There was a problem hiding this comment.
Using // @ts-expect-error to import jsdom will start failing once typings are added (the directive becomes an error) and it hides useful type information for the DOM harness. Since jsdom is now a dependency, consider adding @types/jsdom and removing the suppression so this helper is type-checked normally.
Owner
Author
|
019d1c98-2200-7fd3-9704-c95adfa1cbd9 |
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.
No description provided.