From ae80f05fadea864746e9b40e89d0238f4a855de0 Mon Sep 17 00:00:00 2001 From: oekazuma Date: Wed, 12 Aug 2026 14:39:09 +0900 Subject: [PATCH 1/2] fix(vite): score crashed rules as not-run on the dev dashboard, matching CLI and build mode The dev dashboard's whole-project runner and live per-route ingest both discarded runRules' failedRules, so a crashed rule silently inflated Health there while the CLI and vite build path already excluded it via withFailedRulesOff. Thread the failure-adjusted config through both layers (static: analyzeProject's returned config via a closure the middleware reads per-request; live: failedRuleIds on the ingest payload, unioned in the store) and apply withFailedRulesOff where the snapshot is built. Also extracts formatFailedRuleWarning to core (replacing three copy-pasted "rule ... failed and was skipped" string literals, one of which had drifted and lost the first-line cap) and wraps the vite plugin/handle's warn sinks with terminalSafe. Co-Authored-By: Claude Fable 5 --- .changeset/dashboard-failed-rules.md | 8 ++ packages/cli/src/index.ts | 3 +- packages/core/src/config-apply.ts | 5 + packages/core/src/index.ts | 3 +- packages/core/test/config-apply.test.ts | 7 ++ packages/vite/src/analyze.ts | 3 +- packages/vite/src/hooks/handle.ts | 25 +++-- packages/vite/src/plugin.ts | 22 +++- packages/vite/src/ui/analysis.ts | 16 ++- packages/vite/src/ui/middleware.ts | 13 ++- packages/vite/src/ui/snapshot.ts | 24 ++++- packages/vite/src/ui/store.ts | 18 +++- packages/vite/test/dev-handle.test.ts | 64 +++++++++++ packages/vite/test/ui-analysis.test.ts | 21 ++++ packages/vite/test/ui-middleware.test.ts | 132 ++++++++++++++++++++++- packages/vite/test/ui-snapshot.test.ts | 38 ++++++- packages/vite/test/ui-store.test.ts | 35 ++++++ 17 files changed, 399 insertions(+), 38 deletions(-) create mode 100644 .changeset/dashboard-failed-rules.md diff --git a/.changeset/dashboard-failed-rules.md b/.changeset/dashboard-failed-rules.md new file mode 100644 index 000000000..b926e90b0 --- /dev/null +++ b/.changeset/dashboard-failed-rules.md @@ -0,0 +1,8 @@ +--- +'@svelte-vitals/core': minor +'@svelte-vitals/vite': patch +--- + +`@svelte-vitals/core` now exports `formatFailedRuleWarning`, the "rule … failed and was skipped" message formatter shared by the CLI, build mode, and (now) the dev dashboard. + +The dev dashboard now scores a crashed rule as not-run (matching the CLI and build mode) instead of silently inflating Health; plugin warnings strip terminal escape sequences. diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index ef5921e87..ce0d91fc4 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -19,6 +19,7 @@ import { applyOverrides, settingSeverity, withFailedRulesOff, + formatFailedRuleWarning, terminalSafe, type Severity, type RuleSetting, @@ -268,7 +269,7 @@ function skippedFileWarnings(facts: { file: string; parseFailed?: true }[]): str * as clean. Message capped to its first line so a multi-line stack trace can't flood the terminal. */ function failedRuleWarnings(failedRules: { id: string; message: string }[]): string[] { - return failedRules.map((f) => `rule ${f.id} failed and was skipped: ${f.message.split('\n')[0]}`); + return failedRules.map(formatFailedRuleWarning); } /** diff --git a/packages/core/src/config-apply.ts b/packages/core/src/config-apply.ts index 2e2539b75..16def8372 100644 --- a/packages/core/src/config-apply.ts +++ b/packages/core/src/config-apply.ts @@ -36,6 +36,11 @@ export function withFailedRulesOff(config: Config, failedRuleIds: readonly strin }; } +/** One-line "rule failed and was skipped" warning; capped to the message's first line so a stack trace can't flood a terminal. */ +export function formatFailedRuleWarning(f: { id: string; message: string }): string { + return `rule ${f.id} failed and was skipped: ${f.message.split('\n')[0]}`; +} + /** Apply per-rule severity overrides to results (design §6). */ export function applyRuleSeverities(results: Result[], config: Config): Result[] { return results.map((result) => { diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 7d856ede1..563389700 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -182,7 +182,8 @@ export { overrideMatches, settingSeverity, settingOptions, - withFailedRulesOff + withFailedRulesOff, + formatFailedRuleWarning } from './config-apply.js'; export type { CompiledOverride } from './config-apply.js'; diff --git a/packages/core/test/config-apply.test.ts b/packages/core/test/config-apply.test.ts index 417c07401..b1843ce46 100644 --- a/packages/core/test/config-apply.test.ts +++ b/packages/core/test/config-apply.test.ts @@ -7,6 +7,7 @@ import { overrideMatches, defineConfig, withFailedRulesOff, + formatFailedRuleWarning, type Rule, type Result } from '../src/index.js'; @@ -186,6 +187,12 @@ describe('withFailedRulesOff', () => { }); }); +describe('formatFailedRuleWarning', () => { + it('caps the message to its first line', () => { + expect(formatFailedRuleWarning({ id: 'x', message: 'boom\nstack' })).toBe('rule x failed and was skipped: boom'); + }); +}); + describe('override matching', () => { const config = defineConfig({ overrides: [{ files: 'src/lib/**', rules: { 'architecture/prop-count': 'off' } }] diff --git a/packages/vite/src/analyze.ts b/packages/vite/src/analyze.ts index 62976892d..2a8a19183 100644 --- a/packages/vite/src/analyze.ts +++ b/packages/vite/src/analyze.ts @@ -5,6 +5,7 @@ import { applyOverrides, runRules, withFailedRulesOff, + formatFailedRuleWarning, computeScore, summarize, hasFailureAtOrAbove, @@ -118,7 +119,7 @@ export async function analyze( const results = applyOverrides(applyRuleSeverities(rawResults, config), config); // Surfaced through the same `warnings` channel as config-file issues (plugin.ts logs each with // `console.warn`). - for (const f of failedRules) warnings.push(`rule ${f.id} failed and was skipped: ${f.message.split('\n')[0]}`); + for (const f of failedRules) warnings.push(formatFailedRuleWarning(f)); // A failed rule examined nothing, so its weight must not stay in the Health denominator — same // correction the CLI's `analyzeProject` applies, used by every downstream consumer here so the // score, reports, and fail decision agree. diff --git a/packages/vite/src/hooks/handle.ts b/packages/vite/src/hooks/handle.ts index ddb1b8add..e9c8726a2 100644 --- a/packages/vite/src/hooks/handle.ts +++ b/packages/vite/src/hooks/handle.ts @@ -5,9 +5,11 @@ import { applyRuleSeverities, defineConfig, effectiveSeverity, + formatFailedRuleWarning, isPenalized, runRules, selectRules, + terminalSafe, type Config, type Project, type ResolvedHead, @@ -37,14 +39,16 @@ export function findingSignature(results: Result[], config: Config): string { .join('|'); } -async function postIngest(origin: string, route: string, results: Result[]): Promise { +const warn = (line: string): void => console.warn(terminalSafe(line)); + +async function postIngest(origin: string, route: string, results: Result[], failedRuleIds: string[]): Promise { // `origin` comes from the request (Host header), so a spoofed Host must not // redirect this server-side POST to an arbitrary external host. if (!isLoopbackOrigin(origin)) { // Accessing the app over LAN/--host yields a non-loopback origin, so the live // UI silently stops updating — surface why when debugging is enabled. if (globalThis.process?.env?.SVELTE_VITALS_DEBUG) { - console.warn( + warn( `[svelte-vitals] live UI ingest skipped for non-loopback origin ${origin} — open the dashboard via localhost` ); } @@ -54,7 +58,9 @@ async function postIngest(origin: string, route: string, results: Result[]): Pro await fetch(`${origin}/__svelte-vitals/ingest`, { method: 'POST', headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ route, results }) + // failedRuleIds is always sent, empty array included, so a route that recovers from + // a previously-crashing rule clears its stale entry on the receiving store. + body: JSON.stringify({ route, results, failedRuleIds }) }); } catch { // dev tooling must never break a request — swallow ingest failures @@ -90,25 +96,28 @@ async function analyzeAndIngest( config }); const results = applyRuleSeverities(ruleResults, config); + const failedRuleIds = failedRules.map((f) => f.id); // Same debug-only channel as this function's own catch below — a failed rule is dropped // silently otherwise, since this hot per-request path has no other diagnostics surface. if (failedRules.length > 0 && globalThis.process?.env?.SVELTE_VITALS_DEBUG) { - for (const f of failedRules) console.warn(`[svelte-vitals] rule ${f.id} failed and was skipped: ${f.message}`); + for (const f of failedRules) warn(formatFailedRuleWarning(f)); } // Skip a repeat POST (and the SSE churn it would cause) when a route re-renders - // with the exact same findings — e.g. an unrelated HMR pass. - const signature = findingSignature(results, config); + // with the exact same findings — e.g. an unrelated HMR pass. The failed-ids suffix + // means a route that stops crashing (same findings, no more failures) still counts + // as a change, so its recovery reaches the store instead of being signature-skipped. + const signature = `${findingSignature(results, config)}|failed:${[...failedRuleIds].sort().join(',')}`; if (lastSignature.get(route) === signature) return; lastSignature.set(route, signature); - if (globalThis.process?.env?.SVELTE_VITALS_UI) void postIngest(origin, route, results); + if (globalThis.process?.env?.SVELTE_VITALS_UI) void postIngest(origin, route, results, failedRuleIds); } catch (err) { // Dev tooling must never break the request: swallow any parse/rule error. // Set SVELTE_VITALS_DEBUG to surface tool-internal errors while debugging. if (globalThis.process?.env?.SVELTE_VITALS_DEBUG) { - console.warn('[svelte-vitals] dev analysis failed:', err); + warn(`[svelte-vitals] dev analysis failed: ${err instanceof Error ? err.message : String(err)}`); } } } diff --git a/packages/vite/src/plugin.ts b/packages/vite/src/plugin.ts index 2e39bb833..e2e91814f 100644 --- a/packages/vite/src/plugin.ts +++ b/packages/vite/src/plugin.ts @@ -16,6 +16,7 @@ import { defaultConfig, resolveRuleOptions, shouldSkipRangeCheck, + terminalSafe, validateRuleSetting } from '@svelte-vitals/core'; import { findUnknownRuleIds, knownRuleIds, ruleOptionsSpec } from 'svelte-vitals'; @@ -42,6 +43,9 @@ const CONFIG_BASENAMES = new Set([ const IGNORED_SEGMENTS = new Set(['node_modules', '.svelte-kit', 'build', 'dist']); +/** Analyzed-repo-derived strings (rule messages, config warnings) can carry raw terminal escapes — sanitize at this sink boundary, not per interpolation. */ +const warn = (line: string): void => console.warn(terminalSafe(line)); + /** * Whether a `server.watcher` event on `file` should trigger a dev-dashboard re-analysis: * anything under `src/` or `static/` (the default SvelteKit layout this dashboard assumes), @@ -226,10 +230,10 @@ export function svelteVitals(options: SvelteVitalsOptions = {}): Plugin | Plugin // gate instead of failing the whole build — distinct from `result.failed`. // Config-file validation errors never reach here: resolved above, before // the try, they propagate and fail the build instead. - console.warn(`svelte-vitals: skipped — analysis failed: ${err instanceof Error ? err.message : String(err)}`); + warn(`svelte-vitals: skipped — analysis failed: ${err instanceof Error ? err.message : String(err)}`); return; } - for (const w of result.warnings) console.warn(`svelte-vitals: ${w}`); + for (const w of result.warnings) warn(`svelte-vitals: ${w}`); if (result.routeCount === 0) return; if (options.report !== false) { @@ -277,9 +281,14 @@ export function svelteVitals(options: SvelteVitalsOptions = {}): Plugin | Plugin config = mergeConfig(options, undefined); warnings = []; } - for (const w of warnings) console.warn(`svelte-vitals: ${w}`); + for (const w of warnings) warn(`svelte-vitals: ${w}`); const store = createStore(); + // The whole-project runner's failure-adjusted config (crashed rules forced 'off'), + // read by installUiMiddleware on every request via the getter below — a plain + // variable would only ever see the value at configureServer time, not later re-runs. + let staticConfig: Config | undefined; + // Whole-project static analysis: one run at startup (never blocking dev-server // start) plus a debounced re-run on relevant source changes (design doc // 2026-07-08-dev-dashboard-whole-project-design.md). Failures are warned and the @@ -290,7 +299,10 @@ export function svelteVitals(options: SvelteVitalsOptions = {}): Plugin | Plugin metaComponents: options.metaComponents, rules: options.rules, failOn: options.failOn, - onResults: (results) => store.setStatic(results), + onResults: (results, cfg) => { + store.setStatic(results); + if (cfg) staticConfig = cfg; + }, onError: (err) => console.warn('[svelte-vitals] dev analysis failed:', err), onStatusChange: (analyzing) => store.setAnalyzing(analyzing) }); @@ -307,7 +319,7 @@ export function svelteVitals(options: SvelteVitalsOptions = {}): Plugin | Plugin runner.stop(); }); - installUiMiddleware(server, config, readPackageVersion(), store, readCoreVersion()); + installUiMiddleware(server, config, readPackageVersion(), store, readCoreVersion(), () => staticConfig); // The dashboard has no separate CLI entry point (unlike `vitest --ui`) to signal // it exists, so announce it the same way Vite announces its own dev server: as an diff --git a/packages/vite/src/ui/analysis.ts b/packages/vite/src/ui/analysis.ts index ed58c5bf2..1c201879f 100644 --- a/packages/vite/src/ui/analysis.ts +++ b/packages/vite/src/ui/analysis.ts @@ -1,5 +1,5 @@ import { relative, sep } from 'node:path'; -import type { Result, RuleSetting, Severity, TreatDynamicAs } from '@svelte-vitals/core'; +import type { Config, Result, RuleSetting, Severity, TreatDynamicAs } from '@svelte-vitals/core'; import { analyzeProject, type ParseCache } from 'svelte-vitals'; /** The subset of `analyzeProject` (from `svelte-vitals`) the runner needs. Injectable for tests. */ @@ -10,7 +10,7 @@ export type AnalyzeFn = (opts: { rules?: Record; failOn?: Severity; parseCache?: ParseCache; -}) => Promise<{ results: Result[] }>; +}) => Promise<{ results: Result[]; config?: Config }>; export interface AnalysisRunnerOptions { /** Project root to analyze (passed as `cwd` to `analyzeProject`). */ @@ -21,7 +21,8 @@ export interface AnalysisRunnerOptions { failOn?: Severity; /** `analyzeProject`-compatible function, injectable for tests. Defaults to `analyzeProject`. */ analyze?: AnalyzeFn; - onResults(results: Result[]): void; + /** `config` is `analyzeProject`'s failure-adjusted config (crashed rules forced `'off'`) — omitted when the injected `analyze` doesn't return one. */ + onResults(results: Result[], config?: Config): void; onError(err: unknown): void; /** Called `true` right before a run starts its `analyze()` call and `false` once that run settles — including right before a coalesced follow-up starts again, so a rapid burst of changes may emit false-then-true between runs rather than staying true throughout. */ onStatusChange?(analyzing: boolean): void; @@ -57,7 +58,7 @@ export function createAnalysisRunner(opts: AnalysisRunnerOptions): AnalysisRunne running = true; opts.onStatusChange?.(true); try { - const { results } = await analyze({ + const { results, config } = await analyze({ cwd: opts.root, treatDynamicAs: opts.treatDynamicAs, metaComponents: opts.metaComponents, @@ -65,7 +66,12 @@ export function createAnalysisRunner(opts: AnalysisRunnerOptions): AnalysisRunne failOn: opts.failOn, parseCache }); - if (!stopped) opts.onResults(results); + // Passing a 2nd arg only when defined keeps callers that ignore it (and tests + // asserting exact call args) unaffected by this addition. + if (!stopped) { + if (config !== undefined) opts.onResults(results, config); + else opts.onResults(results); + } } catch (err) { if (!stopped) opts.onError(err); } finally { diff --git a/packages/vite/src/ui/middleware.ts b/packages/vite/src/ui/middleware.ts index e89458f98..fdafc5090 100644 --- a/packages/vite/src/ui/middleware.ts +++ b/packages/vite/src/ui/middleware.ts @@ -59,7 +59,9 @@ export function installUiMiddleware( config: Config, version: string, store: FindingsStore, - coreVersion?: string + coreVersion?: string, + /** Reads the whole-project runner's current failure-adjusted config; called per request so a later re-analysis is reflected without re-mounting the middleware. Undefined until the first run completes. */ + getStaticConfig?: () => Config | undefined ): void { const clients = new Set(); @@ -112,9 +114,10 @@ export function installUiMiddleware( req.on('data', (c: Buffer) => chunks.push(c)); req.on('end', () => { try { - const { route, results } = JSON.parse(Buffer.concat(chunks).toString('utf8')); + const { route, results, failedRuleIds } = JSON.parse(Buffer.concat(chunks).toString('utf8')); if (typeof route === 'string' && Array.isArray(results)) { - store.set(route, results.filter(isResultLike)); + const failedIds = Array.isArray(failedRuleIds) ? failedRuleIds.filter((id) => typeof id === 'string') : []; + store.set(route, results.filter(isResultLike), failedIds); } } catch { // ignore malformed ingest payloads — dev tooling must not crash the dev server @@ -139,7 +142,7 @@ export function installUiMiddleware( if (url.startsWith('/data.json')) { try { - const snapshot = buildSnapshot(store, config, { version, coreVersion }); + const snapshot = buildSnapshot(store, config, { version, coreVersion }, getStaticConfig?.()); res.setHeader('Content-Type', 'application/json'); res.end(JSON.stringify(snapshot)); } catch { @@ -152,7 +155,7 @@ export function installUiMiddleware( // Last line of defense that validated data should never reach: if the renderer // throws anyway, return a plain-text 500 and never take down the dev server. try { - const html = renderAppShell(buildSnapshot(store, config, { version, coreVersion })); + const html = renderAppShell(buildSnapshot(store, config, { version, coreVersion }, getStaticConfig?.())); res.setHeader('Content-Type', 'text/html'); res.end(html); } catch { diff --git a/packages/vite/src/ui/snapshot.ts b/packages/vite/src/ui/snapshot.ts index ea42898a3..0c2b87d16 100644 --- a/packages/vite/src/ui/snapshot.ts +++ b/packages/vite/src/ui/snapshot.ts @@ -1,4 +1,11 @@ -import { buildJsonReport, safeHref, type AppSnapshot, type Config, type JsonReport } from '@svelte-vitals/core'; +import { + buildJsonReport, + safeHref, + withFailedRulesOff, + type AppSnapshot, + type Config, + type JsonReport +} from '@svelte-vitals/core'; import type { FindingsStore } from './store.js'; type Issue = JsonReport['routes'][number]['issues'][number]; @@ -23,17 +30,26 @@ function sanitizeReport(report: JsonReport): JsonReport { }; } -/** Build the payload shared by the dashboard shell's embedded JSON and the /data.json endpoint. */ +/** + * Build the payload shared by the dashboard shell's embedded JSON and the /data.json endpoint. + * `staticConfig` is the whole-project runner's failure-adjusted config (crashed static rules + * already forced `'off'`) — falls back to `config` before the first run completes. Live-layer + * crashed rules (`store.failedRuleIds()`) are layered on top the same way the CLI and build mode + * apply `withFailedRulesOff`, so a rule that crashed on either layer scores as not-run instead of + * inflating Health. + */ export function buildSnapshot( store: FindingsStore, config: Config, - meta: { version: string; coreVersion?: string } + meta: { version: string; coreVersion?: string }, + staticConfig?: Config ): AppSnapshot { + const scoringConfig = withFailedRulesOff(staticConfig ?? config, store.failedRuleIds()); return { // No rule-id list threaded through: `report.rules` is seeded from `store.snapshot()` // alone here, so presence means "produced a result", not "was selected" — unlike the // `json` reporter (design doc 2026-08-03-json-rule-evidence-design.md, Not in scope). - report: sanitizeReport(buildJsonReport(store.snapshot(), config, meta)), + report: sanitizeReport(buildJsonReport(store.snapshot(), scoringConfig, meta)), badges: store.badges(), analyzing: store.isAnalyzing(), sequence: store.sequence(), diff --git a/packages/vite/src/ui/store.ts b/packages/vite/src/ui/store.ts index d0d578672..c6878185e 100644 --- a/packages/vite/src/ui/store.ts +++ b/packages/vite/src/ui/store.ts @@ -9,8 +9,12 @@ export type RouteBadge = 'measured' | 'static'; * the merged view per the design doc (2026-07-08-dev-dashboard-whole-project-design.md §2). */ export interface FindingsStore { - /** Replace a route's live findings (route stamped onto results missing one) and notify subscribers. */ - set(route: string, results: Result[]): void; + /** + * Replace a route's live findings (route stamped onto results missing one) and notify + * subscribers. `failedRuleIds` replaces that route's live-layer failed-rule set — an + * omitted or empty array clears it, so a route re-analyzed with no failures recovers. + */ + set(route: string, results: Result[], failedRuleIds?: string[]): void; /** Replace the whole static (whole-project) layer and notify subscribers. */ setStatic(results: Result[]): void; /** Mark whether a whole-project analysis run is currently in flight; participates in subscribe/notify like a findings change. */ @@ -20,6 +24,8 @@ export interface FindingsStore { snapshot(): Result[]; /** Per-route provenance for the dashboard's badges: 'measured' (live) or 'static'. */ badges(): Record; + /** Union of failed rule ids across every live (ingested) route, for `withFailedRulesOff`. */ + failedRuleIds(): string[]; /** Monotonically increasing counter, bumped once per notify() — lets consumers discard stale fetches. */ sequence(): number; /** Subscribe to change notifications; returns an unsubscribe function. */ @@ -72,6 +78,7 @@ export function composeBadges(staticResults: Result[], liveByRoute: Map(); + const liveFailedByRoute = new Map(); const subs = new Set<() => void>(); let analyzing = false; let seq = 0; @@ -82,11 +89,13 @@ export function createStore(): FindingsStore { } return { - set(route, results) { + set(route, results, failedRuleIds) { liveByRoute.set( route, results.map((r) => (r.route ? r : { ...r, route })) ); + if (failedRuleIds && failedRuleIds.length > 0) liveFailedByRoute.set(route, failedRuleIds); + else liveFailedByRoute.delete(route); notify(); }, setStatic(results) { @@ -106,6 +115,9 @@ export function createStore(): FindingsStore { badges() { return composeBadges(staticResults, liveByRoute); }, + failedRuleIds() { + return [...new Set([...liveFailedByRoute.values()].flat())].sort(); + }, sequence() { return seq; }, diff --git a/packages/vite/test/dev-handle.test.ts b/packages/vite/test/dev-handle.test.ts index 7e21ff46b..9f853658d 100644 --- a/packages/vite/test/dev-handle.test.ts +++ b/packages/vite/test/dev-handle.test.ts @@ -175,6 +175,70 @@ describe('svelteVitalsHandle', () => { expect(penalizedIds(sentResults(fetchMock))).not.toContain('seo/title-presence'); }); + it("forwards a crashed rule's id as failedRuleIds on the ingest POST", async () => { + vi.resetModules(); + vi.doMock('@svelte-vitals/core', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + runRules: async () => ({ + results: [], + examined: {}, + failedRules: [{ id: 'seo/title-presence', message: 'boom' }] + }) + }; + }); + const fetchMock = setup(); + try { + const { svelteVitalsHandle: mockedHandle } = await import('../src/hooks/index.js'); + const handle = mockedHandle(); + await handle({ event: fakeEvent('/none', '/none'), resolve: resolveWith([PAGE_NO_TITLE]) }); + await flush(); + const [, init] = fetchMock.mock.calls[0]!; + const sent = JSON.parse((init as RequestInit).body as string); + expect(sent.failedRuleIds).toEqual(['seo/title-presence']); + } finally { + vi.doUnmock('@svelte-vitals/core'); + vi.resetModules(); + } + }); + + it('sends an empty failedRuleIds array once a previously-crashing rule recovers', async () => { + vi.resetModules(); + let shouldFail = true; + vi.doMock('@svelte-vitals/core', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + runRules: async (rules: unknown, ctx: Parameters[1]) => { + if (shouldFail) + return { results: [], examined: {}, failedRules: [{ id: 'seo/title-presence', message: 'boom' }] }; + return actual.runRules(rules as never, ctx); + } + }; + }); + const fetchMock = setup(); + try { + const { svelteVitalsHandle: mockedHandle } = await import('../src/hooks/index.js'); + const handle = mockedHandle(); + await handle({ event: fakeEvent('/none', '/none'), resolve: resolveWith([PAGE_NO_TITLE]) }); + await flush(); + shouldFail = false; + // Re-render the same page: `findingSignature` alone would be unchanged (results now + // real instead of empty, but the crash's presence is what must invalidate the skip), + // so this POST must still fire. + await handle({ event: fakeEvent('/none', '/none'), resolve: resolveWith([PAGE_NO_TITLE]) }); + await flush(); + expect(fetchMock).toHaveBeenCalledTimes(2); + const [, init] = fetchMock.mock.calls[1]!; + const sent = JSON.parse((init as RequestInit).body as string); + expect(sent.failedRuleIds).toEqual([]); + } finally { + vi.doUnmock('@svelte-vitals/core'); + vi.resetModules(); + } + }); + it('surfaces swallowed analysis errors when SVELTE_VITALS_DEBUG is set', async () => { vi.resetModules(); vi.doMock('../src/providers/rendered/parse-html.js', () => ({ diff --git a/packages/vite/test/ui-analysis.test.ts b/packages/vite/test/ui-analysis.test.ts index 1d30c75a3..0b97dbe20 100644 --- a/packages/vite/test/ui-analysis.test.ts +++ b/packages/vite/test/ui-analysis.test.ts @@ -53,6 +53,27 @@ describe('createAnalysisRunner', () => { }); }); + it('passes the failure-adjusted config through to onResults as a 2nd arg when analyze returns one', async () => { + const adjustedConfig = { + rules: { 'seo/title-presence': 'off' } + } as unknown as import('@svelte-vitals/core').Config; + const analyze = vi.fn(async () => ({ results: [], config: adjustedConfig })); + const onResults = vi.fn(); + const runner = createAnalysisRunner({ root: '/proj', analyze, onResults, onError: vi.fn() }); + runner.start(); + await vi.waitFor(() => expect(onResults).toHaveBeenCalledTimes(1)); + expect(onResults).toHaveBeenCalledWith([], adjustedConfig); + }); + + it('calls onResults with a single arg when analyze omits config (existing callers unaffected)', async () => { + const analyze = vi.fn(async () => ({ results: [] })); + const onResults = vi.fn(); + const runner = createAnalysisRunner({ root: '/proj', analyze, onResults, onError: vi.fn() }); + runner.start(); + await vi.waitFor(() => expect(onResults).toHaveBeenCalledTimes(1)); + expect(onResults).toHaveBeenCalledWith([]); + }); + it('coalesces N rapid notifyChange calls into a single debounced run', async () => { const analyze = vi.fn(async () => ({ results: [] })); const runner = createAnalysisRunner({ diff --git a/packages/vite/test/ui-middleware.test.ts b/packages/vite/test/ui-middleware.test.ts index 7d3ed1a86..1582b7be6 100644 --- a/packages/vite/test/ui-middleware.test.ts +++ b/packages/vite/test/ui-middleware.test.ts @@ -3,21 +3,22 @@ import { EventEmitter } from 'node:events'; import type { IncomingMessage, ServerResponse } from 'node:http'; import type { ViteDevServer } from 'vite'; import { installUiMiddleware } from '../src/ui/middleware.js'; -import { createStore } from '../src/ui/store.js'; -import { defineConfig } from '@svelte-vitals/core'; +import { createStore, type FindingsStore } from '../src/ui/store.js'; +import { defineConfig, type Config } from '@svelte-vitals/core'; type MiddlewareHandler = (req: IncomingMessage, res: ServerResponse, next: () => void) => void; // Capture the handler that installUiMiddleware registers on server.middlewares.use(path, fn). -function setup(coreVersion?: string) { +function setup(coreVersion?: string, getStaticConfig?: () => Config | undefined, store: FindingsStore = createStore()) { let handler: MiddlewareHandler = () => {}; const httpServer = new EventEmitter(); const server = { httpServer, middlewares: { use: (_path: string, fn: MiddlewareHandler) => (handler = fn) } } as unknown as ViteDevServer; - installUiMiddleware(server, defineConfig({}), '9.9.9', createStore(), coreVersion); + installUiMiddleware(server, defineConfig({}), '9.9.9', store, coreVersion, getStaticConfig); return { + store, call: (req: IncomingMessage, res: ServerResponse) => handler(req, res, () => {}), closeServer: () => httpServer.emit('close') }; @@ -334,4 +335,127 @@ describe('installUiMiddleware', () => { call(getReq('/data.json', { host: 'evil.example' }), jr); expect(jr.statusCode).toBe(403); }); + + it('an ingested failedRuleIds list lowers the score vs. the same payload without it', async () => { + // 'warning', not 'critical' — so the critical-cap doesn't mask the denominator shift. + const body = (failedRuleIds?: string[]) => + JSON.stringify({ + route: '/a', + results: [ + { + id: 'seo/canonical-url', + message: 'm', + category: 'seo', + detection: { presence: 'none', value: 'absent' }, + route: '/a', + severity: 'warning' + } + ], + ...(failedRuleIds ? { failedRuleIds } : {}) + }); + + const control = setup(); + const cr = postReq('/ingest'); + control.call(cr, res()); + cr.emit('data', Buffer.from(body())); + cr.emit('end'); + await new Promise((r) => setTimeout(r, 0)); + const controlData = JSON.parse( + (() => { + const jr = res(); + control.call(getReq('/data.json'), jr); + return jr.chunks.join(''); + })() + ); + + const failing = setup(); + const fr = postReq('/ingest'); + failing.call(fr, res()); + fr.emit('data', Buffer.from(body(['seo/title-presence']))); + fr.emit('end'); + await new Promise((r) => setTimeout(r, 0)); + const failingData = JSON.parse( + (() => { + const jr = res(); + failing.call(getReq('/data.json'), jr); + return jr.chunks.join(''); + })() + ); + + expect(failingData.report.score).not.toBe(controlData.report.score); + }); + + it('tolerates a non-array failedRuleIds field (treated as no failures)', async () => { + const { call } = setup(); + const ireq = postReq('/ingest'); + call(ireq, res()); + ireq.emit( + 'data', + Buffer.from( + JSON.stringify({ + route: '/a', + results: [ + { + id: 'seo/title-presence', + message: 'm', + category: 'seo', + detection: { presence: 'none', value: 'absent' }, + route: '/a', + severity: 'critical' + } + ], + failedRuleIds: 'nonsense' + }) + ) + ); + ireq.emit('end'); + await new Promise((r) => setTimeout(r, 0)); + const gr = res(); + call(getReq('/'), gr); + expect(gr.statusCode).not.toBe(500); // did not crash on the malformed field + expect(gr.chunks.join('')).toContain('seo/title-presence'); // finding still stored + }); + + it('tolerates an absent failedRuleIds field (treated as no failures)', async () => { + const { call } = setup(); + const ireq = postReq('/ingest'); + call(ireq, res()); + ireq.emit('data', Buffer.from(ingestBody)); // no failedRuleIds key at all + ireq.emit('end'); + await new Promise((r) => setTimeout(r, 0)); + const gr = res(); + call(getReq('/'), gr); + expect(gr.statusCode).not.toBe(500); + }); + + it('reads getStaticConfig per request so a later re-analysis is reflected without re-mounting', () => { + // A mutable holder (not a reassigned `let`) so the getter reads whatever's current at + // request time — the object reference passed to setup() never changes, only its field. + const configHolder: { current?: Config } = {}; + const store = createStore(); + store.setStatic([ + { + id: 'seo/canonical-url', + message: 'm', + category: 'seo', + detection: { presence: 'none', value: 'absent' }, + route: '/a', + severity: 'warning' + } + ]); + const { call } = setup(undefined, () => configHolder.current, store); + + const before = res(); + call(getReq('/data.json'), before); + const beforeScore = JSON.parse(before.chunks.join('')).report.score; + + // A later whole-project run turns the rule off — the getter reads the CURRENT value at + // request time, not a snapshot taken when installUiMiddleware was called. + configHolder.current = defineConfig({ rules: { 'seo/canonical-url': 'off' } }); + const after = res(); + call(getReq('/data.json'), after); + const afterScore = JSON.parse(after.chunks.join('')).report.score; + + expect(afterScore).not.toBe(beforeScore); + }); }); diff --git a/packages/vite/test/ui-snapshot.test.ts b/packages/vite/test/ui-snapshot.test.ts index db034c848..fbcbd1060 100644 --- a/packages/vite/test/ui-snapshot.test.ts +++ b/packages/vite/test/ui-snapshot.test.ts @@ -1,7 +1,7 @@ import { describe, it, expect } from 'vitest'; import { buildSnapshot } from '../src/ui/snapshot.js'; import { createStore } from '../src/ui/store.js'; -import { defineConfig, type Result } from '@svelte-vitals/core'; +import { defineConfig, withFailedRulesOff, buildJsonReport, type Result } from '@svelte-vitals/core'; const r = (id: string, route: string, extra: Partial = {}): Result => ({ @@ -54,4 +54,40 @@ describe('buildSnapshot', () => { const second = buildSnapshot(store, defineConfig({}), { version: '9.9.9' }); expect(second.sequence).toBeGreaterThan(first.sequence); }); + + it('scores a live-layer failed rule as not-run, matching withFailedRulesOff', () => { + const store = createStore(); + // 'warning' (not the r() default 'critical') so the critical-cap doesn't mask the + // denominator shift this test is actually pinning. + const finding = r('seo/canonical-url', '/a', { severity: 'warning' }); + store.setStatic([finding]); + const config = defineConfig({}); + + const control = buildSnapshot(store, config, { version: '9.9.9' }); + + // seo/title-presence (a different, real rule) reported as failed — no result for it exists, + // same as a rule that crashed and produced nothing. + store.set('/a', [finding], ['seo/title-presence']); + const withFailure = buildSnapshot(store, config, { version: '9.9.9' }); + + expect(withFailure.report.score).not.toBe(control.report.score); + const expectedConfig = withFailedRulesOff(config, ['seo/title-presence']); + const expected = buildJsonReport(store.snapshot(), expectedConfig, { version: '9.9.9' }); + expect(withFailure.report.score).toBe(expected.score); + }); + + it('scores a static-layer failed rule as not-run when staticConfig is passed', () => { + const store = createStore(); + store.setStatic([r('seo/canonical-url', '/a', { severity: 'warning' })]); + const config = defineConfig({}); + + const control = buildSnapshot(store, config, { version: '9.9.9' }); + + const staticConfig = withFailedRulesOff(config, ['seo/title-presence']); + const withFailure = buildSnapshot(store, config, { version: '9.9.9' }, staticConfig); + + expect(withFailure.report.score).not.toBe(control.report.score); + const expected = buildJsonReport(store.snapshot(), staticConfig, { version: '9.9.9' }); + expect(withFailure.report.score).toBe(expected.score); + }); }); diff --git a/packages/vite/test/ui-store.test.ts b/packages/vite/test/ui-store.test.ts index 8ab72517a..636f8c3d2 100644 --- a/packages/vite/test/ui-store.test.ts +++ b/packages/vite/test/ui-store.test.ts @@ -142,6 +142,41 @@ describe('createStore', () => { expect(fn).toHaveBeenCalledTimes(2); }); + it('failedRuleIds() is empty when nothing has failed', () => { + const s = createStore(); + s.set('/a', [r('seo/title-presence', '/a')]); + expect(s.failedRuleIds()).toEqual([]); + }); + + it('failedRuleIds() unions failed ids across routes, sorted', () => { + const s = createStore(); + s.set('/a', [r('seo/title-presence', '/a')], ['seo/json-ld']); + s.set('/b', [r('seo/description-presence', '/b')], ['seo/canonical-url']); + expect(s.failedRuleIds()).toEqual(['seo/canonical-url', 'seo/json-ld']); + }); + + it('re-set on a route replaces its failed-rule ids, not appends', () => { + const s = createStore(); + s.set('/a', [r('seo/title-presence', '/a')], ['seo/json-ld']); + s.set('/a', [r('seo/title-presence', '/a')], ['seo/canonical-url']); + expect(s.failedRuleIds()).toEqual(['seo/canonical-url']); + }); + + it('re-set with no failedRuleIds clears a route that previously failed (recovery)', () => { + const s = createStore(); + s.set('/a', [r('seo/title-presence', '/a')], ['seo/json-ld']); + expect(s.failedRuleIds()).toEqual(['seo/json-ld']); + s.set('/a', [r('seo/title-presence', '/a')]); + expect(s.failedRuleIds()).toEqual([]); + }); + + it('re-set with an empty failedRuleIds array clears a route that previously failed', () => { + const s = createStore(); + s.set('/a', [r('seo/title-presence', '/a')], ['seo/json-ld']); + s.set('/a', [r('seo/title-presence', '/a')], []); + expect(s.failedRuleIds()).toEqual([]); + }); + it('sequence() strictly increases across set/setStatic/setAnalyzing', () => { const s = createStore(); const seq0 = s.sequence(); From d8d176e19f870c467f4606f49c40a23f7dce8d4f Mon Sep 17 00:00:00 2001 From: oekazuma Date: Wed, 12 Aug 2026 14:48:46 +0900 Subject: [PATCH 2/2] fix(vite): stop swapping the dashboard's scoring config, thread failed-rule ids instead MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Revision to the previous commit: buildSnapshot's staticConfig param silently dropped plugin-option weights/overrides once the first whole-project run completed, since analyzeProject's returned config never saw those plugin-only values. Replace the config-swap with an ids-only contract: analyzeProject now also returns failedRuleIds (already folded into its own returned config via withFailedRulesOff, exposed separately for callers with their own base config); the vite runner/plugin/snapshot/middleware thread that id list instead of a Config, so the dashboard's base config — and any plugin-option weights/overrides it carries — is never swapped, only layered with withFailedRulesOff. Co-Authored-By: Claude Fable 5 --- .changeset/dashboard-failed-rules.md | 5 +++- packages/cli/src/index.ts | 9 ++++--- .../cli/test/rule-failure-isolation.test.ts | 7 +++++- packages/vite/src/plugin.ts | 15 +++++------ packages/vite/src/ui/analysis.ts | 12 ++++----- packages/vite/src/ui/middleware.ts | 8 +++--- packages/vite/src/ui/snapshot.ts | 15 +++++------ packages/vite/test/ui-analysis.test.ts | 11 +++----- packages/vite/test/ui-middleware.test.ts | 22 +++++++++------- packages/vite/test/ui-snapshot.test.ts | 25 +++++++++++++++---- 10 files changed, 78 insertions(+), 51 deletions(-) diff --git a/.changeset/dashboard-failed-rules.md b/.changeset/dashboard-failed-rules.md index b926e90b0..312d08e9e 100644 --- a/.changeset/dashboard-failed-rules.md +++ b/.changeset/dashboard-failed-rules.md @@ -1,8 +1,11 @@ --- '@svelte-vitals/core': minor +'svelte-vitals': minor '@svelte-vitals/vite': patch --- `@svelte-vitals/core` now exports `formatFailedRuleWarning`, the "rule … failed and was skipped" message formatter shared by the CLI, build mode, and (now) the dev dashboard. -The dev dashboard now scores a crashed rule as not-run (matching the CLI and build mode) instead of silently inflating Health; plugin warnings strip terminal escape sequences. +`svelte-vitals`'s `analyzeProject` now also returns `failedRuleIds`, the ids of rules that crashed during the run (already folded into its returned `config` via `withFailedRulesOff`, exposed separately so a caller with its own base config can apply the same correction without adopting `analyzeProject`'s config). + +The dev dashboard now scores a crashed rule as not-run (matching the CLI and build mode) instead of silently inflating Health, without disturbing plugin-option `weights`/`overrides`; plugin warnings strip terminal escape sequences. diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index ce0d91fc4..66aaffa3f 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -196,6 +196,8 @@ export interface AnalyzeResult { ruleIds: string[]; /** Per-rule, per-declaration counts of places examined, unfiltered by `--diff`/`--baseline`/suppressions. */ examined: Record>; + /** Ids of rules `runRules` caught throwing — already folded into `config` via `withFailedRulesOff`; exposed separately so a caller with its own base config (the vite dev dashboard) can apply the same correction without adopting this call's `config`. */ + failedRuleIds: string[]; /** Non-fatal issues surfaced during analysis: config-file problems (unknown top-level keys, invalid enum values), version-floor notices, `--rules`/overrides conflicts, and skipped-file notices. Empty when none apply. */ warnings: string[]; /** @@ -337,16 +339,15 @@ export async function analyzeProject(opts: AnalyzeOptions = {}): Promise f.id) - ); + const failedRuleIds = failedRules.map((f) => f.id); + const scoringConfig = withFailedRulesOff(config, failedRuleIds); return { results, config: scoringConfig, version: readPackageVersion(), ruleIds: rules.map((r) => r.id), examined, + failedRuleIds, warnings: [...warnings, ...skippedFileWarnings([...components, ...kitModules]), ...failedRuleWarnings(failedRules)], loadedConfig: loaded }; diff --git a/packages/cli/test/rule-failure-isolation.test.ts b/packages/cli/test/rule-failure-isolation.test.ts index c385b602e..1c3fbf858 100644 --- a/packages/cli/test/rule-failure-isolation.test.ts +++ b/packages/cli/test/rule-failure-isolation.test.ts @@ -25,7 +25,7 @@ vi.mock('@svelte-vitals/core', async (importOriginal) => { return { ...actual, allRules }; }); -const { run } = await import('../src/index.js'); +const { run, analyzeProject } = await import('../src/index.js'); function capture() { const out: string[] = []; @@ -56,4 +56,9 @@ describe('rule-failure isolation (audit 2608-CORE-06)', () => { ); expect(cap.err.some((l) => l.includes('with a second line'))).toBe(false); }); + + it('analyzeProject returns the crashed rule id in failedRuleIds', async () => { + const result = await analyzeProject({ cwd: fixtureDir, allowRules: ['seo/title-presence'] }); + expect(result.failedRuleIds).toEqual(['seo/title-presence']); + }); }); diff --git a/packages/vite/src/plugin.ts b/packages/vite/src/plugin.ts index e2e91814f..42ee757d7 100644 --- a/packages/vite/src/plugin.ts +++ b/packages/vite/src/plugin.ts @@ -284,10 +284,11 @@ export function svelteVitals(options: SvelteVitalsOptions = {}): Plugin | Plugin for (const w of warnings) warn(`svelte-vitals: ${w}`); const store = createStore(); - // The whole-project runner's failure-adjusted config (crashed rules forced 'off'), - // read by installUiMiddleware on every request via the getter below — a plain - // variable would only ever see the value at configureServer time, not later re-runs. - let staticConfig: Config | undefined; + // The whole-project runner's crashed-rule ids, read by installUiMiddleware on every + // request via the getter below — a plain variable would only ever see the value at + // configureServer time, not later re-runs. Ids only, not a config: `config` above + // (carrying plugin-option weights/overrides) must stay the scoring base always. + let staticFailedRuleIds: string[] = []; // Whole-project static analysis: one run at startup (never blocking dev-server // start) plus a debounced re-run on relevant source changes (design doc @@ -299,9 +300,9 @@ export function svelteVitals(options: SvelteVitalsOptions = {}): Plugin | Plugin metaComponents: options.metaComponents, rules: options.rules, failOn: options.failOn, - onResults: (results, cfg) => { + onResults: (results, failedRuleIds) => { store.setStatic(results); - if (cfg) staticConfig = cfg; + staticFailedRuleIds = failedRuleIds ?? []; }, onError: (err) => console.warn('[svelte-vitals] dev analysis failed:', err), onStatusChange: (analyzing) => store.setAnalyzing(analyzing) @@ -319,7 +320,7 @@ export function svelteVitals(options: SvelteVitalsOptions = {}): Plugin | Plugin runner.stop(); }); - installUiMiddleware(server, config, readPackageVersion(), store, readCoreVersion(), () => staticConfig); + installUiMiddleware(server, config, readPackageVersion(), store, readCoreVersion(), () => staticFailedRuleIds); // The dashboard has no separate CLI entry point (unlike `vitest --ui`) to signal // it exists, so announce it the same way Vite announces its own dev server: as an diff --git a/packages/vite/src/ui/analysis.ts b/packages/vite/src/ui/analysis.ts index 1c201879f..794148dfe 100644 --- a/packages/vite/src/ui/analysis.ts +++ b/packages/vite/src/ui/analysis.ts @@ -1,5 +1,5 @@ import { relative, sep } from 'node:path'; -import type { Config, Result, RuleSetting, Severity, TreatDynamicAs } from '@svelte-vitals/core'; +import type { Result, RuleSetting, Severity, TreatDynamicAs } from '@svelte-vitals/core'; import { analyzeProject, type ParseCache } from 'svelte-vitals'; /** The subset of `analyzeProject` (from `svelte-vitals`) the runner needs. Injectable for tests. */ @@ -10,7 +10,7 @@ export type AnalyzeFn = (opts: { rules?: Record; failOn?: Severity; parseCache?: ParseCache; -}) => Promise<{ results: Result[]; config?: Config }>; +}) => Promise<{ results: Result[]; failedRuleIds?: string[] }>; export interface AnalysisRunnerOptions { /** Project root to analyze (passed as `cwd` to `analyzeProject`). */ @@ -21,8 +21,8 @@ export interface AnalysisRunnerOptions { failOn?: Severity; /** `analyzeProject`-compatible function, injectable for tests. Defaults to `analyzeProject`. */ analyze?: AnalyzeFn; - /** `config` is `analyzeProject`'s failure-adjusted config (crashed rules forced `'off'`) — omitted when the injected `analyze` doesn't return one. */ - onResults(results: Result[], config?: Config): void; + /** `failedRuleIds` is `analyzeProject`'s crashed-rule ids — omitted when the injected `analyze` doesn't return them. Ids only, not a config: the base config (plugin-option weights/overrides included) must stay the caller's, never swapped for `analyzeProject`'s own. */ + onResults(results: Result[], failedRuleIds?: string[]): void; onError(err: unknown): void; /** Called `true` right before a run starts its `analyze()` call and `false` once that run settles — including right before a coalesced follow-up starts again, so a rapid burst of changes may emit false-then-true between runs rather than staying true throughout. */ onStatusChange?(analyzing: boolean): void; @@ -58,7 +58,7 @@ export function createAnalysisRunner(opts: AnalysisRunnerOptions): AnalysisRunne running = true; opts.onStatusChange?.(true); try { - const { results, config } = await analyze({ + const { results, failedRuleIds } = await analyze({ cwd: opts.root, treatDynamicAs: opts.treatDynamicAs, metaComponents: opts.metaComponents, @@ -69,7 +69,7 @@ export function createAnalysisRunner(opts: AnalysisRunnerOptions): AnalysisRunne // Passing a 2nd arg only when defined keeps callers that ignore it (and tests // asserting exact call args) unaffected by this addition. if (!stopped) { - if (config !== undefined) opts.onResults(results, config); + if (failedRuleIds !== undefined) opts.onResults(results, failedRuleIds); else opts.onResults(results); } } catch (err) { diff --git a/packages/vite/src/ui/middleware.ts b/packages/vite/src/ui/middleware.ts index fdafc5090..b1e6403d2 100644 --- a/packages/vite/src/ui/middleware.ts +++ b/packages/vite/src/ui/middleware.ts @@ -60,8 +60,8 @@ export function installUiMiddleware( version: string, store: FindingsStore, coreVersion?: string, - /** Reads the whole-project runner's current failure-adjusted config; called per request so a later re-analysis is reflected without re-mounting the middleware. Undefined until the first run completes. */ - getStaticConfig?: () => Config | undefined + /** Reads the whole-project runner's current crashed-rule ids; called per request so a later re-analysis is reflected without re-mounting the middleware. Empty/undefined until the first run completes or when nothing has failed. */ + getStaticFailedRuleIds?: () => string[] | undefined ): void { const clients = new Set(); @@ -142,7 +142,7 @@ export function installUiMiddleware( if (url.startsWith('/data.json')) { try { - const snapshot = buildSnapshot(store, config, { version, coreVersion }, getStaticConfig?.()); + const snapshot = buildSnapshot(store, config, { version, coreVersion }, getStaticFailedRuleIds?.()); res.setHeader('Content-Type', 'application/json'); res.end(JSON.stringify(snapshot)); } catch { @@ -155,7 +155,7 @@ export function installUiMiddleware( // Last line of defense that validated data should never reach: if the renderer // throws anyway, return a plain-text 500 and never take down the dev server. try { - const html = renderAppShell(buildSnapshot(store, config, { version, coreVersion }, getStaticConfig?.())); + const html = renderAppShell(buildSnapshot(store, config, { version, coreVersion }, getStaticFailedRuleIds?.())); res.setHeader('Content-Type', 'text/html'); res.end(html); } catch { diff --git a/packages/vite/src/ui/snapshot.ts b/packages/vite/src/ui/snapshot.ts index 0c2b87d16..3b3b3cc1f 100644 --- a/packages/vite/src/ui/snapshot.ts +++ b/packages/vite/src/ui/snapshot.ts @@ -32,19 +32,20 @@ function sanitizeReport(report: JsonReport): JsonReport { /** * Build the payload shared by the dashboard shell's embedded JSON and the /data.json endpoint. - * `staticConfig` is the whole-project runner's failure-adjusted config (crashed static rules - * already forced `'off'`) — falls back to `config` before the first run completes. Live-layer - * crashed rules (`store.failedRuleIds()`) are layered on top the same way the CLI and build mode - * apply `withFailedRulesOff`, so a rule that crashed on either layer scores as not-run instead of - * inflating Health. + * `config` is never swapped for another config — plugin-option `weights`/`overrides` must + * survive every request. `staticFailedRuleIds` (the whole-project runner's crashed-rule ids) + * and the store's live-layer union (`store.failedRuleIds()`) are both layered onto `config` via + * `withFailedRulesOff`, the same correction the CLI and build mode apply, so a rule that crashed + * on either layer scores as not-run instead of inflating Health. */ export function buildSnapshot( store: FindingsStore, config: Config, meta: { version: string; coreVersion?: string }, - staticConfig?: Config + staticFailedRuleIds?: string[] ): AppSnapshot { - const scoringConfig = withFailedRulesOff(staticConfig ?? config, store.failedRuleIds()); + const failedRuleIds = [...new Set([...(staticFailedRuleIds ?? []), ...store.failedRuleIds()])]; + const scoringConfig = withFailedRulesOff(config, failedRuleIds); return { // No rule-id list threaded through: `report.rules` is seeded from `store.snapshot()` // alone here, so presence means "produced a result", not "was selected" — unlike the diff --git a/packages/vite/test/ui-analysis.test.ts b/packages/vite/test/ui-analysis.test.ts index 0b97dbe20..cf864766b 100644 --- a/packages/vite/test/ui-analysis.test.ts +++ b/packages/vite/test/ui-analysis.test.ts @@ -53,19 +53,16 @@ describe('createAnalysisRunner', () => { }); }); - it('passes the failure-adjusted config through to onResults as a 2nd arg when analyze returns one', async () => { - const adjustedConfig = { - rules: { 'seo/title-presence': 'off' } - } as unknown as import('@svelte-vitals/core').Config; - const analyze = vi.fn(async () => ({ results: [], config: adjustedConfig })); + it('passes failedRuleIds through to onResults as a 2nd arg when analyze returns them', async () => { + const analyze = vi.fn(async () => ({ results: [], failedRuleIds: ['seo/title-presence'] })); const onResults = vi.fn(); const runner = createAnalysisRunner({ root: '/proj', analyze, onResults, onError: vi.fn() }); runner.start(); await vi.waitFor(() => expect(onResults).toHaveBeenCalledTimes(1)); - expect(onResults).toHaveBeenCalledWith([], adjustedConfig); + expect(onResults).toHaveBeenCalledWith([], ['seo/title-presence']); }); - it('calls onResults with a single arg when analyze omits config (existing callers unaffected)', async () => { + it('calls onResults with a single arg when analyze omits failedRuleIds (existing callers unaffected)', async () => { const analyze = vi.fn(async () => ({ results: [] })); const onResults = vi.fn(); const runner = createAnalysisRunner({ root: '/proj', analyze, onResults, onError: vi.fn() }); diff --git a/packages/vite/test/ui-middleware.test.ts b/packages/vite/test/ui-middleware.test.ts index 1582b7be6..f69b09483 100644 --- a/packages/vite/test/ui-middleware.test.ts +++ b/packages/vite/test/ui-middleware.test.ts @@ -4,19 +4,23 @@ import type { IncomingMessage, ServerResponse } from 'node:http'; import type { ViteDevServer } from 'vite'; import { installUiMiddleware } from '../src/ui/middleware.js'; import { createStore, type FindingsStore } from '../src/ui/store.js'; -import { defineConfig, type Config } from '@svelte-vitals/core'; +import { defineConfig } from '@svelte-vitals/core'; type MiddlewareHandler = (req: IncomingMessage, res: ServerResponse, next: () => void) => void; // Capture the handler that installUiMiddleware registers on server.middlewares.use(path, fn). -function setup(coreVersion?: string, getStaticConfig?: () => Config | undefined, store: FindingsStore = createStore()) { +function setup( + coreVersion?: string, + getStaticFailedRuleIds?: () => string[] | undefined, + store: FindingsStore = createStore() +) { let handler: MiddlewareHandler = () => {}; const httpServer = new EventEmitter(); const server = { httpServer, middlewares: { use: (_path: string, fn: MiddlewareHandler) => (handler = fn) } } as unknown as ViteDevServer; - installUiMiddleware(server, defineConfig({}), '9.9.9', store, coreVersion, getStaticConfig); + installUiMiddleware(server, defineConfig({}), '9.9.9', store, coreVersion, getStaticFailedRuleIds); return { store, call: (req: IncomingMessage, res: ServerResponse) => handler(req, res, () => {}), @@ -428,10 +432,10 @@ describe('installUiMiddleware', () => { expect(gr.statusCode).not.toBe(500); }); - it('reads getStaticConfig per request so a later re-analysis is reflected without re-mounting', () => { + it('reads getStaticFailedRuleIds per request so a later re-analysis is reflected without re-mounting', () => { // A mutable holder (not a reassigned `let`) so the getter reads whatever's current at // request time — the object reference passed to setup() never changes, only its field. - const configHolder: { current?: Config } = {}; + const idsHolder: { current?: string[] } = {}; const store = createStore(); store.setStatic([ { @@ -443,15 +447,15 @@ describe('installUiMiddleware', () => { severity: 'warning' } ]); - const { call } = setup(undefined, () => configHolder.current, store); + const { call } = setup(undefined, () => idsHolder.current, store); const before = res(); call(getReq('/data.json'), before); const beforeScore = JSON.parse(before.chunks.join('')).report.score; - // A later whole-project run turns the rule off — the getter reads the CURRENT value at - // request time, not a snapshot taken when installUiMiddleware was called. - configHolder.current = defineConfig({ rules: { 'seo/canonical-url': 'off' } }); + // A later whole-project run reports seo/canonical-url as failed — the getter reads the + // CURRENT value at request time, not a snapshot taken when installUiMiddleware was called. + idsHolder.current = ['seo/canonical-url']; const after = res(); call(getReq('/data.json'), after); const afterScore = JSON.parse(after.chunks.join('')).report.score; diff --git a/packages/vite/test/ui-snapshot.test.ts b/packages/vite/test/ui-snapshot.test.ts index fbcbd1060..316825109 100644 --- a/packages/vite/test/ui-snapshot.test.ts +++ b/packages/vite/test/ui-snapshot.test.ts @@ -76,18 +76,33 @@ describe('buildSnapshot', () => { expect(withFailure.report.score).toBe(expected.score); }); - it('scores a static-layer failed rule as not-run when staticConfig is passed', () => { + it('scores a static-layer failed rule as not-run when staticFailedRuleIds is passed', () => { const store = createStore(); store.setStatic([r('seo/canonical-url', '/a', { severity: 'warning' })]); const config = defineConfig({}); const control = buildSnapshot(store, config, { version: '9.9.9' }); - - const staticConfig = withFailedRulesOff(config, ['seo/title-presence']); - const withFailure = buildSnapshot(store, config, { version: '9.9.9' }, staticConfig); + const withFailure = buildSnapshot(store, config, { version: '9.9.9' }, ['seo/title-presence']); expect(withFailure.report.score).not.toBe(control.report.score); - const expected = buildJsonReport(store.snapshot(), staticConfig, { version: '9.9.9' }); + const expected = buildJsonReport(store.snapshot(), withFailedRulesOff(config, ['seo/title-presence']), { + version: '9.9.9' + }); expect(withFailure.report.score).toBe(expected.score); }); + + it('preserves plugin-option weights once the static layer reports a failed rule (regression: config must never swap)', () => { + const store = createStore(); + store.setStatic([r('seo/canonical-url', '/a', { severity: 'warning' })]); + // A non-default seo weight, as a plugin-option config would carry — analyzeProject + // (and any mocked equivalent) never sees this value, so the fix must thread it through + // untouched rather than falling back to analyzeProject's own unweighted config. + const weightedConfig = defineConfig({ weights: { seo: 5 } }); + + const before = buildSnapshot(store, weightedConfig, { version: '9.9.9' }); + const after = buildSnapshot(store, weightedConfig, { version: '9.9.9' }, ['seo/title-presence']); + + expect(after.report.weights).toEqual(before.report.weights); + expect(after.report.weights.seo).toBe(5); + }); });