From c12987e30145205e505b90679321dc2cbcff10a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Mon, 8 Jun 2026 21:10:43 -0400 Subject: [PATCH] fix(producer): revert Proxy-based wrapTimeline to plain-object approach (#1284) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(producer): revert Proxy-based wrapTimeline to plain-object approach The `new Proxy` wrapper for GSAP timelines introduced in #1279 causes Chrome headless to hang indefinitely during page.goto — DOMContentLoaded never fires. The plain-object approach (explicit method allowlist) loads in <800ms on the same composition. The Proxy's generic get/set traps interact badly with Chrome's internal object inspection (Symbol checks, thenable probing, DevTools serialization) during HTML parsing, creating a permanent navigation hang. The maybePublishRenderReady listener fix from #1279 is preserved — only the wrapTimeline implementation is reverted. Compositions using GSAP methods outside the allowlist (eventCallback, labels, repeat, etc.) will see those calls silently dropped rather than forwarded. This is the same behavior as v0.6.81 and earlier. A safer forwarding approach can be explored separately without blocking renders. * fix(producer): address review — stale meta.json descriptions + silently-dropped methods doc - three-boundary: description referenced Proxy fix but the test uses onUpdate in to() vars (allowlist path), not eventCallback - three-boundary-deferred: same — pins Bug 2's deferred-race, not Bug 1 - Add inline doc comment listing silently-dropped GSAP methods and the onUpdate workaround * ci: add page.goto timing canary to CLI smoke test Parse page.goto completion times from the render log and fail if the slowest navigation exceeds 5s. Catches wrapTimeline regressions that block DOMContentLoaded before the 60s timeout fires. Refs: #1285 * fix(producer): forward all GSAP methods via dynamic enumeration at wrap time Instead of silently dropping methods outside a static allowlist, enumerate the real timeline's prototype chain at wrap time and generate plain-object forwarding stubs for every method not already covered. This achieves the same coverage as the `new Proxy` approach from #1279 without the Chrome headless navigation hang — no Proxy trap surfaces are exposed to Chrome internals. Methods prefixed with `_` (GSAP private) are skipped. All forwarded methods flush pending batch operations before delegating, matching the existing allowlist behavior. Closes #1285 * fix(producer): make proxy non-thenable + harden CI canary - Skip `then` in forwardRemainingMethods — GSAP timelines are thenable (tl.then resolves on completion), and forwarding it makes the proxy thenable too: Promise.resolve(proxy) or await proxy hangs forever for paused timelines - Add unit test: Promise.resolve(proxy) resolves immediately, real then() is never called - CI canary: exit 1 (not 0) when no page.goto timing is found in logs, so a log-format change loudly breaks CI instead of silently disabling the canary --- .github/workflows/ci.yml | 18 +- .../src/generated/hf-early-stub-inline.ts | 2 +- .../producer/src/services/fileServer.test.ts | 57 +++++ packages/producer/stubs/hf-early-stub.ts | 198 ++++++++++++------ .../three-boundary-deferred/meta.json | 2 +- .../distributed/three-boundary/meta.json | 2 +- 6 files changed, 208 insertions(+), 71 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 92993fde1..98c952043 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -430,10 +430,26 @@ jobs: --quality standard \ --workers auto \ --strict \ - --output /tmp/hf-cli-inside/renders/inside.mp4 + --output /tmp/hf-cli-inside/renders/inside.mp4 2>&1 | tee /tmp/hf-cli-render.log test -s /tmp/hf-cli-inside/renders/inside.mp4 + - name: Assert page.goto completes under 5s budget + run: | + set -euo pipefail + # Extract the longest page.goto time from render logs. + # Format: [initSession:MODE] page.goto complete (NNNms) + MAX_MS=$(grep -oP 'page\.goto complete \(\K[0-9]+' /tmp/hf-cli-render.log | sort -n | tail -1) + if [ -z "$MAX_MS" ]; then + echo "::error::No page.goto timing found in render logs — log format may have changed. Update the grep pattern." + exit 1 + fi + echo "Slowest page.goto: ${MAX_MS}ms (budget: 5000ms)" + if [ "$MAX_MS" -gt 5000 ]; then + echo "::error::page.goto took ${MAX_MS}ms — exceeds 5s budget. Possible Proxy/stub regression." + exit 1 + fi + - name: Pack CLI tarball run: | set -euo pipefail diff --git a/packages/producer/src/generated/hf-early-stub-inline.ts b/packages/producer/src/generated/hf-early-stub-inline.ts index 1a429e1e9..eb1c1290e 100644 --- a/packages/producer/src/generated/hf-early-stub-inline.ts +++ b/packages/producer/src/generated/hf-early-stub-inline.ts @@ -1,6 +1,6 @@ // AUTO-GENERATED by scripts/build-hf-early-stub.ts — do not edit const HF_EARLY_STUB_IIFE: string = - '"use strict";(()=>{var k=100,T=[],u=[],a=!1,r=!1;function w(n){let e=window.__HF_VIRTUAL_TIME__?.originalRequestAnimationFrame;return typeof e=="function"?e(n):requestAnimationFrame(n)}function p(n){let e=window.__HF_VIRTUAL_TIME__?.originalSetTimeout;if(typeof e=="function"){e(n,0);return}setTimeout(n,0)}function g(n){return n!==null&&typeof n=="object"&&"__hfIsProxy"in n?n.__hfReal:n}function m(n){let e=n.proxy.__hfReal,i=e[n.method];if(typeof i=="function"){let t=n.method==="add"?n.args.map(g):n.args;i.call(e,...t)}}function y(n,e,i){let t={proxy:n,method:e,args:i};return n.__hfQueue.push(t),u.push(t),P(),n}function c(n){let e=n.proxy.__hfQueue.indexOf(n);e>=0&&n.proxy.__hfQueue.splice(e,1)}function f(){for(;u.length>0;){let n=u.shift();n&&(c(n),m(n))}x()}function d(){r=!1,window.__hfTimelinesBuilding=!1;try{window.dispatchEvent(new CustomEvent("hf-timelines-built"))}catch{}}function x(){r||(r=!0,p(()=>{u.length===0?d():r=!1}))}function _(){a=!1;let n=u.splice(0,k);for(let e of n)c(e),m(e);u.length>0?(a=!0,w(_)):d()}function P(){a||(a=!0,window.__hfTimelinesBuilding=!0,w(_))}var O=new Set(["to","from","fromTo","set","add"]),R=new Set(["__hfReal","__hfQueue","__hfIsProxy"]);function v(n,e,i){return(...t)=>{f();let o=e.call(n,...t);return o===n?i():o}}function I(n){let e={__hfReal:n,__hfQueue:[],__hfIsProxy:!0},i=new Proxy(e,{get(t,o,l){if(R.has(o))return e[o];if(typeof o=="string"&&O.has(o))return(...h)=>y(i,o,h);let s=n[o];return typeof s=="function"?v(n,s,()=>l):(s!==void 0&&f(),s)},set(t,o,l){return f(),n[o]=l,!0}});return T.push(i),i}if(typeof window<"u"){window.__hf||(window.__hf={}),window.__hfTimelinesBuilding=!1;let n=null;try{Object.defineProperty(window,"gsap",{configurable:!0,enumerable:!0,get(){return n},set(e){if(n=e,!e||typeof e.timeline!="function")return;let i=e.timeline.bind(e);e.timeline=t=>I(i(t))}})}catch{}}})();\n'; + '"use strict";(()=>{var T=100,_=[],u=[],l=!1,s=!1;function w(n){let i=window.__HF_VIRTUAL_TIME__?.originalRequestAnimationFrame;return typeof i=="function"?i(n):requestAnimationFrame(n)}function y(n){let i=window.__HF_VIRTUAL_TIME__?.originalSetTimeout;if(typeof i=="function"){i(n,0);return}setTimeout(n,0)}function g(n){return n!==null&&typeof n=="object"&&"__hfIsProxy"in n?n.__hfReal:n}function m(n){let i=n.proxy.__hfReal,e=i[n.method];if(typeof e=="function"){let o=n.method==="add"?n.args.map(g):n.args;e.call(i,...o)}}function r(n,i,e){let o={proxy:n,method:i,args:e};return n.__hfQueue.push(o),u.push(o),P(),n}function c(n){let i=n.proxy.__hfQueue.indexOf(n);i>=0&&n.proxy.__hfQueue.splice(i,1)}function t(){for(;u.length>0;){let n=u.shift();n&&(c(n),m(n))}x()}function d(){s=!1,window.__hfTimelinesBuilding=!1;try{window.dispatchEvent(new CustomEvent("hf-timelines-built"))}catch{}}function x(){s||(s=!0,y(()=>{u.length===0?d():s=!1}))}function k(){l=!1;let n=u.splice(0,T);for(let i of n)c(i),m(i);u.length>0?(l=!0,w(k)):d()}function P(){l||(l=!0,window.__hfTimelinesBuilding=!0,w(k))}var O=new Set(["to","from","fromTo","set","add"]);function b(n,i){let e=i;for(;e!==null&&e!==Object.prototype;){for(let o of Object.getOwnPropertyNames(e)){if(o==="constructor"||o==="then"||o in n||O.has(o)||o.charAt(0)==="_")continue;let a=Object.getOwnPropertyDescriptor(e,o);if(!a||typeof a.value!="function")continue;let p=a.value;n[o]=function(...h){t();let f=p.call(i,...h);return f===i?n:f}}e=Object.getPrototypeOf(e)}}function v(n){let i={__hfReal:n,__hfQueue:[],__hfIsProxy:!0,to(...e){return r(i,"to",e)},from(...e){return r(i,"from",e)},fromTo(...e){return r(i,"fromTo",e)},set(...e){return r(i,"set",e)},add(...e){return r(i,"add",e)},pause(...e){return t(),n.pause(...e),i},play(...e){return t(),n.play(...e),i},seek(...e){return t(),n.seek(...e),i},totalTime(...e){return t(),e.length>0?(n.totalTime(...e),i):n.totalTime()},time(...e){return t(),e.length>0?(n.time(...e),i):n.time()},duration(...e){return t(),e.length>0?(n.duration(...e),i):n.duration()},getChildren(...e){t();let o=n.getChildren(...e);return Array.isArray(o)?o:[]},paused(...e){return t(),e.length>0?(n.paused(...e),i):n.paused()},timeScale(...e){return t(),e.length>0?(n.timeScale(...e),i):n.timeScale()},kill(){t(),n.kill()}};return b(i,n),_.push(i),i}if(typeof window<"u"){window.__hf||(window.__hf={}),window.__hfTimelinesBuilding=!1;let n=null;try{Object.defineProperty(window,"gsap",{configurable:!0,enumerable:!0,get(){return n},set(i){if(n=i,!i||typeof i.timeline!="function")return;let e=i.timeline.bind(i);i.timeline=o=>v(e(o))}})}catch{}}})();\n'; /** * Returns the pre-built HyperFrames early stub IIFE as a string constant. diff --git a/packages/producer/src/services/fileServer.test.ts b/packages/producer/src/services/fileServer.test.ts index 6d5af7a99..8172594f8 100644 --- a/packages/producer/src/services/fileServer.test.ts +++ b/packages/producer/src/services/fileServer.test.ts @@ -414,6 +414,63 @@ describe("HF_EARLY_STUB + HF_BRIDGE_SCRIPT integration", () => { expect(sandbox.window.__hfTimelinesBuilding).toBe(false); }); + it("proxy is non-thenable — Promise.resolve(proxy) resolves immediately", async () => { + const sandbox: { + window: Record & { + __hf?: Record; + __hfTimelinesBuilding?: boolean; + gsap?: { timeline: () => Record }; + requestAnimationFrame: typeof requestAnimationFrame; + setTimeout: typeof setTimeout; + }; + document: Record; + CustomEvent: typeof CustomEvent; + } = { + window: { + requestAnimationFrame: (() => 1) as typeof requestAnimationFrame, + setTimeout: (() => 1) as typeof setTimeout, + }, + document: {}, + CustomEvent, + }; + sandbox.window.window = sandbox.window; + sandbox.window.document = sandbox.document; + sandbox.window.CustomEvent = sandbox.CustomEvent; + + new Function("window", "document", "CustomEvent", `with (window) {\n${HF_EARLY_STUB}\n}`)( + sandbox.window, + sandbox.document, + sandbox.CustomEvent, + ); + + sandbox.window.gsap = { + timeline: () => ({ + to: () => {}, + from: () => {}, + fromTo: () => {}, + set: () => {}, + pause: () => {}, + play: () => {}, + seek: () => {}, + totalTime: () => 0, + time: () => 0, + duration: () => 10, + add: () => {}, + getChildren: () => [], + paused: () => true, + timeScale: () => 1, + kill: () => {}, + then: (_resolve: () => void) => { + throw new Error("Real then() was called — proxy is thenable"); + }, + }), + }; + + const timeline = sandbox.window.gsap.timeline(); + const resolved = await Promise.resolve(timeline); + expect(resolved).toBe(timeline); + }); + it("keeps bridge duration at zero until the runtime publishes render readiness", () => { const sandbox: { window: Record & { diff --git a/packages/producer/stubs/hf-early-stub.ts b/packages/producer/stubs/hf-early-stub.ts index d0fa57361..aa2634bef 100644 --- a/packages/producer/stubs/hf-early-stub.ts +++ b/packages/producer/stubs/hf-early-stub.ts @@ -244,82 +244,146 @@ function scheduleBatch(): void { // ─── Timeline proxy factory ─────────────────────────────────────────────────── /** - * Create a queuing proxy around a real GSAP timeline. - * - * All methods return `proxy` so that callers who chain off the returned value - * continue to go through the proxy for the duration of the batching phase. + * Methods queued for rAF-based batch flush (mutating tween additions). + * These return the proxy for chaining and never synchronously flush. */ -// Methods that queue operations for batched flush (mutating tween additions). -const BATCHED_METHODS = new Set(["to", "from", "fromTo", "set", "add"]); -const PROXY_STATE_KEYS = new Set(["__hfReal", "__hfQueue", "__hfIsProxy"]); +const BATCHED_METHODS = new Set(["to", "from", "fromTo", "set", "add"]); -function createFlushingMethodWrapper( - real: GsapTimeline, - fn: (...a: unknown[]) => unknown, - proxyRef: () => unknown, -): (...args: unknown[]) => unknown { - return (...args: unknown[]) => { - flushPendingOperations(); - const result = fn.call(real, ...args); - return result === real ? proxyRef() : result; - }; +/** + * Walk the real timeline's prototype chain and generate forwarding stubs on + * `proxy` for every public method not already present. Each stub flushes + * pending operations, calls the real method, and returns `proxy` when the + * real method returns `this` (for chaining). Private GSAP internals (keys + * starting with `_`) and `then` are skipped — `then` makes GSAP timelines + * thenable, which would cause `Promise.resolve(proxy)` / `await proxy` to + * hang for paused timelines. + */ +// fallow-ignore-next-line complexity +function forwardRemainingMethods(proxy: TimelineProxy, real: GsapTimeline): void { + let obj: object | null = real as object; + while (obj !== null && obj !== Object.prototype) { + for (const key of Object.getOwnPropertyNames(obj)) { + if (key === "constructor" || key === "then" || key in proxy || BATCHED_METHODS.has(key)) + continue; + if (key.charAt(0) === "_") continue; + const desc = Object.getOwnPropertyDescriptor(obj, key); + if (!desc || typeof desc.value !== "function") continue; + const fn = desc.value as (...a: unknown[]) => unknown; + (proxy as Record)[key] = function ( + this: unknown, + ...args: unknown[] + ): unknown { + flushPendingOperations(); + const result = fn.call(real, ...args); + return result === real ? proxy : result; + }; + } + obj = Object.getPrototypeOf(obj); + } } +/** + * Create a queuing proxy around a real GSAP timeline. + * + * Batched methods (to/from/fromTo/set/add) queue operations for rAF flush. + * All other methods on the real timeline are forwarded via dynamically + * generated stubs that flush pending operations before delegating. This + * uses a plain object — not `new Proxy` — because Chrome's headless shell + * hangs when Proxy traps are exposed during page.goto navigation (Symbol + * probing, thenable checks, DevTools serialization). + */ function wrapTimeline(real: GsapTimeline): TimelineProxy { - const state = { + const proxy: TimelineProxy = { __hfReal: real, - __hfQueue: [] as TimelineOperation[], - __hfIsProxy: true as const, + __hfQueue: [], + __hfIsProxy: true, + + to(...args: unknown[]): TimelineProxy { + return enqueueTimelineOperation(proxy, "to", args); + }, + from(...args: unknown[]): TimelineProxy { + return enqueueTimelineOperation(proxy, "from", args); + }, + fromTo(...args: unknown[]): TimelineProxy { + return enqueueTimelineOperation(proxy, "fromTo", args); + }, + set(...args: unknown[]): TimelineProxy { + return enqueueTimelineOperation(proxy, "set", args); + }, + add(...args: unknown[]): TimelineProxy { + return enqueueTimelineOperation(proxy, "add", args); + }, + + pause(...args: unknown[]): TimelineProxy { + flushPendingOperations(); + real.pause(...args); + return proxy; + }, + play(...args: unknown[]): TimelineProxy { + flushPendingOperations(); + real.play(...args); + return proxy; + }, + seek(...args: unknown[]): TimelineProxy { + flushPendingOperations(); + real.seek(...args); + return proxy; + }, + totalTime(...args: unknown[]): unknown { + flushPendingOperations(); + if (args.length > 0) { + real.totalTime(...args); + return proxy; + } + return real.totalTime(); + }, + time(...args: unknown[]): unknown { + flushPendingOperations(); + if (args.length > 0) { + real.time(...args); + return proxy; + } + return real.time(); + }, + duration(...args: unknown[]): unknown { + flushPendingOperations(); + if (args.length > 0) { + real.duration(...args); + return proxy; + } + return real.duration(); + }, + getChildren(...args: unknown[]): unknown[] { + flushPendingOperations(); + const children = real.getChildren(...args); + return Array.isArray(children) ? children : []; + }, + paused(...args: unknown[]): unknown { + flushPendingOperations(); + if (args.length > 0) { + real.paused(...args); + return proxy; + } + return real.paused(); + }, + timeScale(...args: unknown[]): unknown { + flushPendingOperations(); + if (args.length > 0) { + real.timeScale(...args); + return proxy; + } + return real.timeScale(); + }, + kill(): void { + flushPendingOperations(); + real.kill(); + }, }; - // Use a Proxy so that ANY method or property access on the timeline is - // forwarded to the real GSAP timeline after flushing pending operations. - // The previous plain-object approach only forwarded an explicit allowlist, - // which silently dropped calls like eventCallback(), vars, labels(), - // repeat(), and other GSAP API surface — causing black frames when - // compositions used anything outside the allowlist. - const proxy = new Proxy(state, { - // fallow-ignore-next-line complexity - get(_target, prop, receiver) { - if (PROXY_STATE_KEYS.has(prop as string)) { - return state[prop as keyof typeof state]; - } - if (typeof prop === "string" && BATCHED_METHODS.has(prop)) { - return (...args: unknown[]) => - enqueueTimelineOperation( - proxy as unknown as TimelineProxy, - prop as TimelineOperationMethod, - args, - ); - } - const value = (real as Record)[prop]; - // Forwarded methods (getChildren, eventCallback, labels, etc.) flush - // then delegate. Return values are NOT re-proxied — getChildren() - // returns raw GSAP child timelines. This is intentional: batching - // applies to the root timeline only; child timelines returned by - // getChildren are used for enumeration/cleanup, not tween additions. - if (typeof value === "function") { - return createFlushingMethodWrapper( - real, - value as (...a: unknown[]) => unknown, - () => receiver, - ); - } - // Non-function property reads also flush so post-batch reads (e.g. - // tl.vars, tl.data) see state consistent with all applied tweens. - if (value !== undefined) flushPendingOperations(); - return value; - }, + forwardRemainingMethods(proxy, real); - set(_target, prop, value) { - flushPendingOperations(); - (real as Record)[prop] = value; - return true; - }, - }); - - activeProxies.push(proxy as unknown as TimelineProxy); - return proxy as unknown as TimelineProxy; + activeProxies.push(proxy); + return proxy; } // ─── Entry point ───────────────────────────────────────────────────────────── diff --git a/packages/producer/tests/distributed/three-boundary-deferred/meta.json b/packages/producer/tests/distributed/three-boundary-deferred/meta.json index ae937e32d..c02292ff6 100644 --- a/packages/producer/tests/distributed/three-boundary-deferred/meta.json +++ b/packages/producer/tests/distributed/three-boundary-deferred/meta.json @@ -1,6 +1,6 @@ { "name": "Distributed: Three.js WebGL boundary (setTimeout-deferred)", - "description": "Same Three.js purple cube as three-boundary, but the entire GSAP timeline and Three.js setup is deferred via setTimeout(fn, 0) — the exact pattern from issue #1260. Exercises the maybePublishRenderReady race where __hfTimelinesBuilding starts false at DOMContentLoaded then flips true after init.ts has already run. Without the hf-timelines-built re-registration fix, this composition times out with 'Composition has zero duration'.", + "description": "Same Three.js purple cube as three-boundary, but the entire GSAP timeline and Three.js setup is deferred via setTimeout(fn, 0). Exercises the maybePublishRenderReady race where __hfTimelinesBuilding starts false at DOMContentLoaded then flips true after init.ts has already run. Without the hf-timelines-built re-registration fix (#1279), this composition times out with 'Composition has zero duration'.", "tags": ["distributed", "threejs", "webgl", "deferred"], "minPsnr": 25, diff --git a/packages/producer/tests/distributed/three-boundary/meta.json b/packages/producer/tests/distributed/three-boundary/meta.json index 2f9b654a6..d6de03162 100644 --- a/packages/producer/tests/distributed/three-boundary/meta.json +++ b/packages/producer/tests/distributed/three-boundary/meta.json @@ -1,6 +1,6 @@ { "name": "Distributed: Three.js WebGL boundary", - "description": "2s composition (20 frames @ 10fps) with a Three.js purple cube on a dark background. Validates that the GSAP batching proxy correctly forwards WebGL render callbacks and that CDP screenshot capture composites WebGL canvas layers — catches the v0.6.80 regression where the plain-object proxy silently dropped eventCallback() and other non-allowlisted GSAP methods, producing black frames.", + "description": "2s composition (20 frames @ 10fps) with a Three.js purple cube on a dark background. Validates that the GSAP batching proxy correctly handles onUpdate callbacks in to() vars and that CDP screenshot capture composites WebGL canvas layers.", "tags": ["distributed", "threejs", "webgl"], "minPsnr": 25,