feat(core): share one HTML shell between --reporter html and the live dashboard - #231
Merged
Merged
Conversation
… dashboard
The 2026-07-09 vite-ui redesign explicitly accepted that the dashboard
and the CLI's --reporter html would diverge ("Tradeoff, stated
explicitly"), and they did: the dashboard gained the master/detail
layout, search/sort, dark mode, the wordmark, and the AI Prompt while
the static report kept the old single-page design.
This reinstates the original shared-renderer architecture, but with the
dashboard's UI as the shared one. The shell (style + client script +
renderer + snapshot type) moves into core as reporter/app-shell.ts:
- renderAppShell(snapshot) emits the same document for both surfaces;
snapshot.live gates the SSE wiring, /data.json refetch, and the
connection/analyzing indicators, and picks the <title>.
- buildHtmlDocument/formatHtmlReport keep their public signatures and
now emit the shared shell with live: false (plus a badge clamp and
server-side docsUrl sanitizing, since the client renders hrefs from
the embedded snapshot).
- packages/vite deletes dashboard{,-script,-style}.ts and serves
core's shell with live: true; buildSnapshot keeps its own docsUrl
sanitizing because /data.json responses bypass renderAppShell.
- Core html tests rewritten against the embedded-snapshot contract;
a new vite jsdom test pins the static mode (no EventSource, no
connection dot, AI Prompt still present).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (15)
💤 Files with no reviewable changes (2)
📝 WalkthroughWalkthroughThe static HTML reporter and Vite live dashboard now use a shared ChangesShared app shell
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Reporter
participant CoreAppShell
participant ViteMiddleware
participant Browser
Reporter->>CoreAppShell: buildHtmlDocument(report, meta)
CoreAppShell->>Browser: self-contained static HTML
ViteMiddleware->>CoreAppShell: renderAppShell(live snapshot)
CoreAppShell->>Browser: live dashboard HTML
Browser->>ViteMiddleware: EventSource updates when snapshot.live is true
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR re-centralizes the HTML UI rendering into @svelte-vitals/core so the CLI’s static --reporter html output and the Vite live dashboard share the same app shell and can’t drift in features/behavior.
Changes:
- Introduces a new shared core renderer (
renderAppShell) with embedded snapshot + inline style/script; static mode gates SSE/refetch/connection UI. - Updates
@svelte-vitals/viteUI middleware and tests to use the shared core shell, deleting the Vite-local dashboard shell/style implementation. - Updates HTML reporter tests and docs to reflect the shared-shell contract and static-vs-live parity expectations.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/vite/test/ui-dashboard.test.ts | Switches dashboard shell rendering to core renderAppShell; updates snapshot fixture with live: true. |
| packages/vite/test/dashboard-script-staleness.test.ts | Uses core APP_SCRIPT; updates embedded snapshot fixture to include live: true. |
| packages/vite/test/dashboard-script-ai-prompt.test.ts | Uses core APP_SCRIPT; updates embedded snapshot fixture to include live: true. |
| packages/vite/test/app-shell-static.test.ts | Adds jsdom test ensuring static HTML report boots the shared shell without creating EventSource. |
| packages/vite/src/ui/snapshot.ts | Aligns dashboard snapshot type to core AppSnapshot and ensures live snapshots set live: true. |
| packages/vite/src/ui/middleware.ts | Serves the core renderAppShell output instead of the Vite-local dashboard renderer. |
| packages/vite/src/ui/dashboard.ts | Deletes the Vite-local dashboard HTML shell renderer. |
| packages/vite/src/ui/dashboard-style.ts | Deletes the Vite-local dashboard CSS (now sourced from core). |
| packages/core/test/html-report.test.ts | Rewrites HTML report tests around the embedded snapshot + shared-shell parity guarantees. |
| packages/core/src/reporter/html.ts | Keeps HTML primitives (escapeHtml, safeHref, etc.) and re-exports report entry points from app-shell. |
| packages/core/src/reporter/app-shell.ts | Adds the shared app shell renderer, snapshot type, and static html reporter implementation. |
| packages/core/src/index.ts | Exposes new shared-shell exports (renderAppShell, AppSnapshot, APP_SCRIPT, APP_STYLE, etc.). |
| docs/src/content/docs/guides/reporters.md | Updates English docs to describe the shared UI between report and dashboard. |
| docs/src/content/docs/ja/guides/reporters.md | Updates Japanese docs to describe the shared UI between report and dashboard. |
| .changeset/shared-html-report-shell.md | Adds changeset for core/cli/vite minor release reflecting the shared renderer change. |
Comments suppressed due to low confidence (2)
packages/core/src/reporter/app-shell.ts:711
renderAppShellsanitizes the embedded report (docsUrl) but does not clampsnapshot.badges. SincerenderAppShellis now a public API and the client script uses badge values to build class names (dv-badge-${badge}) and visible text, an unexpected string can leak into the DOM and styling. Consider filtering badges to the known set (measured|static) here too, not only inbuildHtmlDocument.
packages/core/src/reporter/app-shell.ts:10app-shell.tsimportssafeHreffrom./html.js, buthtml.tsre-exportsbuildHtmlDocument/formatHtmlReportfrom./app-shell.js, creating a circular dependency (app-shell→html→app-shell). ESM often tolerates this, but it’s fragile for tooling/bundlers and makes future refactors riskier. Consider movingsafeHref(and any shared helpers) into a small third module (e.g.html-utils.ts) that bothhtml.tsandapp-shell.tsimport, so the dependency graph stays acyclic.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| siteIssues: [] | ||
| }; | ||
| const html = buildHtmlDocument(evil, { version: '0' }); | ||
| // embedJson escapes `<` to <, so the raw tag never exists in the document. |
Copilot review on PR #231: embedJson replaces `<` with the literal escape sequence < (the parsed JSON still contains the original string), not with an HTML entity — say so.
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
svelte-vitals --reporter htmlhad drifted from the live dashboard: the 2026-07-09 vite-ui redesign explicitly accepted the divergence ("Tradeoff, stated explicitly" indocs/superpowers/specs/2026-07-09-vite-ui-vitest-parity-design.md), and since then the dashboard gained the master/detail layout, search/sort, dark mode, the wordmark brand, and the per-finding AI Prompt while the static report kept the old single-page design.This reinstates the original shared-renderer architecture — one renderer behind both surfaces — with the dashboard's UI as the shared one:
packages/core/src/reporter/app-shell.ts(new; assembled from the vite dashboard's style/script/renderer, not rewritten):renderAppShell(snapshot)emits the same document for both surfaces.snapshot.livegates the SSE wiring, the/data.jsonrefetch, and the connection/analyzing indicators, and picks the<title>(svelte-vitals dashboardvssvelte-vitals report).buildHtmlDocument/formatHtmlReportkeep their public signatures and now emit the shared shell withlive: false— plus a badge clamp and server-sidedocsUrlsanitizing, since the client script renders hrefs straight from the embedded snapshot andbuildHtmlDocumentis a public API fed loosely-typed reports.packages/vitedeletesdashboard.ts/dashboard-script.ts/dashboard-style.ts(−583 lines) and serves core's shell withlive: true.buildSnapshotkeeps its own docsUrl sanitizing because/data.jsonresponses bypassrenderAppShell.Test plan
html-report.test.tsrewritten against the embedded-snapshot contract, porting every hardening guarantee (script-breakout via</script>,javascript:docsUrl stripped server-side, attacker-controlled category keys, badge clamping) plus a new byte-level parity test:live: truevslive: falsedocuments are identical except the title and the flagpackages/vite/test/app-shell-static.test.tsboots the real static document in jsdom and pins the static contract: full layout renders, no EventSource is ever constructed, no connection/analyzing indicator, AI Prompt still present--reporter htmloutput starts with the new shell,"live":falseembedded,dv-appcontainers present.d.tssize kept in check (APP_STYLE/APP_SCRIPTannotated: stringso the giant template literals don't become literal types)Note
packages/action/distturned out not to need a rebuild: the action never imports the HTML reporter, so esbuild tree-shakes the shell out of its bundle entirely (verified by rebuilding — byte-identical output), and the cli's shared chunk hash is unchanged because core is an external dependency of the cli bundle. CI's "Verify action dist is up to date" gate passes as-is.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation