From dfca302d3747b6d1abd919c1d1cdd879fe92b7d8 Mon Sep 17 00:00:00 2001 From: Rames Jusso Date: Wed, 6 May 2026 04:37:04 +0000 Subject: [PATCH] fix(bundler): runtime mode opt-in, ASI-safe joinJsChunks, prune dead subs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per @vai-bot's review on hf#641: Important #1: dead `src=""` substitution sites ============================================= Now that `bundleToSingleHtml` inlines the runtime IIFE by default, the empty `src=""` placeholder is never emitted in the no-env-var path — the 5 downstream substitution sites that grep for `src=""` were dead. Two of them (studio dev server + studio vite preview) genuinely WANT the placeholder so they can hot-reload a local /api/runtime.js endpoint without re-inlining ~150 KB on every composition edit. Three of them (CLI validate, snapshot, layout) were just doing the same inlining the bundler already does. Resolution: - Add a `runtime: "inline" | "placeholder"` option to `BundleOptions`. Default is "inline" (matches the self-contained-bundle promise the function name makes). The two studio surfaces explicitly pass `{ runtime: "placeholder" }` to opt in. - studioServer.ts + studio/vite.config.ts: pass the option, keep their existing string-replace logic unchanged. - validate.ts + snapshot.ts + layout.ts: delete the now-redundant runtime substitution code (regex never matches the new inlined-runtime shape). Important #2: joinJsChunks ASI hazard ====================================== The new helper appended `;` to chunks not already ending in `;` and joined on `\n`. If a chunk ended with a `// line comment`, the appended semicolon was eaten by the comment, leaving the next chunk's first statement attached to the previous chunk's last expression — exactly the ASI hazard the helper exists to prevent. Fix: append `\n;` instead of `;` for chunks not already terminated. The newline closes the line comment, the standalone `;` becomes the statement separator. For typical chunks (already ending in `;`), output is unchanged — still clean `\n`-joined chunks with no bare-semicolon lines. Also added a trailing `;` to `wrapScopedCompositionScript`'s IIFE close (`})()` → `})();`) so composition scripts join cleanly without falling through to the `\n;` fallback. New test: regression guard at the chunk boundary verifies every inline script body in the bundle parses cleanly via esbuild even when a source JS file ends with a line comment. Verification ============ - `bun run --filter @hyperframes/core test` — 653/653 pass - `bun run --filter @hyperframes/cli test` — 243/243 pass - `bun run --filter @hyperframes/{core,cli,studio} typecheck` — clean - `bunx oxfmt --check` + `bunx oxlint` on all touched files — clean Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/cli/src/commands/layout.ts | 25 ++------- packages/cli/src/commands/snapshot.ts | 17 +----- packages/cli/src/commands/validate.ts | 24 ++------ packages/cli/src/server/studioServer.ts | 7 ++- .../core/src/compiler/compositionScoping.ts | 2 +- .../core/src/compiler/htmlBundler.test.ts | 34 ++++++++++++ packages/core/src/compiler/htmlBundler.ts | 55 +++++++++++++++---- packages/studio/vite.config.ts | 12 ++-- 8 files changed, 103 insertions(+), 73 deletions(-) diff --git a/packages/cli/src/commands/layout.ts b/packages/cli/src/commands/layout.ts index a8a713963..fa27dd70b 100644 --- a/packages/cli/src/commands/layout.ts +++ b/packages/cli/src/commands/layout.ts @@ -1,6 +1,6 @@ import { defineCommand } from "citty"; import { existsSync, readFileSync } from "node:fs"; -import { dirname, join, resolve } from "node:path"; +import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import type { Example } from "./_examples.js"; import { c } from "../ui/colors.js"; @@ -107,27 +107,10 @@ async function seekTo(page: import("puppeteer-core").Page, time: number): Promis } async function bundleProjectHtml(projectDir: string): Promise { + // `bundleToSingleHtml` now inlines the runtime IIFE by default, so the + // previous post-bundle runtime substitution is no longer needed. const { bundleToSingleHtml } = await import("@hyperframes/core/compiler"); - let html = await bundleToSingleHtml(projectDir); - - const runtimePath = resolve( - __dirname, - "..", - "..", - "..", - "core", - "dist", - "hyperframe.runtime.iife.js", - ); - if (existsSync(runtimePath)) { - const runtimeSource = readFileSync(runtimePath, "utf-8"); - html = html.replace( - /]*data-hyperframes-preview-runtime[^>]*src="[^"]*"[^>]*><\/script>/, - () => ``, - ); - } - - return html; + return bundleToSingleHtml(projectDir); } async function alignViewportToComposition( diff --git a/packages/cli/src/commands/snapshot.ts b/packages/cli/src/commands/snapshot.ts index 2c1d26cce..0fd2a81da 100644 --- a/packages/cli/src/commands/snapshot.ts +++ b/packages/cli/src/commands/snapshot.ts @@ -97,20 +97,9 @@ async function captureSnapshots( const numFrames = opts.frames ?? 5; - // 1. Bundle - let html = await bundleToSingleHtml(projectDir); - - // Inject local runtime if available. - // Uses the same multi-strategy resolver as the studio preview server - // (runtimeSource.ts) so snapshot works in dev (tsx), built CLI, and npx. - const { loadRuntimeSource } = await import("../server/runtimeSource.js"); - const runtimeSource = await loadRuntimeSource(); - if (runtimeSource) { - html = html.replace( - /]*data-hyperframes-preview-runtime[^>]*src="[^"]*"[^>]*><\/script>/, - () => ``, - ); - } + // 1. Bundle. `bundleToSingleHtml` now inlines the runtime IIFE by default, + // so the previous post-bundle runtime substitution is no longer needed. + const html = await bundleToSingleHtml(projectDir); const server = await serveStaticProjectHtml(projectDir, html); diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts index ed2bd7f99..8c9a2fba0 100644 --- a/packages/cli/src/commands/validate.ts +++ b/packages/cli/src/commands/validate.ts @@ -1,6 +1,6 @@ import { defineCommand } from "citty"; import { existsSync, readFileSync } from "node:fs"; -import { resolve, join, dirname } from "node:path"; +import { join, dirname } from "node:path"; import { fileURLToPath } from "node:url"; import { resolveProject } from "../utils/project.js"; import { resolveCompositionViewportFromHtml } from "../utils/compositionViewport.js"; @@ -113,24 +113,10 @@ async function validateInBrowser( const { bundleToSingleHtml } = await import("@hyperframes/core/compiler"); const { ensureBrowser } = await import("../browser/manager.js"); - let html = await bundleToSingleHtml(projectDir); - - const runtimePath = resolve( - __dirname, - "..", - "..", - "..", - "core", - "dist", - "hyperframe.runtime.iife.js", - ); - if (existsSync(runtimePath)) { - const runtimeSource = readFileSync(runtimePath, "utf-8"); - html = html.replace( - /]*data-hyperframes-preview-runtime[^>]*src="[^"]*"[^>]*><\/script>/, - () => ``, - ); - } + // `bundleToSingleHtml` now inlines the runtime IIFE by default, so the + // previous post-bundle regex substitution (which matched `src="..."` on the + // runtime tag) is no longer needed — there's no `src` attribute to match. + const html = await bundleToSingleHtml(projectDir); const { createServer } = await import("node:http"); const { getMimeType } = await import("@hyperframes/core/studio-api"); diff --git a/packages/cli/src/server/studioServer.ts b/packages/cli/src/server/studioServer.ts index ccceccffb..3397d7f33 100644 --- a/packages/cli/src/server/studioServer.ts +++ b/packages/cli/src/server/studioServer.ts @@ -153,8 +153,11 @@ export function createStudioServer(options: StudioServerOptions): StudioServer { async bundle(dir: string): Promise { try { const { bundleToSingleHtml } = await import("@hyperframes/core/compiler"); - let html = await bundleToSingleHtml(dir); - // Fix empty runtime src from bundler — point to the local runtime endpoint + // Studio dev server: ask the bundler for an empty `src=""` placeholder so + // we can point it at our hot-reloadable local runtime endpoint. Inlining + // ~150 KB of runtime body on every preview render would defeat browser + // caching across composition edits. + let html = await bundleToSingleHtml(dir, { runtime: "placeholder" }); html = html.replace( 'data-hyperframes-preview-runtime="1" src=""', 'data-hyperframes-preview-runtime="1" src="/api/runtime.js"', diff --git a/packages/core/src/compiler/compositionScoping.ts b/packages/core/src/compiler/compositionScoping.ts index 5a5030e16..bc7153011 100644 --- a/packages/core/src/compiler/compositionScoping.ts +++ b/packages/core/src/compiler/compositionScoping.ts @@ -286,5 +286,5 @@ ${source} }; __hfFindRoot(); __hfRun(); -})()`; +})();`; } diff --git a/packages/core/src/compiler/htmlBundler.test.ts b/packages/core/src/compiler/htmlBundler.test.ts index d9686b849..225072391 100644 --- a/packages/core/src/compiler/htmlBundler.test.ts +++ b/packages/core/src/compiler/htmlBundler.test.ts @@ -82,6 +82,40 @@ describe("bundleToSingleHtml", () => { expect(innerLength).toBeGreaterThan(1000); }); + it("preserves chunk integrity when a chunk ends with a line comment (ASI hazard guard)", async () => { + // Regression guard for the joinJsChunks helper. If a chunk ends with `// ...` + // and we naively appended `;` on the same line, the appended semicolon would + // be eaten by the comment, leaving the next chunk's first statement attached + // to the previous chunk's last expression. Verify the helper appends `\n;` + // instead so the comment terminates and the semicolon stands alone. + const dir = makeTempProject({ + "index.html": ` + +
+ + + +`, + // Chunk A ends with a // line comment — without the \n separator before + // the appended ;, that ; would be eaten by the comment. + "local-a.js": "window.__a = 1 // trailing line comment", + "local-b.js": "window.__b = 2", + }); + + const bundled = await bundleToSingleHtml(dir); + // Run every inline script body through esbuild; if the line comment ate + // the separator, parse would fail with an unexpected-token error somewhere + // around the chunk boundary. + const { transformSync } = await import("esbuild"); + const re = /]*>([\s\S]*?)<\/script>/g; + let m: RegExpExecArray | null; + while ((m = re.exec(bundled)) !== null) { + const body = m[1]; + if (!body || !body.trim()) continue; + expect(() => transformSync(body, { loader: "js", minify: false })).not.toThrow(); + } + }); + it("does not produce stray bare-semicolon lines between concatenated JS chunks", async () => { // Regression guard: hf#XXX. Earlier the bundler joined script chunks with // `\n;\n`, which produces a lone `;` on its own line between chunks. Valid diff --git a/packages/core/src/compiler/htmlBundler.ts b/packages/core/src/compiler/htmlBundler.ts index 00cae2a0e..def7c8463 100644 --- a/packages/core/src/compiler/htmlBundler.ts +++ b/packages/core/src/compiler/htmlBundler.ts @@ -27,20 +27,24 @@ function getRuntimeScriptUrl(): string { return configured || DEFAULT_RUNTIME_SCRIPT_URL; } -function injectInterceptor(html: string): string { +function injectInterceptor(html: string, runtimeMode: "inline" | "placeholder" = "inline"): string { const sanitized = stripEmbeddedRuntimeScripts(html); if (sanitized.includes(RUNTIME_BOOTSTRAP_ATTR)) return sanitized; - // When a runtime URL is configured (HYPERFRAME_RUNTIME_URL env var), the bundle - // points at it via src=… and the host page serves the script. When no URL is - // configured — the common `bundleToSingleHtml` use case — inline the runtime - // body so the bundle is genuinely self-contained. An empty src="" attribute - // would otherwise resolve to the page URL and trigger an infinite-fetch loop. + // Three modes for the runtime `; + } else if (runtimeMode === "placeholder") { + tag = ``; } else { const inlinedRuntime = getHyperframeRuntimeScript(); tag = ``; @@ -293,16 +297,27 @@ function coalesceHeadStylesAndBodyScripts(document: Document): void { } /** - * Concatenate JS chunks safely. Each chunk gets a trailing `;` if it doesn't - * already end in one, so the joined output never inserts a stray bare-semicolon - * line between chunks (the `\n;\n` separator pattern produces a lone `;` on its - * own line, which is valid JS but reads as a code smell to most linters). + * Concatenate JS chunks safely. Goals: + * - Each chunk's last statement is terminated, so joining can't introduce ASI + * surprises (e.g. `a()` followed by `(b)()` — the second chunk would parse + * as a call on the first's return value). + * - In the common case (chunk already ends with `;` — typical of esbuild + * output and IIFE-wrapped composition scripts ending in `})();`), the join + * produces clean output: chunks separated by `\n` with no stray bare + * semicolon lines. + * - Defensive against trailing line comments. If a chunk ends with `// ...` + * and we appended `;` on the same line, the appended semicolon would be + * swallowed by the comment, leaving the next chunk's first statement + * attached to the previous chunk's last expression — exactly the ASI + * hazard this helper exists to prevent. So when a chunk doesn't already + * end in `;`, we append `\n;` instead — the newline closes any line + * comment, and the standalone `;` becomes the statement separator. */ function joinJsChunks(chunks: string[]): string { return chunks .map((chunk) => chunk.trim()) .filter((chunk) => chunk.length > 0) - .map((chunk) => (chunk.endsWith(";") ? chunk : chunk + ";")) + .map((chunk) => (chunk.endsWith(";") ? chunk : chunk + "\n;")) .join("\n"); } @@ -319,6 +334,22 @@ function stripJsCommentsParserSafe(source: string): string { export interface BundleOptions { /** Optional media duration prober (e.g., ffprobe). If omitted, media durations are not resolved. */ probeMediaDuration?: MediaDurationProber; + /** + * How to handle the HyperFrames runtime ` so the caller can + * substitute it with a real URL via string replace. Used by the dev studio + * server and vite preview to point at a local runtime endpoint, which keeps + * the runtime cacheable across hot-reloads instead of re-inlining ~150 KB + * on every change. + * + * The `HYPERFRAME_RUNTIME_URL` env var, when set, takes precedence over both + * modes and emits `