fix(runner): resolve the compiler chunk's real module shape on retry (DEV-2569) - #249
Merged
Conversation
…(DEV-2569) The retry mechanism shipped in #235 recovers the fetch and then cannot compile: `Failed to transpile /index.js for the parcel sandbox: e.transform is not a function`. Verification on deployed master c83975c found it; the two import sites in transpile.ts are byte-identical in source and are not identical in the bundle. Vite rewrites only the bare specifier, and @babel/standalone is CJS, so the primary path gets a CJS-interop hop the `@vite-ignore` retry does not. Measured in the deployed bundle: primary: import("./babel-<hash>.js").then(t => t.b).then(t => t.default ?? t) retry: import(`${url}?hotRetry=1`).then(o => o.default ?? o) chunk: export { Hke as b } // Hke = _mergeNamespaces({default: babel}, [cjs]) so the retry resolved the raw module record `{b: {…}}`, `m.default` was undefined, and the loader returned the record. `asBabel` replaces the fixed `default ?? m` unwrap at both sites with a shape check over the record, its `default`, and one wrapper level, preferring `default` at each — so the primary keeps resolving exactly what it resolves today, and the retry stops depending on an interop hop it never gets. Walking values rather than a hardcoded `b` survives Rollup renaming that export. A miss throws, from inside the loaders, so it becomes a CompilerUnavailableError and is latched, carded and filed under ["tier1-compiler-asset"]. Left un-thrown it surfaced downstream inside babel.transform and landed in the visitor-source bucket as if it were the visitor's typo — the other half of the defect. The URL rides in the thrown message because assetUrl is recovered by regexing the cause. Also removes the exposure that made a retry necessary: the compiler now ships as `assets/compiler-babel.js` with no content hash. Workers Assets serves this app with `not_found_handling: "single-page-application"`, so a deploy rotated the hashed chunk out and its path answered `200 text/html` — a stranded tab could never succeed. No cache-header change goes with it; Workers Assets already serves every asset as `max-age=0, must-revalidate` with an ETag (measured on prod). Renaming only, no `manualChunks`: assigning the package to a named chunk was measured to pull Rollup's shared getDefaultExportFromCjs helper in with it, which made two ordinary chunks import 2.3 MB statically and modulepreload it. Tests. The existing loader suite imports packages/runtime/dist, where both paths really are equivalent, which is why #235's ten tests were green over a broken retry — so the shape rule is pinned three ways instead. Five unit cases fix the three measured namespace shapes and the compiler-error routing. A deterministic Playwright case in preview-recovery.spec.ts blocks the chunk against a real `vite build`, asserts the card copy and the two bounded attempts, then lifts the block and requires the recovered compiler to hand a sandbox to the bundler — an attempted bundler request, which `buildSetup` never reaches when the transpile throws (measured: 2 attempts with this fix, 0 without). `data-preview-status` is deliberately not that oracle: `booting` precedes `error`, so asserting it passed with the fix reverted. Everything off-localhost is aborted, because a `parcel` sandbox loads its bundler from a versioned host stubShell's globs never matched. `scripts/check-compiler-chunk.mjs`, run in the authoring CI job, pins the hash-free name and that the chunk stays lazy — verified red against the eager-chunk regression above. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 001de30. Configure here.
Five findings, all real. The new deterministic e2e aborted everything off-localhost, and this file is listed in e2e-live.yml with `E2E_BASE_URL` pointed at a deployment for the nightly canary — where `baseURL` is off-localhost, so `page.goto` itself was aborted and the canary would have gone red every night. Gated to the local build: the test's premise is the artifact the run just built, and the deployed target may predate the stable chunk name. Its E2E_LIVE companion now asserts the chunk was actually requested before waiting on the card, so a target whose build predates that rename fails saying so instead of as an opaque 60 s timeout. `asBabel` is called without a URL at the primary site, and that is where a genuine shape change is met first — so `assetUrl` is null there and no retry is attempted. Both are correct (nothing at that site knows the hashed path, and refetching the same URL returns the same bytes and the same shape), but the unit test claimed otherwise: it passed CHUNK into the *primary* loader and asserted `assetUrl === CHUNK`, a property the shipped path does not have. The test now models the URL-less primary — asserting the null and the declined retry — with a separate case for the retry site, which does keep its URL. Writing that second case turned up a real fragility: `retryOf` was called *outside* the try in `createRetryingLoader`, so a synchronous throw from it escaped un-wrapped, and an error that is not a CompilerUnavailableError is filed as the visitor's own compile failure — the mis-routing half of this very defect. Production is safe today (the real `retryOf` normalises inside `.then`, so it rejects), but it was one refactor from regressing. Now inside the try, covered for both failure modes, and verified red without the change. `check-compiler-chunk.mjs` guarded `dist/` but not `dist/assets/`, so a half-cleaned build printed a raw ENOENT stack in place of the script's own message. And it ran only in ci.yml's PR build, so the hash silently returning could still ship from a push or workflow_dispatch that never had one — master.yml now runs it on the deploy build too, before the upload. Verified: pnpm test 847 pass / 0 fail, typecheck clean, chunk check green, preview-recovery.spec.ts green on the ungated path (4 passed) and correctly skipping the local-build case when E2E_BASE_URL is set. 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.

