revert(runner): keep the compiler chunk's content hash (DEV-2569) - #250
Merged
Conversation
Reverts the hash-free `assets/compiler-babel.js` from #249. The shape fix in that PR — `asBabel` at both import sites — is untouched and is the fix the ticket was about; this is only the second half, which a high-effort review showed to be a net regression for the exact population it targeted. The chunk is not self-contained. Rollup hoists the shared CJS interop helpers into the entry, so the emitted compiler chunk opens with import { c as SD, g as Nke } from "./index-<hash>.js"; and that path is content-hashed. Verified on the deployed build (the merge of #249 is live): `/assets/compiler-babel.js` begins `import{c as SD,g as Nke}from"./index-BpieVTaX.js"`, index.html loads that same hashed entry, and the entry's top level is `createRoot(document.getElementById("root")).render(…)` plus Sentry.init. So in the scenario the rename was meant to cure — a tab open across a deploy that then triggers its first Tier-1 compile — the stable path resolves against the *new* build, statically pulls a 1.3 MB entry the tab has never loaded, and evaluates a second complete copy of the app. React 18 clears the root container on mount, so the visitor's workspace is detached and silently remounted from a different build: unsaved edits gone, no card, two Sentry clients. That is worse than the carded failure it replaced, which tells the visitor to reload and is what `describeRuntimeError` and `rearmCompilerLoad`'s docblock both promise. The mirror case is the same root cause: a cached stable chunk against a newer page has a static import of a rotated-out hashed path, i.e. the original DEV-2569 failure on a fresh load. The hash is therefore load-bearing: it is what makes a rotated chunk fail loudly. A stable path is still the right end state, but it needs the compiler built as its own self-contained artifact — or the SPA fallback stopped from answering /assets/* — rather than a `chunkFileNames` rename. Recorded in the config comment so it is not re-added, and a follow-up will carry the real fix. `check-compiler-chunk.mjs` is rewritten around the hashed name and keeps its reason for existing (the `manualChunks` variant that made the chunk eager is still red). Two gaps the same review found are closed: it now also matches a bare `import"./babel-<hash>.js"` — the form Rollup emits when the importer uses none of the chunk's exports, which was eager while matching no `from` pattern (verified red by injecting one) — and it prints the chunk's own count of content-hashed dependencies, the number that has to reach zero before a rename is safe. It stays wired into ci.yml and master.yml. Verified: pnpm test 847 pass / 0 fail, typecheck clean, guard green on the build and red on an injected bare import, preview-recovery.spec.ts 4 passed / 5 live-skipped against the rebuilt app. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Follow-up to #249, from the high-effort review of it. Reverts only the hash-free compiler chunk name. The shape fix —
asBabelat both import sites, which is what DEV-2569's second pass was actually about — is untouched.The chunk is not self-contained
Rollup hoists the shared CJS interop helpers into the entry, so the emitted compiler chunk opens with a static import of a content-hashed file. Verified against the live deploy of #249:
and that entry chunk's top level is
createRoot(document.getElementById("root")).render(…)plusSentry.init(main.tsx: "Must stay first: initialises error reporting before any other module runs").So in the one scenario the rename existed to cure — a tab open across a deploy that then triggers its first Tier-1 compile — the stable path resolves against the new build, statically pulls a 1.3 MB entry chunk the tab has never loaded, and evaluates a second complete copy of the app. React 18 clears the root container on mount, so the visitor's workspace is detached and silently remounted from a different build: unsaved edits gone, no card, two Sentry clients, a second copy of every module singleton including a second
babelLoaderlatch.That is strictly worse than what it replaced. The carded failure it removed says "Restart the preview to try again, or reload the page — a tab left open across a deployment has to reload to pick up the current version" — actionable, and it does not throw away work. Not losing unsaved edits is the stated reason
rearmCompilerLoadexists at all.The mirror case has the same root cause: a cached stable
compiler-babel.jsagainst a newer page carries a static import of a rotated-out./index-<oldhash>.js, so the SPA fallback answers200 text/htmland the original DEV-2569 failure is back — now reachable on a fresh load rather than only in a stale tab.The hash is load-bearing: it is what makes a rotated chunk fail loudly instead of resolving against the wrong build. #249's new e2e could not have caught this — it exercises one build, where the compiler chunk and the entry agree by construction.
A stable path is still the right end state. It needs the compiler built as its own self-contained artifact (its own entry/bundle carrying its own interop helper), or the SPA fallback stopped from answering
/assets/*so a rotated chunk returns a real 404 — not achunkFileNamesrename. Recorded in the vite config comment so it is not re-added, and a follow-up ticket will carry it.The guard keeps its job, and closes two holes
scripts/check-compiler-chunk.mjsis rewritten around the hashed name. It still catches what it was written for — themanualChunksvariant that pulledgetDefaultExportFromCjsin and made the 2.3 MB chunk statically imported and modulepreloaded — and the same review found two gaps, both closed:import"./babel-<hash>.js", the form Rollup emits when the importer uses none of the chunk's exports. That shape was eager while matching nofrompattern and passing the dynamic-import count. Verified red by injecting one into a built chunk.1 hashed dependency/ies (index-<hash>.js) — must be 0 before it can be renamed. Reported rather than failed: it is the tripwire for when the rename becomes safe.Still wired into
ci.yml's authoring job andmaster.yml's deploy build.Verification
pnpm testpnpm typechecknode scripts/check-compiler-chunk.mjsok: assets/babel-U33ShAoN.js, lazily imported by index-DASxLQnA.js only; 1 hashed dependency/iesimports babel-U33ShAoN.js statically — the chunk is no longer lazypnpm e2e e2e/preview-recovery.spec.tsThe e2e route glob goes back to
**/assets/babel-*.js*. It is not vacuous after the change: the test asserts the recorded request list equals["(bare)", "?hotRetry=1"]before Restart, which only holds if the route fired.One review note recorded but not acted on here, since it predates both PRs:
playwright.config.ts's webServer swallows--port/--strictPort(pnpm eats them), and withreuseExistingServer: !CIa local run can silently attach to another worktree's server on 4173 and test a stale build. It bit both the review and me; both runs above used an own-port preview instead.🤖 Generated with Claude Code
Note
Cursor Bugbot is generating a summary for commit c1d64c9. Configure here.