Skip to content

refactor(webview): render webview documents from TSX views - #325

Merged
eFAILution merged 2 commits into
eFAILution:betafrom
X-Guardian:refactor/webview-tsx-views
Oct 6, 2026
Merged

eFAILution merged 2 commits into
eFAILution:betafrom
X-Guardian:refactor/webview-tsx-views

Conversation

@X-Guardian

@X-Guardian X-Guardian commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Moves every webview document out of template literals into TSX views, rendered to a string with preact-render-to-string. Now that the webview CSS and JS are external, this was the last markup that tsc and eslint saw as an opaque string.
  • The compiler now rejects unclosed and stray tags, duplicate attributes, unknown elements and string event handlers. Editors highlight the markup, and view props are typed.
  • Interpolated text and attribute values are escaped by default. The ~40 manual escapeHtml calls are gone. Raw HTML is allowed only in two helpers, which eslint enforces.
  • webviewBuilderMarkup.test.ts now renders every view instead of scanning the provider's source. It also adds a check that nothing else could do: every data-action in the markup must have a handler in its client script.
  • Link related issue(s): follows refactor(webview): move the Component Browser view's inline script, style and handlers to linted files #288

Advantages

Before (template literals) After (TSX views)
Markup errors Invisible to tsc and eslint. A duplicate class shipped in the fatal-error view, and only a purpose-built regex added in #310 caught it, because HTML5 silently drops the second one. A stray </head> renders fine in a browser, so nothing ever flagged it. Compile errors: unclosed or mismatched tags, stray close tags, duplicate attributes, unknown elements, and a string where an event handler belongs.
Escaping Opt-in, one call at a time. Each of the ~40 escapeHtml calls had to be remembered, and a missed one is an XSS hole fed by publisher-controlled names, tags and descriptions. Opt-out. Every interpolation is escaped. Raw HTML means reaching for dangerouslySetInnerHTML, which lint allows only in the two helpers that emit pre-sanitised content.
CSP faults A regex test over the provider's source text caught inline scripts, styles and handlers. It could not tell a builder from any other method, and it matched text, not the rendered document. Lint rejects style=, on* handlers, and inline <script>/<style> where they are written. The tests check the rendered output, which is what the webview actually loads.
Dead buttons Nothing caught a data-action with no handler: it's valid markup and a silent no-op, found only by clicking it. A unit test fails for any data-action its client script does not handle.
Types Interpolated values were type-checked, but the markup around them was not: element names, attribute names and attribute values were all free text. Element names, and the value types of known attributes, are checked against preact's DOM typings. Unknown attribute names still pass. Each view declares its props, which separates data preparation (in the builder) from markup (in the view).
Editor support The markup was one long string: no highlighting, no completion, no go-to-definition. JSX highlighting, completion for elements and attributes, and navigation into the helper components, all with no editor extension.
Testability The builders are methods on a class that imports vscode, so the unit suite could not call them. The only coverage was text-matching the source. The views and renderDocument don't depend on vscode, so the unit suite renders each view directly from fixtures.
Structure Every builder repeated the head, the CSP meta tag, the stylesheet link and the script tag. Loops were .map(...).join('') over nested template strings. Page holds the shell once. Repeated parts are small named components (SourceSection, ComponentCard, Parameter, VersionOptions), each with JSDoc. The provider shrinks by 377 lines.

Change Type

  • feat
  • fix
  • refactor
  • docs
  • test
  • chore

Context

User-facing impact

  • None intended. Before and after output for every view differs only in ways that leave the DOM unchanged (see Validation).

GitLab scope

  • gitlab.com
  • self-managed GitLab
  • both (webview rendering only)

Affected areas

  • Component Browser
  • Hover provider (the detached details panel it opens)
  • Completion provider
  • Validation provider
  • Cache and refresh behavior
  • GitLab API calls/auth/token storage
  • Docs only

What changed by bucket

Toolchain

tsconfig.json, package.json, package-lock.json

  • jsx: react-jsx with jsxImportSource: preact. esbuild reads both from tsconfig.json, so esbuild.js is unchanged.
  • preact and preact-render-to-string are dev dependencies like everything else here. They're bundled into out/extension.js, and node_modules isn't packaged.
  • Preact was chosen over @kitajs/html because it escapes children by default. Kitajs escapes only children marked safe.