Closes DEV-2569 (second pass). Sentry DEMOS-15.
Why this is reopened
The retry mechanism shipped in #235 recovers the fetch and then cannot compile. Verification on deployed master
c83975c2found the card flipping toFailed to transpile /index.js for the parcel sandbox: e.transform is not a function, status stuck onerror, 0 grid cells.Re-confirmed independently against the live bundle (
/assets/index-uxATgr1X.js,/assets/babel-Du2YGyP9.js, fetched 2026-08-20):Mkeis Vite's_mergeNamespaces. Vite rewrites only the bare specifier, and@babel/standaloneis CJS, so the primary site gets a CJS-interop hop that/* @vite-ignore */suppresses on the retry — the retry resolved the chunk's raw module record{b: {…}},m.defaultwasundefined,m.default ?? mreturned the record, and.transformwas not a function.Second half of the defect: because no
CompilerUnavailableErrorwas ever constructed, theTypeErrorsurfaced downstream insidebabel.transform, fell past the compiler-asset branch intier1Report.ts, and landed in the visitor bucket["demo-runtime","sandpack-compile"]atlevel: warning— our own broken retry filed inside DEMOS-15 as if it were a visitor's typo.Part A — resolve the real module shape
asBabel(ns, url)replaces the fixeddefault ?? munwrap at both import sites: a shape walk over the record, itsdefault, and one wrapper level, preferringdefaultat each — so the primary keeps resolving exactly the object it resolves today, and the retry stops depending on an interop hop it never gets. Walking values rather than unwrapping a hardcodedbis what survives Rollup renaming that export; the name is bundler-generated and pinned by nothing.Three measured shapes it has to accept:
dist(whatpipeline/*.test.mjssees){ default: babel, …named }{ __proto__: null, default: babel }{ b: { default: babel, transform } }A miss throws, from inside the loaders, so the rejection reaches
createRetryingLoaderand becomes aCompilerUnavailableError— latched, carded, and filed under["tier1-compiler-asset"]atlevel: error. Wrapped aroundbabelLoader.loadinstead it would have kept landing in the visitor bucket. The URL rides in the thrown message becauseassetUrlis recovered byassetUrlFromregexing the cause, and that hashed path exists nowhere else at runtime.Unchanged: the bounded two attempts, the
?hotRetry=query (a browser caches a failed module fetch in the document's module map, so a retry must ask for a URL it has never seen), the latch, andrearmCompilerLoad. Safari also stays terminal on the first failure —assetUrlFromgets no URL out of "Load failed", soretryOfdeclines.Part B — remove the exposure that made a retry necessary
The compiler now ships as
assets/compiler-babel.js, with no content hash.wrangler.jsoncserves this app from Workers Assets withnot_found_handling: "single-page-application", so a deploy removed the previous build's hashed chunks and their paths answered200 text/htmlrather than a 404 — an HTML body is not a module, so a tab that had not yet fetched the compiler when a deploy landed was asking for a file that no longer existed, forever. A stable path always exists in the deploy that is live.No cache-header work goes with it. Measured on prod 2026-08-20, Workers Assets already serves every asset, hashed or not, as
cache-control: public, max-age=0, must-revalidatewith an ETag, so the stable URL revalidates on each use and picks up new bytes. No_headersfile exists and none is needed.Two accepted consequences, both written into the config comment: a tab open across a deploy that fetches the compiler afterwards gets the new build's babel (benign — we only call
transform, and strictly better than a permanent failure), and the Sentry plugin matches sourcemaps by embedded debug id rather than filename, so a name reused across releases does not confuse symbolication.Renaming only — no
manualChunks. The obvious form (manualChunks: id => id.includes("/@babel/standalone/") ? "compiler-babel" : undefined) was measured to pull Rollup's sharedgetDefaultExportFromCjsinto that chunk (export { … as b, … as g }), after which two ordinary chunks imported the 2.3 MB compiler statically andindex.htmlgained amodulepreloadfor it — every visitor paying for a compiler only Tier-1 examples use.manualChunksassigns a module to a chunk; it does not keep the chunk lazy. Rollup already splits the dynamic import under the implicit namebabel, so a one-linechunkFileNamesis the whole change.Tests — and why the existing suite could not see this
pipeline/transpile-loader.test.mjsimportspackages/runtime/dist/transpile.js, where both import paths really are equivalent. That is why #235's ten loader tests were green over a broken retry, and it means any test added to that file is green with or without this fix. The shape rule is pinned three ways instead.1. Unit (
pipeline/transpile-loader.test.mjs, +5 cases). The three measured namespace shapes, bundler-export-name independence, and the routing: a module that resolves withouttransformsatisfiesisCompilerUnavailableand keeps itsassetUrl. Verified red: with the olddefault ?? msemantics restored, 4 fail —the wrapper is not the compiler.2. Deterministic Playwright (
e2e/preview-recovery.spec.ts).playwright.config.tsalready serves a realvite build, which is the only place the divergence exists. The case blocks**/assets/compiler-babel.js*(trailing*load-bearing, or the?hotRetry=nrequests slip past), asserts the card copy and exactly two bounded attempts, then lifts the block and clicks Restart preview.Two things I got wrong on the way there, both fixed, because they are the kind of thing that reads as coverage and is not:
stubShelldoes not make a Tier-1parcelexample deterministic. Its globs aresandpack.codesandbox.ioandsandpack-bundler.codesandbox.io; a parcel sandbox loads a versioned host (2-19-8-sandpack.codesandbox.io) plus jsdelivr and prod-packager-packages. The case now aborts everything off-localhost on top.data-preview-statusis not the oracle.bootingis on the failure path too (it precedeserror), so asserting it passed with the fix reverted. The oracle is an attempted bundler request:buildSetupthrows insidesandboxFiles()before the client is created, so a failed transpile never reaches the bundler. Measured 2 attempts with this fix, 0 without. The requests are aborted at the route layer, so nothing leaves the machine.Verified red: with only the two
asBabelcalls reverted,Expected: > 0, Received: 0.3.
scripts/check-compiler-chunk.mjs, run in theauthoringCI job (the job that has the build). Pins the hash-free name, thatindex.htmlnever references the chunk, that nothing imports it statically, and that exactly one chunk imports it dynamically. Verified red against themanualChunksvariant above.A second case behind
E2E_LIVE=1drives the same recovery through a real bundler toready+expectGridRendered— the visible recovery that was never clicked through before #235 shipped.preview-recovery.spec.tsis already named ine2e-live.yml, so no workflow change. Not run here (needs the external bundler).Verification
Run from
runner/with rawpnpm(rtk filters have fabricated pass summaries on this repo), against my own preview port — 4173 was held by another worktree.pnpm testpnpm typechecknode scripts/check-compiler-chunk.mjscompiler chunk ok: assets/compiler-babel.js, lazily imported by index-DrczjuWM.js onlypnpm e2e e2e/preview-recovery.spec.tsThe 6
share-view.spec.tsfailures in the full run are its own documentedE2E_BASE_URLgate ("needs a deployed API origin — vite preview has no /api or /d routes") firing because pointing at a private port sets that variable. They skip in PR CI.Left for testing
Only the grouping half of the original acceptance, and it is structurally not machine-checkable:
reportingGate.tsgates onnavigator.webdriver, so no automated run can mint a Sentry event. DEMOS-15's title and the["tier1-compiler-asset"]split need one human look at the issue list after real post-fix traffic. Nothing in this PR verifies that, and the green e2e should not be read as if it does.🤖 Generated with Claude Code
Note
Medium Risk
Touches Tier‑1 transpile loading and production bundle splitting; wrong chunk naming or
asBabellogic could break all parcel previews or regress initial load size, though coverage is strong.Overview
Fixes DEV-2569 / DEMOS-15: Tier‑1 preview could stay broken after a compiler fetch retry because the
?hotRetry=import got a different module shape than the primary@babel/standalonechunk (Vite CJS interop vs@vite-ignore), yieldinge.transform is not a functionand mis-filed Sentry noise.Runtime:
asBabelwalks namespace/default/nested values at both load and retry sites; a miss throws with the URL socreateRetryingLoadersurfacesCompilerUnavailableErrorinstead of a visitor compile error.Build: Authoring Vite
chunkFileNamesemits a hash-freeassets/compiler-babel.jsso deploys don’t leave stale tabs fetching rotated hashed chunks that Workers SPA fallback serves as HTML.scripts/check-compiler-chunk.mjs(CI after build +pnpm check:compiler-chunk) asserts the stable name, lazy dynamic import only, and noindex.htmlpreload.Tests: Five loader unit cases for measured namespace shapes; Playwright blocks
compiler-babel.js, asserts Restart preview reaches the bundler after recovery; optionalE2E_LIVEgrid check.Reviewed by Cursor Bugbot for commit 001de30. Bugbot is set up for automated code reviews on this repo. Configure here.