Skip to content

revert(runner): keep the compiler chunk's content hash (DEV-2569) #250

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
demtario merged 1 commit into from
Aug 20, 2026
Merged

revert(runner): keep the compiler chunk's content hash (DEV-2569) #250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 21 additions & 35 deletions runner/apps/authoring/vite.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,41 +33,27 @@ export default defineConfig({
// plugin's post-upload cleanup never runs to remove — and a manual
// `wrangler deploy` would publish them.
sourcemap: uploadEnabled,
rollupOptions: {
output: {
// Keep @babel/standalone in a chunk whose path does not change per build
// (DEV-2569). This app is served from Workers Assets with
// `not_found_handling: "single-page-application"` (see wrangler.jsonc), so a
// deploy removes the previous build's hashed chunks and their paths answer
// `200 text/html` instead of a 404. A tab that had not yet fetched the ~2.3 MB
// compiler when a deploy landed was then asking for a file that no longer
// existed — forever, since an HTML body is not a module. A stable path always
// exists in the deploy that is currently live.
//
// No cache-header work goes with this: Workers Assets already serves every
// asset, hashed or not, as `cache-control: public, max-age=0, must-revalidate`
// with an ETag (measured on prod 2026-08-20), so the stable URL revalidates on
// each use and picks up the new bytes.
//
// Two accepted consequences. 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 the embedded debug id rather than by filename,
// so reusing a name across releases does not confuse symbolication.
//
// Renaming only, deliberately: no `manualChunks`. Assigning @babel/standalone to
// a named chunk was measured to pull Rollup's shared `getDefaultExportFromCjs`
// helper in with it (`export { … as b, … as g }`), which made two ordinary chunks
// import the 2.3 MB compiler *statically* and put a `modulepreload` for it in
// index.html — the opposite of lazy. Rollup's own split of the dynamic import in
// `packages/runtime/src/transpile.ts` already isolates it under the implicit name
// `babel`; all this does is take the hash off that one file. If a Rollup version
// ever renames that chunk, `scripts/check-compiler-chunk.mjs` goes red in CI rather
// than the hash quietly coming back.
chunkFileNames: (chunk) =>
chunk.name === "babel" ? "assets/compiler-babel.js" : "assets/[name]-[hash].js",
},
},
// ⚠ Do not give the @babel/standalone chunk a hash-free name (reverted from #249,
// DEV-2569). The intent was sound — Workers Assets serves this app with
// `not_found_handling: "single-page-application"`, so a deploy rotates the hashed chunk
// out and its path answers `200 text/html`, which strands a tab that had not fetched the
// compiler yet. But the chunk is *not* self-contained: Rollup hoists the shared CJS
// interop helpers into the entry, so the emitted chunk opens with
//
// import { c as SD, g as Nke } from "./index-<hash>.js";
//
// and that path is content-hashed. Measured on the deployed build: a stable
// `compiler-babel.js` therefore pulls the *new* build's 1.3 MB entry into an old tab, and
// that entry's top level is `createRoot(document.getElementById("root")).render(…)` plus
// `Sentry.init`. React 18 clears the root container, so the visitor's workspace is
// detached and silently remounted from a different build — unsaved edits gone, no card,
// two Sentry clients. That is strictly worse than the carded failure it replaced, which
// tells the visitor to reload (`describeRuntimeError`, and rearmCompilerLoad's docblock).
//
// A stable path is still the right end state; it needs the compiler built as its own
// self-contained artifact (or the SPA fallback stopped from answering /assets/*) rather
// than a `chunkFileNames` rename. Until then the hash is load-bearing: it is what makes a
// rotated chunk fail loudly.
},
plugins: [
react(),
Expand Down
9 changes: 5 additions & 4 deletions runner/e2e/preview-recovery.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,9 +286,10 @@ test("live: fixing a Vue template error clears the preview error card", async ({
// @babel/standalone: ~2.3 MB, code-split, fetched on first compile. It fails for ordinary
// reasons (offline, a blocked request, an extension) and it used to fail permanently for
// one of ours — a deploy rotating the chunk out from under a tab, which Workers Assets
// answers with `200 text/html` rather than a 404. `assets/compiler-babel.js` is hash-free
// now so that population is gone (`scripts/check-compiler-chunk.mjs` pins the name), and
// what is left is the transient case these two tests drive.
// answers with `200 text/html` rather than a 404. The hash-free chunk name meant to close that
// population is reverted (see vite.config.ts — a stable path with a content-hashed dependency
// drags a second copy of the app into an old tab), so a rotated chunk is still cured only by a
// reload and the transient case is what these two tests drive.
//
// This spec is the *only* place the loader can be tested honestly. Its two import sites are
// byte-identical in source and are not identical in the bundle — Vite rewrites the bare
Expand All @@ -305,7 +306,7 @@ test("live: fixing a Vue template error clears the preview error card", async ({

/** The compiler chunk plus any retry query. The trailing `*` is load-bearing: without it
* the `?hotRetry=n` requests slip past the block and the test passes for the wrong reason. */
const COMPILER_CHUNK = "**/assets/compiler-babel.js*";
const COMPILER_CHUNK = "**/assets/babel-*.js*";

test("a blocked compiler chunk cards, and Restart preview really recovers", async ({ page }) => {
// Local-build only, and not because it is slow or flaky: the premise is the artifact this
Expand Down
7 changes: 4 additions & 3 deletions runner/packages/runtime/src/transpile.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,10 @@ function findBabel(value: unknown, depth: number): Babel | null {
* nothing here knows it (that is the whole reason `retryBabelChunk` reads it out of the
* engine's error text). A shape mismatch discovered here therefore reaches
* `CompilerUnavailableError` with `assetUrl: null` and no retry — correct on both counts:
* refetching the same URL returns the same bytes and the same shape, and the chunk now has
* one fixed path (`assets/compiler-babel.js`), so naming it adds nothing the fingerprint
* does not already say. The engine's own wording still rides in `extra.cause`. */
* refetching the same URL returns the same bytes and the same shape, so there is nothing for
* a retry to do. Only the URL is lost, and `extra.cause` still carries the engine's own
* wording — a shape mismatch is a build-shape bug, which the hashed path does not help
* diagnose anyway. */
const loadBabelChunk = createLazyLoader<Babel>(() =>
import("@babel/standalone").then((m) => asBabel(m)),
);
Expand Down
93 changes: 54 additions & 39 deletions runner/scripts/check-compiler-chunk.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,83 +1,98 @@
#!/usr/bin/env node
// The compiler chunk must keep a hash-free path and stay lazily loaded (DEV-2569).
// The compiler chunk must stay lazily loaded, and self-contained enough to be renameable
// (DEV-2569).
//
// Two build-shaped promises no unit test can see, both of which failed in production once:
// `@babel/standalone` is ~2.3 MB and only Tier-1 examples ever compile, so Rollup's split of
// the dynamic import in `packages/runtime/src/transpile.ts` is what keeps it off the initial
// load. Two ways that has been lost or nearly lost, both measured, neither visible to any
// unit test:
//
// 1. `@babel/standalone` ships in its own chunk named `assets/compiler-babel.js`, with no
// content hash. `apps/authoring/wrangler.jsonc` serves this app from Workers Assets with
// `not_found_handling: "single-page-application"`, so a deploy removes the previous
// build's hashed chunks and their paths answer `200 text/html` instead of a 404. A tab
// that had not yet fetched the ~2.3 MB compiler when a deploy landed was asking for a
// file that no longer existed — forever, since an HTML body is not a module (Sentry
// DEMOS-15). The rename is a one-line `chunkFileNames` in `vite.config.ts`, and it rests
// on Rollup's implicit chunk name for the dynamic import in
// `packages/runtime/src/transpile.ts`. If a Rollup version renames that chunk, the hash
// comes back silently — this is what makes it loud instead.
// 1. **It went eager.** Naming the chunk via `manualChunks` pulled Rollup's shared
// `getDefaultExportFromCjs` helper in with it (`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. `manualChunks` assigns a module to a chunk; it does not keep the
// chunk lazy.
//
// 2. It stays *dynamically* imported. Naming it via `manualChunks` was measured to pull
// Rollup's shared `getDefaultExportFromCjs` helper into the same chunk, which made two
// ordinary chunks import 2.3 MB of compiler statically and put a `modulepreload` for it
// in index.html. Every visitor would have paid for a compiler that only Tier-1 examples
// use, and nothing else in the suite would have noticed.
// 2. **It stopped being self-contained.** The emitted chunk opens with
// `import { c, g } from "./index-<hash>.js"` — the content-hashed *entry*, whose top level
// renders the app. That is why #249's hash-free `compiler-babel.js` had to be reverted: a
// stable path with a hashed dependency drags a whole second copy of the app into an old
// tab. The count below is a tripwire, not a ban: if it ever reaches zero the chunk can be
// given a stable name, which is the fix DEV-2569 actually wants.
//
// Run against a real `vite build` output — the `authoring` job in ci.yml does, right after
// building it.
// Run against a real `vite build` output — ci.yml's `authoring` job and master.yml's deploy
// build both do, right after building it.

import { readFileSync, readdirSync, existsSync } from "node:fs";
import path from "node:path";
import { fileURLToPath } from "node:url";

const dist = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../apps/authoring/dist");
const CHUNK = "compiler-babel.js";

if (!existsSync(dist)) {
console.error(`no build to check at ${dist} — run \`pnpm --filter @handsontable/demo-authoring build\` first`);
process.exit(1);
}

const failures = [];
const assetsDir = path.join(dist, "assets");
// Guarded like `dist` itself: a half-cleaned build would otherwise crash with a raw ENOENT
// stack in place of the message this script exists to print.
if (!existsSync(assetsDir)) {
console.error(`${assetsDir} does not exist — the build did not finish`);
process.exit(1);
}

const failures = [];
const assets = readdirSync(assetsDir);

if (!assets.includes(CHUNK)) {
const hashed = assets.filter((f) => /^(compiler-)?babel-.*\.js$/.test(f));
// Rollup names the chunk after the dynamically imported package's entry.
const chunk = assets.find((f) => /^babel-[^/]*\.js$/.test(f));
if (!chunk) {
failures.push(
`assets/${CHUNK} is missing${hashed.length ? ` — found ${hashed.join(", ")} instead, so the chunk was renamed and \`chunkFileNames\` in vite.config.ts no longer matches it` : ""}`,
`no assets/babel-<hash>.js chunk — @babel/standalone is no longer code-split, so every visitor now downloads ~2.3 MB of compiler (found: ${assets.filter((f) => f.endsWith(".js")).join(", ")})`,
);
}

const html = readFileSync(path.join(dist, "index.html"), "utf8");
if (html.includes(CHUNK)) {
failures.push(`index.html references ${CHUNK} (a preload or a script tag) — the compiler must not be part of the initial load`);
if (chunk && html.includes(chunk)) {
failures.push(`index.html references ${chunk} (a preload or a script tag) — the compiler must not be part of the initial load`);
}

const dynamic = [];
for (const file of assets.filter((f) => f.endsWith(".js") && f !== CHUNK)) {
const code = readFileSync(path.join(assetsDir, file), "utf8");
// A static `import … from "./compiler-babel.js"` is the failure mode: it makes the chunk
// eager even though the source only ever writes `import(…)`.
if (/\bfrom\s*["']\.\/compiler-babel\.js["']/.test(code)) {
failures.push(`assets/${file} imports ${CHUNK} statically — the chunk is no longer lazy`);
if (chunk) {
for (const file of assets.filter((f) => f.endsWith(".js") && f !== chunk)) {
const code = readFileSync(path.join(assetsDir, file), "utf8");
// Both static forms. `from "./chunk"` is the ordinary one; a bare `import"./chunk"` is what
// Rollup emits when the importer uses none of the chunk's exports, and it makes the chunk
// just as eager while matching no `from` pattern.
const spec = `["']\\./${chunk.replace(/\./g, "\\.")}["']`;
if (new RegExp(`\\bfrom\\s*${spec}`).test(code) || new RegExp(`\\bimport\\s*${spec}`).test(code)) {
failures.push(`assets/${file} imports ${chunk} statically — the chunk is no longer lazy`);
}
if (code.includes(`import("./${chunk}")`)) dynamic.push(file);
}
if (code.includes(`import("./${CHUNK}")`)) dynamic.push(file);
}

if (dynamic.length !== 1) {
failures.push(
`expected exactly one chunk to dynamically import ${CHUNK}, found ${dynamic.length}${dynamic.length ? ` (${dynamic.join(", ")})` : " — the lazy load is gone"}`,
);
if (dynamic.length !== 1) {
failures.push(
`expected exactly one chunk to dynamically import ${chunk}, found ${dynamic.length}${dynamic.length ? ` (${dynamic.join(", ")})` : " — the lazy load is gone"}`,
);
}
}

// Reported, not failed: this is the number that has to reach zero before the chunk can carry a
// stable name (see the header). Printing it keeps the reason for the hash in front of whoever
// next reads this output.
const crossBuild = chunk
? [...readFileSync(path.join(assetsDir, chunk), "utf8").matchAll(/from"\.\/([^"]+)"/g)].map((m) => m[1])
: [];

if (failures.length) {
console.error("compiler chunk check failed:");
for (const f of failures) console.error(` - ${f}`);
process.exit(1);
}

console.log(`compiler chunk ok: assets/${CHUNK}, lazily imported by ${dynamic[0]} only`);
console.log(
`compiler chunk ok: assets/${chunk}, lazily imported by ${dynamic[0]} only; ` +
`${crossBuild.length} hashed dependency/ies (${crossBuild.join(", ") || "none"}) — must be 0 before it can be renamed`,
);
Loading