mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 04:38:33 +00:00
fix(bundler): runtime mode opt-in, ASI-safe joinJsChunks, prune dead subs
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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
af2f727b3f
commit
dfca302d37
@@ -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<string> {
|
||||
// `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(
|
||||
/<script[^>]*data-hyperframes-preview-runtime[^>]*src="[^"]*"[^>]*><\/script>/,
|
||||
() => `<script data-hyperframes-preview-runtime="1">${runtimeSource}</script>`,
|
||||
);
|
||||
}
|
||||
|
||||
return html;
|
||||
return bundleToSingleHtml(projectDir);
|
||||
}
|
||||
|
||||
async function alignViewportToComposition(
|
||||
|
||||
@@ -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(
|
||||
/<script[^>]*data-hyperframes-preview-runtime[^>]*src="[^"]*"[^>]*><\/script>/,
|
||||
() => `<script data-hyperframes-preview-runtime="1">${runtimeSource}</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);
|
||||
|
||||
|
||||
@@ -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(
|
||||
/<script[^>]*data-hyperframes-preview-runtime[^>]*src="[^"]*"[^>]*><\/script>/,
|
||||
() => `<script data-hyperframes-preview-runtime="1">${runtimeSource}</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");
|
||||
|
||||
@@ -153,8 +153,11 @@ export function createStudioServer(options: StudioServerOptions): StudioServer {
|
||||
async bundle(dir: string): Promise<string | null> {
|
||||
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"',
|
||||
|
||||
@@ -286,5 +286,5 @@ ${source}
|
||||
};
|
||||
__hfFindRoot();
|
||||
__hfRun();
|
||||
})()`;
|
||||
})();`;
|
||||
}
|
||||
|
||||
@@ -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": `<!doctype html>
|
||||
<html><body>
|
||||
<div data-composition-id="root" data-width="320" data-height="180"></div>
|
||||
<script src="local-a.js"></script>
|
||||
<script src="local-b.js"></script>
|
||||
<script>window.__timelines = window.__timelines || {}; window.__timelines.root = {}</script>
|
||||
</body></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 = /<script\b[^>]*>([\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
|
||||
|
||||
@@ -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 <script>:
|
||||
// 1. HYPERFRAME_RUNTIME_URL env var set → emit src="<url>" (production CDN deploy).
|
||||
// 2. runtime: "placeholder" passed → emit src="" for the caller to substitute
|
||||
// (studio + vite preview hot-load a local
|
||||
// runtime endpoint via string replace).
|
||||
// 3. runtime: "inline" (default) → embed the IIFE body directly so the
|
||||
// bundle is genuinely self-contained.
|
||||
const runtimeScriptUrl = getRuntimeScriptUrl();
|
||||
let tag: string;
|
||||
if (runtimeScriptUrl) {
|
||||
const escaped = runtimeScriptUrl.replace(/"/g, """);
|
||||
tag = `<script ${RUNTIME_BOOTSTRAP_ATTR}="1" src="${escaped}"></script>`;
|
||||
} else if (runtimeMode === "placeholder") {
|
||||
tag = `<script ${RUNTIME_BOOTSTRAP_ATTR}="1" src=""></script>`;
|
||||
} else {
|
||||
const inlinedRuntime = getHyperframeRuntimeScript();
|
||||
tag = `<script ${RUNTIME_BOOTSTRAP_ATTR}="1">${inlinedRuntime}</script>`;
|
||||
@@ -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 <script> tag. Default: `"inline"`.
|
||||
*
|
||||
* - `"inline"` — embed the runtime IIFE body directly into the bundle. Produces
|
||||
* genuinely self-contained HTML. Right for CLI render output, validate,
|
||||
* snapshot, and any "ship a single .html file" use case.
|
||||
* - `"placeholder"` — emit `<script ... src=""></script>` 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 `<script ... src="<URL>">` directly.
|
||||
*/
|
||||
runtime?: "inline" | "placeholder";
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -347,7 +378,7 @@ export async function bundleToSingleHtml(
|
||||
);
|
||||
}
|
||||
|
||||
const withInterceptor = injectInterceptor(compiled);
|
||||
const withInterceptor = injectInterceptor(compiled, options?.runtime ?? "inline");
|
||||
const document = parseHTMLContent(withInterceptor);
|
||||
|
||||
// Inline local CSS
|
||||
|
||||
@@ -60,7 +60,9 @@ interface ScreenshotClip {
|
||||
|
||||
function createViteAdapter(dataDir: string, server: ViteDevServer): StudioApiAdapter {
|
||||
// Lazy-load the bundler via Vite's SSR module loader
|
||||
let _bundler: ((dir: string) => Promise<string>) | null = null;
|
||||
let _bundler:
|
||||
| ((dir: string, options?: { runtime?: "inline" | "placeholder" }) => Promise<string>)
|
||||
| null = null;
|
||||
let _producerModulePromise: Promise<{
|
||||
createRenderJob: (config: {
|
||||
fps: 24 | 30 | 60;
|
||||
@@ -78,7 +80,7 @@ function createViteAdapter(dataDir: string, server: ViteDevServer): StudioApiAda
|
||||
if (!_bundler) {
|
||||
try {
|
||||
const mod = await server.ssrLoadModule("@hyperframes/core/compiler");
|
||||
_bundler = (dir: string) => mod.bundleToSingleHtml(dir);
|
||||
_bundler = (dir, options) => mod.bundleToSingleHtml(dir, options);
|
||||
} catch (err) {
|
||||
console.warn("[Studio] Failed to load compiler, previews will use raw HTML:", err);
|
||||
_bundler = null as never;
|
||||
@@ -171,8 +173,10 @@ function createViteAdapter(dataDir: string, server: ViteDevServer): StudioApiAda
|
||||
async bundle(dir: string) {
|
||||
const bundler = await getBundler();
|
||||
if (!bundler) return null;
|
||||
let html = await bundler(dir);
|
||||
// Fix empty runtime src from bundler — point to the CDN runtime
|
||||
// Studio vite preview: bundler emits an empty `src=""` placeholder so we
|
||||
// can point it at the local /api/runtime.js endpoint. Cached by the browser
|
||||
// across composition hot-reloads instead of being inlined fresh each time.
|
||||
let html = await bundler(dir, { runtime: "placeholder" });
|
||||
html = html.replace(
|
||||
'data-hyperframes-preview-runtime="1" src=""',
|
||||
`data-hyperframes-preview-runtime="1" src="${this.runtimeUrl}"`,
|
||||
|
||||
Reference in New Issue
Block a user