feat(player): React bindings at @hyperframes/player/react + example app

[tombeckenham]

Summary

• Adds a ./react subpath to @hyperframes/player with typed React bindings — a <HyperframesPlayer> component with camelCase props for every player attribute, callback props for every player event (CustomEvent detail unwrapped), and an imperative ref handle (play()/pause()/seek()/stopMedia(), the color-grading API, and read-only currentTime/duration/paused/ready/scenes/element/iframeElement).
• SSR-safe by construction: the wrapper registers the custom element on mount via a dynamic package self-reference, so the player module (which touches HTMLElement at module scope) never evaluates during server rendering. ensurePlayerDefined() is exported for eager registration. Attributes are synced imperatively because React 18/19 disagree on custom-element boolean props.
• react is an optional peer (^18 || ^19) — plain web-component consumers are unaffected. The react entry builds separately (ESM+CJS, ~2.8 KB) with the self-reference kept external, so consumers' bundlers code-split the 59 KB element bundle and load it lazily.
• Adds examples/react-player (@hyperframes/react-player-example, private): a vite demo app exercising props→attributes toggles, the event log, and the ref-handle transport bar against a bundled copy of the motion-blur registry composition. examples/* joins the workspace globs, the CI code paths filter, and the tracked-examples gitignore negations.
• Player README gains a React section; package-subpaths.json carries the ./react export contract (browser/bun/node — the packed-consumer fixture installs declared peers, so the subpath is exercised in node import, tsc, and vite browser builds).

Test plan

• @hyperframes/player: build, typecheck, and tests pass (337 tests / 12 files, including 7 new binding tests covering attribute mirroring, kebab-case mapping, prop updates/removal, event forwarding, latest-callback semantics without listener rebinding, and ref-handle delegation)
• @hyperframes/react-player-example: vite build, typecheck, and smoke tests pass
• Driven end-to-end in Chrome: ready → play → timeupdate → seek (frame-accurate) → ended → pause, with zero console errors; built-in controls chrome verified alongside the custom ref-handle transport
• check-workspace-contracts, check-package-cycles, check-tracked-artifacts, oxlint, oxfmt all clean

[miguel-heygen]

Exact-head review at da1a1452ee834978c3beb85a72715d7812456dd6.

Audited: packages/player/src/react/{player,register,index}.ts(x) end-to-end; package exports/build/peer setup; React binding tests; the raw player’s attribute/event/lifecycle surface; README React contract.

Trusting: the example app’s visual CSS and bundled 572-line composition fixture (reviewed integration points only).

Strengths

• packages/player/src/react/register.ts:13-22 keeps the DOM-touching player entry behind a dynamic import, and the package self-reference remains external in tsup.config.ts:23-35; that is the right package shape for a framework binding without taxing web-component consumers.
• player.tsx:141-142,197-218 uses a latest-callback ref plus one listener set with symmetric cleanup, avoiding listener churn and stale closures.
• The optional React peer and ./react subpath are preferable to a second package: one player version owns both the element and its binding.

Blockers

packages/player/src/react/player.tsx:13,240-274 — React 18 SSR is not cleanly supported by the current hook

The package claims React 18/19 SSR safety, but the component calls useLayoutEffect directly. React 18 server rendering emits the standard “useLayoutEffect does nothing on the server” warning when this component is rendered through renderToString/a server framework. The existing suite only installs React 19 and never server-renders the component, so it does not verify the advertised React 18 SSR contract.

Use an isomorphic layout effect (useEffect on the server, useLayoutEffect in the browser) and add a React 18 SSR consumer/test that imports @hyperframes/player/react, renders the component, proves the root player module was not evaluated, and asserts no SSR warning/error.

packages/player/src/react/player.tsx:199-218,240-257 — listeners bind after attribute synchronization can already fire events

All layout effects run before passive effects. This wrapper synchronizes attributes in a layout effect, but binds player event listeners in a passive effect. The raw element synchronously dispatches ratechange and volumechange from attributeChangedCallback when playback-rate, volume, or muted is applied (hyperframes-player.ts:233-255,462-475). Those initial events therefore fire before onRateChange/onVolumeChange is attached. srcdoc also begins loading before listeners bind, leaving an avoidable fast-ready race.

Bind listeners in a layout effect declared before attribute synchronization (or combine the two in deterministic order). Add a test double with observedAttributes that dispatches during attributeChangedCallback; the current stub cannot expose this race.

packages/player/src/react/player.tsx:53-72,202-213 — the “every player event” contract drops three real events, including errors

The raw player also emits:

• runtimeprotocolerror (runtime-message-handler.ts:65-71)
• audioownershipchange (parent-media.ts:91-103,252-276)
• playbackerror (parent-media.ts:335-340)

The React binding exposes none of them. In particular, parent-proxy audio failures cannot reach a typed React callback even though this binding advertises callbacks for every player event; consumers must escape through ref.element.addEventListener, defeating the promise. Add typed callback props/listeners/tests for all three, or explicitly declare them private and remove the “every event” claim. playbackerror and runtimeprotocolerror should remain observable either way.

Important

packages/player/src/react/player.tsx:20-72,276-280 accepts only player-specific props plus className/style, then drops normal host attributes. React apps cannot set id, title, role, tabIndex, aria-*, or data-* on the rendered player through this component. A public interactive binding should forward a safe host-attribute surface (or accept an explicit elementProps) while retaining imperative synchronization for player attributes.

Product/API recommendation

Yes, HyperFrames should ship a React binding. The value is not cosmetic JSX: it centralizes SSR-safe registration, presence-based boolean semantics across React 18/19, typed custom-event unwrapping, and the imperative handle. Those are real repeated integration costs, and @hyperframes/player/react is the correct distribution shape. The blockers above are reasons to finish the binding contract before publishing it, not reasons to delete the feature.

Only the WIP check is currently reported at this head; the full CI matrix described in the PR body is not present on GitHub yet.

Verdict: REQUEST CHANGES
Reasoning: The React subpath is worth having, but the first cut does not yet satisfy its advertised React 18 SSR and complete-event contracts, and it has a deterministic listener-order race that drops initial player events.

— Magi

[jrusso1020]

Additive review at da1a1452, concurring with @magi's CHANGES_REQUESTED. I read packages/player/src/react/{player.tsx,register.ts,index.ts} + the test end-to-end and cross-checked the element (hyperframes-player.ts), package exports, and tsup config. I verified Magi's blockers at source rather than restating them, and I'm adding a couple of findings + the product angle.

Strengths (verified, genuinely well done):

• SSR-safety holds at the import-graph level. The /react entry has no static value-import of the element — every reference to hyperframes-player.ts/shader-options.ts is import type (erased), and the only path to the element is the import("@hyperframes/player") inside ensurePlayerDefined() (register.ts:19), guarded by a window/customElements check. createElement("hyperframes-player", {class, style}) emits identical markup on server and client, so there's no hydration mismatch (the imperative attrs are added post-hydration). The package-subpaths.json contract even encodes this — ././slideshow are browser-only while ./react is [browser,bun,node].
• The 18-vs-19 boolean divergence is sidestepped correctly — props go through imperative syncAttribute/syncBooleanAttribute (presence-based) in a layout effect, not JSX; only class/style go through createElement (and class, not className, is right for a custom element).
• Optional-peer + code-split is correct — react is an optional peer, the main . entry carries no react, and the /react build keeps @hyperframes/player external so consumers code-split the element. A plain web-component consumer is genuinely unaffected.

Concurring with @magi's blockers — all four verified:

1. Event-binding race — confirmed, and I can add precision on when it's deterministic. Listeners bind in a passive effect (player.tsx:199), which runs after commit. When the element is already defined (any 2nd+ <HyperframesPlayer>, or after an eager ensurePlayerDefined()), React inserts the tag and it upgrades synchronously during commit — so connectedCallback and the useLayoutEffect attribute application (which triggers attributeChangedCallback → ratechange/volumechange, and a fast srcdoc → ready) all fire before the passive listener effect binds. Those initial events are then deterministically missed. Fix: attach the listeners in a layout effect before the attribute sync (or via the element ref callback), so they're present before any synchronous connect/attr event.
2. "Every player event" is false — confirmed at source. The element re-dispatches three more public events through its dispatchEvent hooks that the wrapper has no callback for: audioownershipchange (parent-media.ts:99,273), playbackerror (parent-media.ts:339), and runtimeprotocolerror (runtime-message-handler.ts:68). Two are the error paths React consumers most need. (shadertransitionstate is covered.)
3. useLayoutEffect SSR warning + no RN18 SSR test — concur; for an SSR-headline feature, useIsomorphicLayoutEffect + an actual renderToString test is the right bar.
4. Host-prop passthrough gap — concur; createElement forwards only class/style, so id/role/tabIndex/aria-*/data-* are dropped. A public component needs a safe forwarding surface (Magi's elementProps is a good shape).

Additive findings @magi didn't raise:

• nit (concurrent-safety): callbacksRef.current = props runs during render (player.tsx:142). The React-recommended latest-ref pattern updates the ref in an effect — a discarded/interrupted render (transitions, Suspense) can momentarily set a ref that never commits, so an event firing in that window would call an uncommitted callback. Move it into a useLayoutEffect(() => { callbacksRef.current = props; }).
• positive verification (worth keeping): autoPlay/loop are synced as attributes even though they're not in the element's observedAttributes — I confirmed this is correct because the element reads them on-demand (get loop() => hasAttribute("loop") at :497; if (hasAttribute("autoplay")) play() at :713/:734), and the useLayoutEffect sets them before the async registration triggers the upgrade. The author clearly understood the element's contract; just don't let a future refactor "fix" these into observedAttributes assumptions.

Product take (James's want/need question): yes, build it — and I'd gate the "yes" on a drift-guard. Concrete adopter evidence: the in-flight hyperframes-cloudflare-template PR #10 already ships a hand-rolled src/components/Player.tsx wrapping <hyperframes-player> (manual src→srcdoc swap, etc.). Adopters are already reinventing this glue and will hit exactly the footguns this PR handles — SSR module-scope eval, 18/19 boolean mapping, CustomEvent-detail unwrapping, and the non-obvious autoplay/loop-via-hasAttribute detail I verified above. A documented snippet would get several of those wrong, so one tested SSR-safe binding is worth owning. But the missing-3-events blocker is the maintenance risk realized: the wrapper's hand-maintained event/attr surface already drifted from the element. So the durable fix isn't just "add 3 events" — it's a contract test asserting the wrapper's event list ⊇ the element's dispatched public events (and prop list ⊇ observed/consumed attrs). That test would have caught this automatically and is what makes "yes" sustainable.

Verdict: COMMENT — concurring with @magi's REQUEST CHANGES. The feature earns its place and the core design (SSR, 18/19, code-split) is right; finish the contract (Magi's four items + the render-time ref nit + a surface-drift test) and it's a clear ship.

— Jerrai