From 79c43d54cfa55c03bfcc61b4470948f68a2150aa Mon Sep 17 00:00:00 2001 From: Virang Jhaveri Date: Thu, 14 May 2026 16:41:04 +0530 Subject: [PATCH 1/2] fix(studio): preserve active Player ref during cross-fade unmount MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `NLEPreview` renders two `` instances during a refresh cross-fade (retiring + active) sharing the same forwarded `iframeRef`. The retiring Player's cleanup unconditionally nulled the ref, even though the active Player had already claimed it — leaving `iframeRef.current === null`. The null ref breaks the source filter in `useTimelinePlayer`'s message handler (`if (e.source && ourIframe && e.source !== ourIframe.contentWindow) return;`): when `ourIframe` is null the guard short-circuits and every sibling iframe's `hf-preview` postMessage (e.g. the sidebar composition thumbnails in `CompositionsTab`) flows into `processTimelineMessage`. Each sidebar iframe sends its own composition's clip set, so the studio oscillates `setElements` between the manifests of every visible thumbnail — remounting `CompositionThumbnail` on every cycle and visibly flickering the timeline for as long as the Player is on screen. Page refresh hides the bug because there's no retiring Player to clobber the ref. Fix: only null the forwarded ref when it still points at our own iframe. --- .../studio/src/player/components/Player.tsx | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/studio/src/player/components/Player.tsx b/packages/studio/src/player/components/Player.tsx index 6f9f37eda..cd04b5e17 100644 --- a/packages/studio/src/player/components/Player.tsx +++ b/packages/studio/src/player/components/Player.tsx @@ -268,11 +268,18 @@ export const Player = forwardRef( if (assetPollRef.current) clearInterval(assetPollRef.current); assetPollRef.current = null; container.removeChild(player); - // Clear the forwarded ref - if (typeof ref === "function") { - ref(null); - } else if (ref) { - (ref as React.MutableRefObject).current = null; + // Clear the forwarded ref ONLY if it still points at our iframe. + // During the cross-fade in NLEPreview, the retiring Player unmounts + // after the new Player has already claimed the shared ref. Without + // this guard, the retiring Player's cleanup would null out the + // active Player's iframe ref, breaking the source filter on + // hf-preview postMessages and letting sidebar composition iframes' + // timeline messages flood the main player store. + if (typeof ref !== "function" && ref) { + const objectRef = ref as React.MutableRefObject; + if (objectRef.current === iframe) { + objectRef.current = null; + } } }; }); From e81c427a01c5692327e3d0486b05af957965b2fe Mon Sep 17 00:00:00 2001 From: Virang Jhaveri Date: Thu, 14 May 2026 18:40:25 +0530 Subject: [PATCH 2/2] test(studio): cover player ref preservation --- .../src/player/components/Player.test.ts | 136 +++++++++++++++++- 1 file changed, 134 insertions(+), 2 deletions(-) diff --git a/packages/studio/src/player/components/Player.test.ts b/packages/studio/src/player/components/Player.test.ts index 32e106971..07807bef7 100644 --- a/packages/studio/src/player/components/Player.test.ts +++ b/packages/studio/src/player/components/Player.test.ts @@ -1,7 +1,79 @@ // @vitest-environment happy-dom -import { describe, expect, it } from "vitest"; -import { hasUnloadedAssets, shouldShowCompositionLoadingOverlay } from "./Player"; +import React, { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { describe, expect, it, vi } from "vitest"; +import { Player, hasUnloadedAssets, shouldShowCompositionLoadingOverlay } from "./Player"; + +// React 19 warns unless the test environment opts into act(). +globalThis.IS_REACT_ACT_ENVIRONMENT = true; + +vi.mock("@hyperframes/player", () => ({})); + +interface StubHyperframesPlayerElement extends HTMLElement { + iframeElement: HTMLIFrameElement; +} + +function installHyperframesPlayerStub(): { + iframes: HTMLIFrameElement[]; + restore: () => void; +} { + const originalCreateElement = document.createElement.bind(document); + const iframes: HTMLIFrameElement[] = []; + + vi.spyOn(document, "createElement").mockImplementation( + (tagName: string, options?: ElementCreationOptions) => { + if (tagName !== "hyperframes-player") { + return originalCreateElement(tagName, options); + } + + const player = originalCreateElement("div") as StubHyperframesPlayerElement; + const iframe = originalCreateElement("iframe"); + Object.defineProperty(player, "iframeElement", { + configurable: true, + value: iframe, + }); + iframes.push(iframe); + return player; + }, + ); + + return { + iframes, + restore: () => vi.mocked(document.createElement).mockRestore(), + }; +} + +function renderPlayer({ + key, + ref, + onLoad = () => {}, +}: { + key: string; + ref?: React.Ref; + onLoad?: () => void; +}) { + return React.createElement(Player, { + key, + ref, + projectId: "timeline-edit-playground", + onLoad, + }); +} + +async function flushMountedPlayer(): Promise { + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); +} + +async function renderWithAct(root: Root, node: React.ReactNode): Promise { + await act(async () => { + root.render(node); + }); + await flushMountedPlayer(); +} describe("composition loading overlay", () => { it("shows while the composition is loading", () => { @@ -56,3 +128,63 @@ describe("composition loading overlay", () => { iframe.remove(); }); }); + +describe("Player forwarded iframe ref", () => { + it("preserves a shared object ref when a retiring Player unmounts after a new Player claims it", async () => { + const { iframes, restore } = installHyperframesPlayerStub(); + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + const iframeRef = React.createRef(); + + try { + await renderWithAct(root, renderPlayer({ key: "old", ref: iframeRef })); + const oldIframe = iframes[0]; + expect(iframeRef.current).toBe(oldIframe); + + await renderWithAct( + root, + React.createElement( + React.Fragment, + null, + renderPlayer({ key: "old" }), + renderPlayer({ key: "new", ref: iframeRef }), + ), + ); + const newIframe = iframes[1]; + expect(iframeRef.current).toBe(newIframe); + + await renderWithAct(root, renderPlayer({ key: "new", ref: iframeRef })); + + expect(iframeRef.current).toBe(newIframe); + } finally { + await act(async () => { + root.unmount(); + }); + host.remove(); + restore(); + } + }); + + it("clears the object ref when the unmounting Player still owns it", async () => { + const { iframes, restore } = installHyperframesPlayerStub(); + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + const iframeRef = React.createRef(); + + try { + await renderWithAct(root, renderPlayer({ key: "only", ref: iframeRef })); + expect(iframeRef.current).toBe(iframes[0]); + + await act(async () => { + root.unmount(); + }); + + expect(iframeRef.current).toBeNull(); + } finally { + host.remove(); + restore(); + } + }); +});