Skip to content

fix(runner): resolve the compiler chunk's real module shape on retry (DEV-2569) - #249

Merged
demtario merged 2 commits into
masterfrom
fix/DEV-2569-retry-module-shape
Aug 20, 2026
Merged

fix(runner): resolve the compiler chunk's real module shape on retry (DEV-2569)#249
demtario merged 2 commits into
masterfrom
fix/DEV-2569-retry-module-shape

Conversation

@demtario

@demtario demtario commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

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 c83975c2 found the card flipping to Failed to transpile /index.js for the parcel sandbox: e.transform is not a function, status stuck on error, 0 grid cells.

Re-confirmed independently against the live bundle (/assets/index-uxATgr1X.js, /assets/babel-Du2YGyP9.js, fetched 2026-08-20):

primary: import("./babel-Du2YGyP9.js").then(t => t.b).then(t => t.default ?? t)
retry:   import(`${n}${r}hotRetry=${e+1}`).then(o => o.default ?? o)
chunk:   const Hke = Mke({__proto__: null, default: qke}, [bH]); export { Hke as b };

Mke is Vite's _mergeNamespaces. Vite rewrites only the bare specifier, and @babel/standalone is 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.default was undefined, m.default ?? m returned the record, and .transform was not a function.

Second half of the defect: because no CompilerUnavailableError was ever constructed, the TypeError surfaced downstream inside babel.transform, fell past the compiler-asset branch in tier1Report.ts, and landed in the visitor bucket ["demo-runtime","sandpack-compile"] at level: 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 fixed default ?? m unwrap at both import sites: a shape walk over the record, its default, and one wrapper level, preferring default at 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 hardcoded b is what survives Rollup renaming that export; the name is bundler-generated and pinned by nothing.

Three measured shapes it has to accept:

where shape
Node / dist (what pipeline/*.test.mjs sees) { default: babel, …named }
bundled primary, after Vite's hop { __proto__: null, default: babel }
bundled retry, no hop { b: { default: babel, transform } }

A miss throws, from inside the loaders, so the rejection reaches createRetryingLoader and becomes a CompilerUnavailableError — latched, carded, and filed under ["tier1-compiler-asset"] at level: error. Wrapped around babelLoader.load instead it would have kept landing in the visitor bucket. The URL rides in the thrown message because assetUrl is recovered by assetUrlFrom regexing 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, and rearmCompilerLoad. Safari also stays terminal on the first failure — assetUrlFrom gets no URL out of "Load failed", so retryOf declines.

Part B — remove the exposure that made a retry necessary

The compiler now ships as assets/compiler-babel.js, with no content hash. wrangler.jsonc serves this app from Workers Assets with not_found_handling: "single-page-application", so a deploy removed the previous build's hashed chunks and their paths answered 200 text/html rather 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-revalidate with an ETag, so the stable URL revalidates on each use and picks up new bytes. No _headers file 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 shared getDefaultExportFromCjs into that chunk (export { … as b, … as g }), after which two ordinary chunks imported the 2.3 MB compiler statically and index.html gained a modulepreload for it — every visitor paying for a compiler only Tier-1 examples use. manualChunks assigns a module to a chunk; it does not keep the chunk lazy. Rollup already splits the dynamic import under the implicit name babel, so a one-line chunkFileNames is the whole change.

Tests — and why the existing suite could not see this

pipeline/transpile-loader.test.mjs imports packages/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 without transform satisfies isCompilerUnavailable and keeps its assetUrl. Verified red: with the old default ?? m semantics restored, 4 fail — the wrapper is not the compiler.

2. Deterministic Playwright (e2e/preview-recovery.spec.ts). playwright.config.ts already serves a real vite build, which is the only place the divergence exists. The case blocks **/assets/compiler-babel.js* (trailing * load-bearing, or the ?hotRetry=n requests 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:

  • stubShell does not make a Tier-1 parcel example deterministic. Its globs are sandpack.codesandbox.io and sandpack-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-status is not the oracle. booting is on the failure path too (it precedes error), so asserting it passed with the fix reverted. The oracle is an attempted bundler request: buildSetup throws inside sandboxFiles() 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 asBabel calls reverted, Expected: > 0, Received: 0.

3. scripts/check-compiler-chunk.mjs, run in the authoring CI job (the job that has the build). Pins the hash-free name, that index.html never references the chunk, that nothing imports it statically, and that exactly one chunk imports it dynamically. Verified red against the manualChunks variant above.

A second case behind E2E_LIVE=1 drives the same recovery through a real bundler to ready + expectGridRendered — the visible recovery that was never clicked through before #235 shipped. preview-recovery.spec.ts is already named in e2e-live.yml, so no workflow change. Not run here (needs the external bundler).

Verification

Run from runner/ with raw pnpm (rtk filters have fabricated pass summaries on this repo), against my own preview port — 4173 was held by another worktree.

result
pnpm test 845 pass / 0 fail / 2 todo
pnpm typecheck clean
node scripts/check-compiler-chunk.mjs compiler chunk ok: assets/compiler-babel.js, lazily imported by index-DrczjuWM.js only
pnpm e2e e2e/preview-recovery.spec.ts 4 passed, 5 skipped (live)
full deterministic suite 201 passed
presence gate pass — 3 source files changed with a matching test change

The 6 share-view.spec.ts failures in the full run are its own documented E2E_BASE_URL gate ("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.ts gates on navigator.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 asBabel logic 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/standalone chunk (Vite CJS interop vs @vite-ignore), yielding e.transform is not a function and mis-filed Sentry noise.

Runtime: asBabel walks namespace/default/nested values at both load and retry sites; a miss throws with the URL so createRetryingLoader surfaces CompilerUnavailableError instead of a visitor compile error.

Build: Authoring Vite chunkFileNames emits a hash-free assets/compiler-babel.js so 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 no index.html preload.

Tests: Five loader unit cases for measured namespace shapes; Playwright blocks compiler-babel.js, asserts Restart preview reaches the bundler after recovery; optional E2E_LIVE grid check.

Reviewed by Cursor Bugbot for commit 001de30. Bugbot is set up for automated code reviews on this repo. Configure here.

…(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>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ 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.

Comment thread runner/e2e/preview-recovery.spec.ts
@demtario demtario self-assigned this Aug 20, 2026
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>
@demtario
demtario merged commit 977c412 into master Aug 20, 2026
7 checks passed
@demtario
demtario deleted the fix/DEV-2569-retry-module-shape branch August 20, 2026 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant