From 8d2330c0c2b210eef1fa322ab1f6190407a9414b Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Tue, 21 Apr 2026 10:51:32 -0700 Subject: [PATCH] docs(engine): document __name polyfill and add regression test Investigation under HDR follow-ups Chunk 12. The window.__name shim in frameCapture.ts is necessary because @hyperframes/engine ships raw TypeScript and consumers' transpilers may inject __name(fn,"name") wrappers around named functions. When Puppeteer serializes a page.evaluate(callback) via Function.prototype.toString(), those wrappers travel into the browser and crash with ReferenceError: __name is not defined. Empirical findings (probe in /tmp/hf-name-probe): - bun TS loader: does not inject __name. - tsx (esbuild loader, keepNames=true): injects __name; this is the observed crash mode in dev/test (parity-harness, ad-hoc scripts, Vitest under bun run --filter @hyperframes/engine test). - tsc: does not inject. - tsup for @hyperframes/cli with noExternal: [@hyperframes/engine]: bundles the polyfill definition but emits no __name(...) call sites in dist/cli.js (verified by grep). Root cause: engine package.json points main/exports at ./src/index.ts, so each consumer's transpiler decides whether __name appears. Anything routed through tsx triggers the crash; the polyfill makes it a no-op. Decision: keep the shim. It's a single evaluateOnNewDocument call. The alternative (rewriting every page.evaluate(fn) site to page.addScriptTag with raw text, like contrast-audit.browser.js) is significantly more invasive. Changes: - Expand the inline comment in frameCapture.ts so future maintainers see the full per-runtime matrix and the script-tag alternative. - Add frameCapture-namePolyfill.test.ts: a pure unit test (matches the rest of the engine package's no-browser-launch convention) that asserts the polyfill is wired up, runs before the first awaited browser.version(), and probes the active transpiler so the next maintainer can see at a glance whether the upstream behavior has shifted. Verified: - bun run --filter @hyperframes/engine test -> 408/408 pass. - bunx tsc --noEmit -p packages/engine clean. - bunx oxlint and bunx oxfmt --check clean. --- .../frameCapture-namePolyfill.test.ts | 78 +++++++++++++++++++ packages/engine/src/services/frameCapture.ts | 33 ++++++-- 2 files changed, 104 insertions(+), 7 deletions(-) create mode 100644 packages/engine/src/services/frameCapture-namePolyfill.test.ts diff --git a/packages/engine/src/services/frameCapture-namePolyfill.test.ts b/packages/engine/src/services/frameCapture-namePolyfill.test.ts new file mode 100644 index 000000000..b92f8f345 --- /dev/null +++ b/packages/engine/src/services/frameCapture-namePolyfill.test.ts @@ -0,0 +1,78 @@ +import { describe, it, expect } from "vitest"; +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { dirname, resolve } from "node:path"; + +// Regression coverage for the `window.__name` no-op shim that +// `frameCapture.ts` registers via `page.evaluateOnNewDocument`. +// +// Background: `@hyperframes/engine` ships raw TypeScript (see +// `packages/engine/package.json` — main and exports both point at +// `./src/index.ts`). Downstream transpilers like tsx run esbuild with +// keepNames=true, which wraps named functions in `__name(fn, "name")` +// calls. When Puppeteer serializes a `page.evaluate(callback)` argument +// via `Function.prototype.toString()`, those wrappers travel into the +// browser and throw `ReferenceError: __name is not defined` unless we +// install a no-op shim first. +// +// These tests intentionally do NOT launch a browser — the rest of this +// package follows the same pure-unit-test convention. Instead they: +// 1. Assert the polyfill is wired up at the source level so it cannot +// be silently removed by a careless edit. +// 2. Probe the current Vitest runtime so a future maintainer can see at +// a glance whether nested named functions still get `__name(...)` +// wrappers under the test transformer. This is advisory: both +// outcomes are acceptable — the reported observation is what makes +// the test useful when the upstream behavior shifts. + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const FRAME_CAPTURE_PATH = resolve(__dirname, "frameCapture.ts"); + +describe("frameCapture __name polyfill", () => { + it("registers a window.__name shim via evaluateOnNewDocument", () => { + const source = readFileSync(FRAME_CAPTURE_PATH, "utf-8"); + + expect(source).toMatch(/page\.evaluateOnNewDocument\(/); + expect(source).toMatch(/typeof w\.__name !== "function"/); + expect(source).toMatch(/w\.__name\s*=\s*/); + }); + + it("installs the shim before any awaited browser-version checks", () => { + const source = readFileSync(FRAME_CAPTURE_PATH, "utf-8"); + + const polyfillIndex = source.indexOf("page.evaluateOnNewDocument("); + const versionIndex = source.indexOf("await browser.version()"); + + expect(polyfillIndex).toBeGreaterThan(-1); + expect(versionIndex).toBeGreaterThan(-1); + expect(polyfillIndex).toBeLessThan(versionIndex); + }); + + it("documents the current transpiler behavior for nested named functions", () => { + function outer(): { wrapsNested: boolean; wrapsArrow: boolean } { + // The unused declarations are deliberate: we are inspecting whether the + // active transpiler rewrites `outer.toString()` to include + // `__name(nested, ...)` / `__name(arrowNested, ...)` wrappers. + // eslint-disable-next-line no-unused-vars + function nested() { + return 1; + } + // eslint-disable-next-line no-unused-vars + const arrowNested = () => 2; + const src = outer.toString(); + return { + wrapsNested: /__name\(\s*nested\s*,/.test(src), + wrapsArrow: /__name\(\s*\(\)\s*=>\s*2\s*,/.test(src) || /__name\(\s*arrowNested/.test(src), + }; + } + + const { wrapsNested, wrapsArrow } = outer(); + + // Both outcomes are acceptable; the value of this test is in surfacing + // the runtime's behavior on the next failure (or first inspection). + // If both flags become false everywhere this engine is consumed, the + // polyfill above can probably be dropped. Until then it stays. + expect(typeof wrapsNested).toBe("boolean"); + expect(typeof wrapsArrow).toBe("boolean"); + }); +}); diff --git a/packages/engine/src/services/frameCapture.ts b/packages/engine/src/services/frameCapture.ts index 16b12ecbf..1a0ba2f2d 100644 --- a/packages/engine/src/services/frameCapture.ts +++ b/packages/engine/src/services/frameCapture.ts @@ -92,13 +92,32 @@ export async function createCaptureSession( const { browser, captureMode } = await acquireBrowser(chromeArgs, config); const page = await browser.newPage(); - // Polyfill esbuild's keepNames helper inside the page. Tools like tsx/Bun - // transform this engine's source on the fly and wrap every named function - // with `__name(fn, "name")`. When `page.evaluate()` serializes a callback - // and ships it to the browser, those `__name(...)` calls would crash with - // `__name is not defined` because the helper only exists in Node. Defining - // a no-op shim once per page makes the engine work uniformly whether it is - // imported from compiled dist (no helper) or from source via tsx. + // Polyfill esbuild's keepNames helper inside the page. + // + // The engine is published as raw TypeScript (`packages/engine/package.json` + // points `main`/`exports` at `./src/index.ts`) and downstream consumers + // execute it through transpilers that may inject `__name(fn, "name")` + // wrappers around named functions. Empirically, this happens with: + // - tsx (its esbuild loader runs with keepNames=true), used by the + // producer's parity-harness, ad-hoc dev scripts, and the + // `bun run --filter @hyperframes/engine test` Vitest path. + // - any tsup/esbuild build that explicitly enables keepNames. + // + // The HeyGen CLI (`packages/cli`) bundles this engine via tsup with + // keepNames left at its default (false) — verified by grepping + // `packages/cli/dist/cli.js`, where `__name(...)` call sites are absent. + // Bun's TS loader also does not currently inject `__name`. Even so, + // anything that calls `page.evaluate(fn)` with a nested named function + // under tsx (most local development and tests) will serialize bodies + // like `__name(nested,"nested")` and crash with `__name is not defined` + // in the browser. The shim makes such calls a no-op. + // + // An alternative is to load browser-side code as raw text and inject it + // via `page.addScriptTag({ content: ... })` — see + // `packages/cli/src/commands/contrast-audit.browser.js` for that pattern. + // Until every `page.evaluate(fn)` call site migrates, this polyfill is + // the single line of defense. The companion regression test in + // `frameCapture-namePolyfill.test.ts` verifies the shim stays wired up. await page.evaluateOnNewDocument(() => { const w = window as unknown as { __name?: (fn: T, _name: string) => T }; if (typeof w.__name !== "function") {