mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 04:38:33 +00:00
docs(engine): document __name polyfill and add regression test (#385)
## Summary
Document why the `window.__name` polyfill in `frameCapture.ts` is necessary, expand the inline comment with the full per-runtime matrix, and add a regression test that surfaces transpiler behavior on the next failure.
Outcome of the Chunk 12 investigation: **keep the polyfill**.
## Why
`Chunk 12` of `plans/hdr-followups.md`. The polyfill had a vague comment and no test, so it was unclear whether it was still needed or could be deleted.
## Empirical findings
Probe in `/tmp/hf-name-probe`:
| Runtime / build | Injects `__name(fn, "name")` wrappers in `Function.prototype.toString()`? |
|-----------------|---------------------------------------------------------------------------|
| `bun` (TS loader) | No — verified for top-level and nested named functions / arrow expressions. |
| `tsx` (esbuild loader, `keepNames=true`) | **Yes** for nested named functions / arrows; observed crash mode in dev/test. |
| `tsc` (`noEmit` and emit) | No — does not inject the helper. |
| `tsup` for `@hyperframes/cli` (`noExternal: ["@hyperframes/engine"]`) | Polyfill *definition* is bundled, but `__name(...)` *call sites* are absent in `packages/cli/dist/cli.js` (grepped). |
**Root cause.** `@hyperframes/engine`'s `package.json` exports raw TypeScript (`main`/`exports` → `./src/index.ts`), so every consumer's transpiler decides whether to inject `__name`. Anything that runs through `tsx` (producer parity-harness, ad-hoc dev scripts, `bun run --filter @hyperframes/engine test` via Vitest's loader) will serialize wrapped function bodies into `page.evaluate(...)` and crash with `ReferenceError: __name is not defined`.
**Decision.** Keep the no-op `window.__name` shim. Cost is one `evaluateOnNewDocument` call. The alternative (rewriting every `page.evaluate(fn)` site to `page.addScriptTag({ content: "..." })`, like `packages/cli/src/commands/contrast-audit.browser.js` already does) is far more invasive and easy to regress.
## What changed
- Expanded the inline comment in `packages/engine/src/services/frameCapture.ts` to explain the per-runtime matrix above and point to the script-tag alternative.
- New `packages/engine/src/services/frameCapture-namePolyfill.test.ts` — a pure unit test (matches the rest of the engine package's no-browser-launch convention) that:
1. Asserts the polyfill is wired up via `evaluateOnNewDocument` and runs before the first awaited `browser.version()` call.
2. Probes the active Vitest transpiler for `__name(...)` injection so the next maintainer can see at a glance whether the upstream behavior has shifted.
## Test plan
- [x] `bun run --filter @hyperframes/engine test` → 408/408 pass (3 new tests in this file).
- [x] `bunx tsc --noEmit -p packages/engine` clean.
- [x] `bunx oxlint` and `bunx oxfmt --check` clean on edited files.
## Stack
Chunk 12 of `plans/hdr-followups.md`. Independent of all other chunks; closes out the investigation item.
This commit is contained in:
@@ -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?: <T>(fn: T, _name: string) => T };
|
||||
if (typeof w.__name !== "function") {
|
||||
|
||||
Reference in New Issue
Block a user