Rendering

src/webview/render.ts

  • renderDocument(View, props) puts the doctype in front, since JSX can't express one, and returns the string webview.html expects.
  • It has no dependency on vscode, so the unit suite calls it directly.

Shared shell

src/webview/views/Page.tsx

  • Page renders the head (charset, viewport, CSP, stylesheet, title), the view's body, and the nonce'd client script.
  • JsonScript emits the bootstrap <script type="application/json"> through dangerouslySetInnerHTML. Browsers don't decode entities inside a script, so normal escaping would corrupt the JSON, and serializeForScript already makes it safe to emit verbatim.
  • InlineMarkdown does the same for renderInlineMarkdown output.

Views

src/webview/views/ — LoadingView, NoSourcesView, ErrorsView, ErrorView, ComponentBrowserView, ComponentDetailsView

  • One view per former builder. Every id, class and data-* attribute the client scripts use is unchanged.
  • Where JSX would drop a space between inline elements outside a flex container, an explicit {' '} puts it back.
  • The example JSON on the no-sources page is a string literal, so its <pre> keeps its indentation.

Builders

src/providers/componentBrowserProvider.ts

  • Each get…Html method still prepares its data: version data, classifySourceError summaries, URL validation, and the template-file URL. It then calls renderDocument.
  • renderVersionOptions moves into the browser view. The private escapeHtml and renderInlineMarkdown wrappers are removed.
  • 1,663 → 1,286 lines.

CSP

src/webview/csp.ts, src/webview/webviewHtml.ts, tests/unit/csp.test.ts

  • cspPolicy() returns the policy string that Page sets on its meta tag.
  • cspMetaTag has no callers left and is removed. Its tests now target cspPolicy.

Lint

