mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-07 10:06:21 +00:00
## 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.
79 lines
3.5 KiB
TypeScript
79 lines
3.5 KiB
TypeScript
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*<T>/);
|
|
});
|
|
|
|
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");
|
|
});
|
|
});
|