Clean all web warnings: eliminate vitest console noise + build/autofixer issues - #48
Merged
Conversation
…sues - mqtt-store-bootstrap.spec.js: spy and silence the intentional '/api/conversations bootstrap failed' console.warn from the error-path tests; assert the warning fired so suppression never hides a regression. - history-status-normalization.spec.js: poll for the async fetchHistory result instead of a fixed 10ms wait, removing an intermittent flake. - ReadReceipt.svelte: remove dead, malformed $derived (it passed a function to $derived and was never referenced); key the each block. - EmojiPicker.svelte / SearchPanel.svelte: add keys to static each blocks. The bits-ui Dialog 'derived_inert' teardown warning is resolved by the scroll-lock drain in tests/setup.js (#46), so no console filter is needed.
Aztec03hub
added a commit
that referenced
this pull request
Jun 26, 2026
…e import/jsdom overhead (#52) The web unit-test suite's cost was dominated by per-file overhead, not test logic: the same module graph (App, mqtt-store, components, bits-ui, the lucide-svelte icon barrel) was re-imported and re-executed, and a fresh jsdom environment was created, for every one of the 94 spec files. Switch the bulk of the suite to vitest's vmThreads pool, which runs each spec file in its own fresh V8 VM context (full per-file isolation, no cross-file state leakage) while reusing the worker's compiled-module cache across files. The three specs that replace the whole window/location global object (not just define a property on it) cannot run under vmThreads, where those globals are non-configurable; they are routed to a standard threads project via test.projects. No test assertions or test code changed. Measured (matched-load A/B, 3 runs each): wall ~51s -> ~32s (-37%) duration ~47s -> ~30s import ~240s -> ~174s environment ~46s -> ~5.4s (-88%) transform ~71s -> ~64s tests ~16s -> ~12s Still 94 files / 1292 tests pass with 0 unhandled errors and 0 stderr noise (verified 5 consecutive runs). The bits-ui scroll-lock drain in tests/setup.js (PR #46) and the clean output (PR #48) are preserved.
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
1 participant
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.
Goal
Bring the web side (
web/) to a clean state: zero vitest unhandled errors, a clean test console, a clean vite build, and zero real svelte-autofixer issues.What was wrong (analysis)
vitest console noise (real — invisible in a naive
pnpm testtail; the default reporter prints test console output to stderr)[claude-comms] /api/conversations bootstrap failed {…}(console.warn)mqtt-store-bootstrap.spec.js[svelte] derived_inert(read of a derived from a now-destroyed effect)channel-modal-wires.spec.js(+channel-modal-bugfix.spec.js)Dialogteardown timing artifactexpected undefined to be truthy(flake, ~1/5 runs)history-status-normalization.spec.js#fetchHistoryvite build
Already clean — exit 0, no warnings (largest chunk
vendor-ui425 kB < the 500 kB default limit).svelte-autofixer (all 56 components + 5
.svelte.jsmodules + App.svelte)Grep pre-screen + targeted autofixer runs. No Svelte 4 syntax (
on:,export let,<slot>,$:) anywhere.$effectusages are all genuine side-effects (localStorage sync, focus/teardown, element measurement, IntersectionObserver, timers, dev warn) — several already comment that the "should be$derived" suggestion does not apply. Real issues found:ReadReceipt.svelte—tooltipText = $derived(() => {…})passed a function to$derived(should be$derived.by) and was dead code (never referenced; the template inlines the logic). Plus an unkeyed{#each}.EmojiPicker.svelte/SearchPanel.svelte— unkeyed{#each}over static lists.What changed
console.warn; assert the warning fired (strengthens, not weakens).$derived; keyed the each.The
derived_inertwarning is resolved by the bits-ui scroll-lock drain intests/setup.jsfrom #46 (picked up on rebase), so no console filter is needed — confirmed across 4 full runs with no suppression.Before / after
pnpm --dir web testbootstrap failed+ ~26–60derived_inert+ flakepnpm --dir web buildIntentionally-left autofixer advisories (documented):
bind:this(19 components — idiomatic DOM refs for focus/scroll/measure) and "assign inside$effect" on side-effecting effects (e.g.ArtifactList'sreconcilewrites to localStorage, so it must stay an effect — the autofixer itself hedges this one).No Python touched (
src/claude_comms/**.py/ pyproject unchanged).🤖 Generated with Claude Code