eslint.config.js

  • .tsx joins the TypeScript block.
  • For src/webview/views/**, no-restricted-syntax bans:
    • style=, the one CSP fault the compiler can't see
    • on* attributes
    • inline <script> and <style>
    • dangerouslySetInnerHTML outside Page.tsx

Tests

tests/unit/webviewBuilderMarkup.test.ts, tests/unit/loadingView.test.ts

  • Every view is rendered from fixtures that include hostile publisher text. The tests assert:
    • there's no inline code the CSP would block;
    • every script carries the CSP nonce;
    • publisher text stays inert;
    • the bootstrap JSON round-trips, even with </script> in the data;
    • no metadata URL reaches an href;
    • every data-action has a handler.
  • The duplicate-attribute test and NOT_YET_EXTRACTED go. The compiler now rejects duplicate attributes, and NOT_YET_EXTRACTED has nothing left to track.
  • Every tag-matching regex is case-insensitive, so <SCRIPT> or <Style> can't slip past a check. CodeQL flagged two of them as "Bad HTML filtering regexp", and the other four had the same gap.

Validation

Local checks

  • npm run compile
  • npm test: 525 passing. npm run lint is clean (eslint and stylelint).
  • Extension-host suite: 25 passing
  • Manual verification in VS Code Extension Host: the loading view renders styled, with no CSP violations in the webview console
  • Manual verification of the remaining views: outstanding

Equivalence check

The move is meant to be mechanical, so I checked it directly rather than rely on review alone. Every builder was rendered from beta and from this branch with the same fixtures, including hostile names, tags and descriptions, using a stubbed vscode. Both outputs were then normalised (entities decoded, whitespace collapsed) and diffed. What remains is DOM-equivalent:

  • class="" and data-description="" now render as bare attributes, which have the same empty value.
  • Attribute order changed, and so did the order of <link> and <title> in the head.
  • The old <option value="…" > had a stray space inside the tag.

A second pass compared whitespace between adjacent inline elements. The spaces JSX drops all sit inside flex containers with gap: the header buttons, the source and project header spans, the card actions and title, the version control row, and the checkbox and label pairs. Flex layout ignores that whitespace.

Checks that were shown to fire

Each was tested by adding a deliberate fault and then reverting it:

  • A deleted <head> and a misspelt <butto> fail compilation.
  • A duplicate class fails with TS17001, and onClick="…" as a string is a type error.
  • style=, onclick=, dangerouslySetInnerHTML, inline <script> and inline <style> each fail lint.
  • A data-action misspelt as viewDetials fails with no handler for: viewDetials.

Manual test notes

  • In the Extension Development Host, with Developer: Open Webview Developer Tools open for CSP violations:
    • Component Browser. Test Load Versions → dropdown → switch version → Details and Insert. Also test search, expanding and collapsing a source and a project, Refresh, Update Cache, Reset Cache, the version dropdown's context menu, and a failing source's error banner with Show Details.
    • Details panel. Open it from both the browser's Details button and a hover's View Full Component Details. Then switch version, toggle raw YAML, tick individual inputs and Select All, use Refresh Versions, and Insert Component.
    • No sources, errors and error views. Test Open Settings, Try Again, Update Token and Show details.

Breaking Changes

  • No breaking changes
  • Breaking changes (describe below)

Risk and Rollback

  • Main risks: low to medium. The equivalence check covers the rendered output, and the handler check covers the most common silent fault. What neither covers is layout: a dropped space in a container that is not flex would be visible but not caught. The second equivalence pass found none.
  • dangerouslySetInnerHTML is confined to JsonScript and InlineMarkdown by lint. Both emit values that serializeForScript and renderInlineMarkdown have already made safe, and both helpers have their own tests.
  • The bundle grows by the size of preact and its string renderer, which are small.
  • Rollback strategy: revert the commit.

Release Notes Draft

  • Internal: webview markup is now written as type-checked TSX views, escaped by default.

Checklist

  • Branch is up to date with target branch
  • Commit messages follow conventional commits
  • Added/updated docs for behavior or settings changes (this PR description)
  • Added/updated tests for new behavior (rendered-output tests for every view, including the data-action handler check)
  • No secrets or tokens in code, logs, screenshots, or test fixtures

🤖 Generated with Claude Code

Comment thread tests/unit/loadingView.test.ts Fixed
Comment thread tests/unit/webviewBuilderMarkup.test.ts Fixed
@eFAILution
eFAILution deleted the branch eFAILution:beta October 4, 2026 14:52
@eFAILution eFAILution closed this Oct 4, 2026
@eFAILution eFAILution reopened this Oct 4, 2026
@X-Guardian
X-Guardian force-pushed the refactor/webview-tsx-views branch from bef9d63 to c426d41 Compare October 6, 2026 10:08

@eFAILution eFAILution left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ran through the manual test notes in a real VS Code with the packaged build. Browser, details panel from both entry points, no-sources and errors views all render and every button works, no CSP violations. Thanks Simon!

@eFAILution
eFAILution marked this pull request as ready for review October 6, 2026 14:09
@eFAILution

Copy link
Copy Markdown
Owner

Ran the manual test notes against the packaged build in a real VS Code. The browser, the details panel (from both the Details button and the hover link), and the no-sources and errors views all render with their stylesheets. Every control I tried works: search, expand/collapse, the error banner details, the version dropdown and its context menu, Insert, Select All, Refresh Versions, switching versions, Refresh, Update Cache and Reset Cache. No CSP violations in the webview console. I also injected an inline script to confirm the check would catch one, and it did.

A few things I couldn't reach: Update Token (couldn't get GitLab to return an auth error), the single fatal error view (same stylesheet and script as the errors view, which passed), Load Versions (every card already had versions), and the raw YAML toggle (sast has none). The unit tests cover their markup, so I'm not worried about them.

Two things I noticed that aren't from this PR. Insert from the details panel drops the component onto the current line when the cursor sits at the end of an existing component line. And a source with an unreachable host doesn't show up on the errors view at all. Probably worth opening issues for both.

@eFAILution
eFAILution merged commit 79f0cf8 into eFAILution:beta Oct 6, 2026
23 checks passed
@X-Guardian
X-Guardian deleted the refactor/webview-tsx-views branch October 6, 2026 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants