diff --git a/packages/studio/src/hooks/useLintModal.test.tsx b/packages/studio/src/hooks/useLintModal.test.tsx new file mode 100644 index 0000000000..e746d850c6 --- /dev/null +++ b/packages/studio/src/hooks/useLintModal.test.tsx @@ -0,0 +1,105 @@ +// @vitest-environment happy-dom + +import React, { act } from "react"; +import { createRoot } from "react-dom/client"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { useLintModal } from "./useLintModal"; + +Reflect.set(globalThis, "IS_REACT_ACT_ENVIRONMENT", true); + +function mountLintModal(projectId: string | null) { + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + let current: ReturnType | null = null; + + function Harness() { + current = useLintModal(projectId); + return null; + } + + act(() => root.render(React.createElement(Harness))); + return { + read: () => { + if (!current) throw new Error("useLintModal did not render"); + return current; + }, + unmount: () => act(() => root.unmount()), + }; +} + +afterEach(() => { + vi.unstubAllGlobals(); + document.body.innerHTML = ""; +}); + +describe("useLintModal", () => { + it("clears the in-progress flag after a lint run succeeds", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async () => ({ + json: async () => ({ + findings: [{ severity: "error", message: "missing clip", elementId: "hf-1" }], + }), + })), + ); + const harness = mountLintModal("demo"); + + await act(async () => { + harness.read().handleLint(); + }); + + expect(harness.read().linting).toBe(false); + expect(harness.read().lintModal).toEqual([ + { + severity: "error", + message: "missing clip", + file: undefined, + fixHint: undefined, + elementId: "hf-1", + }, + ]); + harness.unmount(); + }); + + it("clears the in-progress flag and reports the failure when the request throws", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async () => { + throw new Error("network down"); + }), + ); + const harness = mountLintModal("demo"); + + await act(async () => { + harness.read().handleLint(); + }); + + // The teardown the removed `finally` clause used to own: a failed run must + // not leave the button spinning. + expect(harness.read().linting).toBe(false); + expect(harness.read().lintModal).toEqual([ + { severity: "error", message: "Failed to run lint: network down" }, + ]); + harness.unmount(); + }); + + it("leaves the flag alone for a background run and does not open the modal", async () => { + const fetchMock = vi.fn(async () => ({ + json: async () => ({ findings: [{ severity: "warning", message: "slow clip" }] }), + })); + vi.stubGlobal("fetch", fetchMock); + const harness = mountLintModal("demo"); + + // Mounting fires the automatic background run. + await act(async () => { + await Promise.resolve(); + }); + + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(harness.read().linting).toBe(false); + expect(harness.read().lintModal).toBeNull(); + expect(harness.read().backgroundFindings).toHaveLength(1); + harness.unmount(); + }); +}); diff --git a/packages/studio/src/hooks/useLintModal.ts b/packages/studio/src/hooks/useLintModal.ts index 399fb75957..fc0e19bc13 100644 --- a/packages/studio/src/hooks/useLintModal.ts +++ b/packages/studio/src/hooks/useLintModal.ts @@ -49,9 +49,11 @@ export function useLintModal(projectId: string | null, refreshKey?: number) { const msg = err instanceof Error ? err.message : String(err); setLintModal([{ severity: "error", message: `Failed to run lint: ${msg}` }]); } - } finally { - if (!opts?.background) setLinting(false); } + // Reached on both paths — the catch above swallows the failure rather than + // rethrowing it, so this is what the `finally` clause it replaces did. The + // React Compiler declines any function with a `finally`. + if (!opts?.background) setLinting(false); }, [projectId], ); diff --git a/packages/studio/src/hooks/useLivePlayheadTime.test.tsx b/packages/studio/src/hooks/useLivePlayheadTime.test.tsx new file mode 100644 index 0000000000..3e2849c742 --- /dev/null +++ b/packages/studio/src/hooks/useLivePlayheadTime.test.tsx @@ -0,0 +1,105 @@ +// @vitest-environment happy-dom +import React, { act } from "react"; +import { createRoot } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { liveTime, usePlayerStore } from "../player/store/playerStore"; +import { useLivePlayheadTime } from "./useLivePlayheadTime"; + +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +/** Past the hook's 33ms throttle. */ +const PAST_THROTTLE_MS = 40; + +function mountReadout() { + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + let current = Number.NaN; + + function Readout() { + current = useLivePlayheadTime(); + return null; + } + + act(() => root.render(React.createElement(Readout))); + return { + read: () => current, + unmount: () => act(() => root.unmount()), + }; +} + +function setTransport(currentTime: number, isPlaying: boolean) { + act(() => usePlayerStore.setState({ currentTime, isPlaying })); +} + +beforeEach(() => { + vi.useFakeTimers(); + usePlayerStore.setState({ currentTime: 0, isPlaying: false }); +}); + +afterEach(() => { + vi.useRealTimers(); + document.body.innerHTML = ""; +}); + +describe("useLivePlayheadTime", () => { + it("reports the store's time while paused, including after a seek", () => { + const readout = mountReadout(); + expect(readout.read()).toBe(0); + + setTransport(4.25, false); + expect(readout.read()).toBe(4.25); + + readout.unmount(); + }); + + it("ignores live notifications while paused", () => { + const readout = mountReadout(); + setTransport(2, false); + + act(() => { + liveTime.notify(9); + vi.advanceTimersByTime(PAST_THROTTLE_MS); + }); + + expect(readout.read()).toBe(2); + readout.unmount(); + }); + + it("follows live notifications while playing, throttled", () => { + const readout = mountReadout(); + setTransport(1, true); + + act(() => liveTime.notify(1.1)); + // Inside the throttle window nothing has been published yet. + expect(readout.read()).toBe(1); + + act(() => { + liveTime.notify(1.4); + vi.advanceTimersByTime(PAST_THROTTLE_MS); + }); + // The flush publishes the newest value seen, not the one that armed it. + expect(readout.read()).toBe(1.4); + + readout.unmount(); + }); + + it("does not show the previous run's live time on the first frame of a new one", () => { + const readout = mountReadout(); + setTransport(1, true); + act(() => { + liveTime.notify(7.5); + vi.advanceTimersByTime(PAST_THROTTLE_MS); + }); + expect(readout.read()).toBe(7.5); + + // Pause, seek back to the top, play again: the readout must start from the + // store, not from where the last run stopped. + setTransport(7.5, false); + setTransport(0, false); + setTransport(0, true); + expect(readout.read()).toBe(0); + + readout.unmount(); + }); +}); diff --git a/packages/studio/src/hooks/useLivePlayheadTime.ts b/packages/studio/src/hooks/useLivePlayheadTime.ts index af7adcd706..45f4b12b5d 100644 --- a/packages/studio/src/hooks/useLivePlayheadTime.ts +++ b/packages/studio/src/hooks/useLivePlayheadTime.ts @@ -11,7 +11,7 @@ * that instead, which is what lets a readout follow the playhead while it is being * dragged as well as while it is playing. */ -import { useEffect, useRef, useState } from "react"; +import { useEffect, useState } from "react"; // The store's own module, not the `player` barrel: the barrel pulls the whole // timeline in, and a timeline component importing this hook closes a cycle. import { liveTime, usePlayerStore } from "../player/store/playerStore"; @@ -22,30 +22,31 @@ const THROTTLE_MS = 33; export function useLivePlayheadTime(): number { const storeTime = usePlayerStore((s) => s.currentTime); const isPlaying = usePlayerStore((s) => s.isPlaying); - const liveRef = useRef(storeTime); - const [, forceRender] = useState(0); - - // Paused, the ref tracks the store so the first frame of playback is never a - // stale value from the last time the transport ran. - if (!isPlaying) liveRef.current = storeTime; + // Null means "nothing heard from this playback run yet", which is why the + // subscription clears it on teardown: the first frame after the transport + // starts must never show a time left over from the last time it ran, and the + // store is the truth up to that point anyway. + const [runTime, setRunTime] = useState(null); useEffect(() => { if (!isPlaying) return; + let latest: number | null = null; let timerId: ReturnType | 0 = 0; const unsubscribe = liveTime.subscribe((t) => { - liveRef.current = t; + latest = t; if (!timerId) { timerId = setTimeout(() => { timerId = 0; - forceRender((v) => v + 1); + setRunTime(latest); }, THROTTLE_MS); } }); return () => { unsubscribe(); if (timerId) clearTimeout(timerId); + setRunTime(null); }; }, [isPlaying]); - return isPlaying ? liveRef.current : storeTime; + return isPlaying ? (runTime ?? storeTime) : storeTime; } diff --git a/packages/studio/src/hooks/useMountEffect.ts b/packages/studio/src/hooks/useMountEffect.ts index 28e12612f9..4947df63f6 100644 --- a/packages/studio/src/hooks/useMountEffect.ts +++ b/packages/studio/src/hooks/useMountEffect.ts @@ -1,4 +1,4 @@ -import { useEffect } from "react"; +import { useEffect, useRef } from "react"; /** * Run an effect exactly once on mount (and optional cleanup on unmount). @@ -13,6 +13,11 @@ import { useEffect } from "react"; * @see https://react.dev/learn/you-might-not-need-an-effect */ export function useMountEffect(effect: () => void | (() => void)) { - // eslint-disable-next-line react-hooks/exhaustive-deps - useEffect(effect, []); + // `useEffect(effect, [])` needed a suppression because `effect` is a new + // closure every render and the empty list says so. Holding the mount-time + // closure in a ref makes the list honest without changing which closure runs: + // `useRef` keeps its initial value, so this is still the first render's + // `effect`, called once, with its return value used as the unmount cleanup. + const mountEffect = useRef(effect); + useEffect(() => mountEffect.current(), []); } diff --git a/packages/studio/src/hooks/useMusicBeatAnalysis.test.tsx b/packages/studio/src/hooks/useMusicBeatAnalysis.test.tsx new file mode 100644 index 0000000000..0ab5527182 --- /dev/null +++ b/packages/studio/src/hooks/useMusicBeatAnalysis.test.tsx @@ -0,0 +1,108 @@ +// @vitest-environment happy-dom + +import React, { act } from "react"; +import { createRoot } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { FileManagerProvider } from "../contexts/FileManagerContext"; +import { usePlayerStore, type TimelineElement } from "../player/store/playerStore"; +import { useMusicBeatAnalysis } from "./useMusicBeatAnalysis"; + +Reflect.set(globalThis, "IS_REACT_ACT_ENVIRONMENT", true); + +function musicElement(): TimelineElement { + return { + id: "music-1", + domId: "music-1", + tag: "audio", + src: "audio/track.mp3", + timelineRole: "music", + start: 0, + duration: 30, + } as unknown as TimelineElement; +} + +/** + * The hook reads its file IO out of this context. Only the two IO functions + * matter here, so the rest of the file-manager surface is left off. + */ +function mountWithIo(io: { + readOptionalProjectFile: (path: string) => Promise; + writeProjectFile: (path: string, content: string) => Promise; +}) { + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + + function Harness() { + useMusicBeatAnalysis(); + return null; + } + + const value = io as unknown as React.ComponentProps["value"]; + act(() => + root.render( + + + , + ), + ); + return { unmount: () => act(() => root.unmount()) }; +} + +beforeEach(() => { + usePlayerStore.setState({ elements: [musicElement()] }); +}); + +afterEach(() => { + usePlayerStore.setState({ elements: [] }); + document.body.innerHTML = ""; +}); + +describe("useMusicBeatAnalysis", () => { + it("reaches the project's file IO on the first commit", async () => { + const readOptionalProjectFile = vi.fn(async () => ""); + const writeProjectFile = vi.fn(async () => {}); + + // The IO is carried in a ref that used to be written during render. It is + // written in an effect now, and that effect is declared before the loader, + // so the loader still finds it on the very first commit. If it did not, the + // hook would take the "no IO" branch and never read the beats file. + const harness = mountWithIo({ readOptionalProjectFile, writeProjectFile }); + await act(async () => { + await Promise.resolve(); + }); + + expect(readOptionalProjectFile).toHaveBeenCalledWith(expect.stringContaining("track")); + harness.unmount(); + }); + + it("registers a beat writer that persists through the same IO", async () => { + const readOptionalProjectFile = vi.fn(async () => ""); + const writeProjectFile = vi.fn(async () => {}); + const harness = mountWithIo({ readOptionalProjectFile, writeProjectFile }); + + await act(async () => { + await Promise.resolve(); + }); + + expect(usePlayerStore.getState().beatPersist).toBeTypeOf("function"); + harness.unmount(); + }); + + it("registers no beat writer when the project has no file IO", async () => { + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + function Harness() { + useMusicBeatAnalysis(); + return null; + } + act(() => root.render(React.createElement(Harness))); + await act(async () => { + await Promise.resolve(); + }); + + expect(usePlayerStore.getState().beatPersist).toBeNull(); + act(() => root.unmount()); + }); +}); diff --git a/packages/studio/src/hooks/useMusicBeatAnalysis.ts b/packages/studio/src/hooks/useMusicBeatAnalysis.ts index 8d0f2dc585..91ec32c67a 100644 --- a/packages/studio/src/hooks/useMusicBeatAnalysis.ts +++ b/packages/studio/src/hooks/useMusicBeatAnalysis.ts @@ -117,10 +117,15 @@ export function useMusicBeatAnalysis(): void { const ioRef = useRef< (ProjectIo & { writeProjectFile: (p: string, c: string) => Promise }) | null >(null); - ioRef.current = - readOptionalProjectFile && writeProjectFile - ? { readOptionalProjectFile, writeProjectFile } - : null; + // Refreshed on commit, not during render. Declared above the two effects + // below so it runs first on every commit, which is what lets them keep + // reading `ioRef.current` at the moment they run. + useEffect(() => { + ioRef.current = + readOptionalProjectFile && writeProjectFile + ? { readOptionalProjectFile, writeProjectFile } + : null; + }); const { musicSrc, isFallbackTrack } = useMemo(() => { const resolved = resolveBeatSourceTrack(elements); diff --git a/packages/studio/src/hooks/usePanelLayout.ts b/packages/studio/src/hooks/usePanelLayout.ts index 692775acae..748b1210e9 100644 --- a/packages/studio/src/hooks/usePanelLayout.ts +++ b/packages/studio/src/hooks/usePanelLayout.ts @@ -57,12 +57,10 @@ export function usePanelLayout(initialState?: InitialPanelLayoutState) { initialState?.rightPanelTab ?? "design", ); const rightPanelTabRef = useRef(rightPanelTab); - rightPanelTabRef.current = rightPanelTab; const [rightInspectorPanes, setRightInspectorPanes] = useState(() => getInitialRightInspectorPanes(initialState?.rightPanelTab), ); const rightInspectorPanesRef = useRef(rightInspectorPanes); - rightInspectorPanesRef.current = rightInspectorPanes; // Set when the user explicitly reopens a panel the window had auto-collapsed, // so the rail cannot immediately swallow it again. Cleared once the window is // wide enough that auto-collapse is no longer in play. @@ -96,7 +94,6 @@ export function usePanelLayout(initialState?: InitialPanelLayoutState) { // Rendered widths, which the drag handles measure from so the seam does not // jump when a panel is currently narrower than its stored preference. const fittedRef = useRef(fitted); - fittedRef.current = fitted; const panelDragRef = useRef<{ side: PanelSide; @@ -152,8 +149,7 @@ export function usePanelLayout(initialState?: InitialPanelLayoutState) { // auto-collapsed state: intent was already false, so the click flipped it to // true (persisting a collapse the user never asked for) while the rail stayed // railed and nothing visibly happened. - const effectiveLeftCollapsedRef = useRef(false); - effectiveLeftCollapsedRef.current = leftCollapsed || leftCollapsedByWidth; + const effectiveLeftCollapsedRef = useRef(leftCollapsed || leftCollapsedByWidth); const toggleLeftSidebar = useCallback(() => { const next = !effectiveLeftCollapsedRef.current; @@ -245,6 +241,20 @@ export function usePanelLayout(initialState?: InitialPanelLayoutState) { setRightInspectorPanes({ design: pane === "design", layers: pane === "layers" }); }, []); + // Every ref above is a latest-value mirror read only from an event handler + // (a pointer drag, a keyboard nudge, a tab click), so it is refreshed on + // commit rather than during render: a handler cannot run before the commit + // that produced the value it reads. `trackedSetRightPanelTab` still writes + // the two tab refs itself, so a burst of calls inside one React batch + // accumulates instead of every call reading the same pre-render value; this + // effect then re-settles them onto what actually rendered. + useEffect(() => { + rightPanelTabRef.current = rightPanelTab; + rightInspectorPanesRef.current = rightInspectorPanes; + fittedRef.current = fitted; + effectiveLeftCollapsedRef.current = leftCollapsed || leftCollapsedByWidth; + }); + return { leftWidth: fitted.left, rightWidth: fitted.right, diff --git a/packages/studio/src/hooks/usePersistentEditHistory.ts b/packages/studio/src/hooks/usePersistentEditHistory.ts index 5c87ca2616..e2c1f12000 100644 --- a/packages/studio/src/hooks/usePersistentEditHistory.ts +++ b/packages/studio/src/hooks/usePersistentEditHistory.ts @@ -309,7 +309,14 @@ export function usePersistentEditHistory(options: UsePersistentEditHistoryOption const storeRef = useRef | null>(null); const storeProjectIdRef = useRef(null); const activeProjectIdRef = useRef(projectId); - activeProjectIdRef.current = projectId; + + // Which project is on screen, refreshed on commit rather than during render. + // Only the callbacks below read it, to refuse an edit aimed at a project the + // user has already navigated away from; declared above the loader so a + // project switch updates it before the store is torn down. + useEffect(() => { + activeProjectIdRef.current = projectId; + }, [projectId]); useEffect(() => { let cancelled = false; diff --git a/packages/studio/src/hooks/usePreviewPersistence.ts b/packages/studio/src/hooks/usePreviewPersistence.ts index ee7039231f..96f89b7bee 100644 --- a/packages/studio/src/hooks/usePreviewPersistence.ts +++ b/packages/studio/src/hooks/usePreviewPersistence.ts @@ -1,4 +1,4 @@ -import { useCallback, useRef, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; import { useMountEffect } from "./useMountEffect"; import { installStudioManualEditSeekReapply, @@ -111,14 +111,24 @@ export function usePreviewPersistence({ const domTextCommitVersionRef = useRef(0); const showToastRef = useRef(showToast); - showToastRef.current = showToast; const domEditSaveQueueRef = useRef | null>(null); - const applyStudioManualEditsToPreviewRef = useRef< - (iframe?: HTMLIFrameElement | null) => Promise - >(async () => {}); - if (!domEditSaveQueueRef.current) { - domEditSaveQueueRef.current = createDomEditSaveQueue({ + // Refreshed on commit rather than during render: the only reader is the save + // queue's `onOpen`, which fires from a failed request. + useEffect(() => { + showToastRef.current = showToast; + }, [showToast]); + + // Built on first use rather than during render. The old shape + // (`if (!ref.current) ref.current = createDomEditSaveQueue(...)`) read and + // wrote a ref while rendering, and moving it into a `useState` initializer + // only relocated the problem: the compiler follows the ref through any + // function render calls. Every reader goes through this, so the queue is + // still created at most once and nobody ever sees a missing one. + const ensureDomEditSaveQueue = useCallback(() => { + const existing = domEditSaveQueueRef.current; + if (existing) return existing; + const created = createDomEditSaveQueue({ onOpen: (event) => { const message = event.statusCode === 409 @@ -137,19 +147,22 @@ export function usePreviewPersistence({ setDomEditSaveQueuePaused(null); }, }); - } + domEditSaveQueueRef.current = created; + return created; + }, []); // ── Queue / drain helpers ── - const queueDomEditSave = useCallback((save: () => Promise): Promise => { - return domEditSaveQueueRef.current?.enqueue(save) ?? save(); - }, []); + const queueDomEditSave = useCallback( + (save: () => Promise): Promise => ensureDomEditSaveQueue().enqueue(save), + [ensureDomEditSaveQueue], + ); const drainPendingDomEditSaves = useCallback(async () => { - return drainStudioSaveQueues(flushStudioPendingEdits, async () => { - return (await domEditSaveQueueRef.current?.waitForIdle()) ?? { status: "clean" as const }; - }); - }, []); + return drainStudioSaveQueues(flushStudioPendingEdits, () => + ensureDomEditSaveQueue().waitForIdle(), + ); + }, [ensureDomEditSaveQueue]); const waitForPendingDomEditSaves = useCallback(async (): Promise => { const result = await drainPendingDomEditSaves(); @@ -157,9 +170,9 @@ export function usePreviewPersistence({ }, [drainPendingDomEditSaves]); const resetDomEditSaveQueueBreaker = useCallback(() => { - domEditSaveQueueRef.current?.reset(); + ensureDomEditSaveQueue().reset(); setDomEditSaveQueuePaused(null); - }, []); + }, [ensureDomEditSaveQueue]); useMountEffect(() => () => { domEditSaveQueueRef.current?.destroy(); @@ -168,22 +181,39 @@ export function usePreviewPersistence({ // ── Apply manual edits (HTML-baked — install seek hooks) ── // reapplyPositionEditsAfterSeek now also handles motion reapply from DOM attributes. + // The live preview frame is resolved in the body rather than as a default + // parameter value, which the React Compiler reads as a render-time ref + // access. `=== undefined` and not `??`, so an explicit `null` argument still + // means "no frame" rather than falling back to the current one. const applyCurrentStudioManualEditsToPreview = useCallback( - (iframe: HTMLIFrameElement | null = previewIframeRef.current) => { - if (!iframe) return; - if (!readIframeDocument(iframe)) return; - installManualEditReapply(iframe); + (iframe?: HTMLIFrameElement | null) => { + const target = iframe === undefined ? previewIframeRef.current : iframe; + if (!target) return; + if (!readIframeDocument(target)) return; + installManualEditReapply(target); }, [previewIframeRef], ); const applyStudioManualEditsToPreview = useCallback( - async (iframe: HTMLIFrameElement | null = previewIframeRef.current) => { - applyCurrentStudioManualEditsToPreview(iframe); + async (iframe?: HTMLIFrameElement | null) => { + applyCurrentStudioManualEditsToPreview( + iframe === undefined ? previewIframeRef.current : iframe, + ); }, [applyCurrentStudioManualEditsToPreview, previewIframeRef], ); - applyStudioManualEditsToPreviewRef.current = applyStudioManualEditsToPreview; + + // Handed to callers so they can invoke the latest closure without holding it. + // Seeded with the first render's function (not a no-op) and refreshed on + // commit, so it is never less current than the render-time assignment it + // replaces by the time a caller can reach it. + const applyStudioManualEditsToPreviewRef = useRef< + (iframe?: HTMLIFrameElement | null) => Promise + >(applyStudioManualEditsToPreview); + useEffect(() => { + applyStudioManualEditsToPreviewRef.current = applyStudioManualEditsToPreview; + }, [applyStudioManualEditsToPreview]); // ── Sync preview after undo/redo ── @@ -231,7 +261,6 @@ export function usePreviewPersistence({ return { domTextCommitVersionRef, - domEditSaveQueueRef, applyStudioManualEditsToPreviewRef, queueDomEditSave, drainPendingDomEditSaves, diff --git a/packages/studio/src/hooks/useProjectCompositionVariables.ts b/packages/studio/src/hooks/useProjectCompositionVariables.ts index f419a8b17b..bf4d0ddde6 100644 --- a/packages/studio/src/hooks/useProjectCompositionVariables.ts +++ b/packages/studio/src/hooks/useProjectCompositionVariables.ts @@ -61,6 +61,11 @@ export function useProjectCompositionVariables( const [groups, setGroups] = useState([]); useEffect(() => { + // `refreshKey` is a re-read token rather than a value the scan consumes, so + // the dependency list used to need a suppression to keep it. Naming it here + // makes the list true instead: re-running only replaces `groups` with a + // fresh read of the same files, so the extra run is idempotent. + void refreshKey; let cancelled = false; const htmlFiles = fileTree.filter((p) => p.endsWith(".html") && p !== excludePath); @@ -76,7 +81,6 @@ export function useProjectCompositionVariables( return () => { cancelled = true; }; - // eslint-disable-next-line react-hooks/exhaustive-deps }, [fileTree, excludePath, readProjectFile, refreshKey]); return groups; @@ -90,6 +94,26 @@ interface EditVariablesDeps { domEditSaveTimestampRef: MutableRefObject; } +/** + * Open a throwaway session on `content`, apply `mutate`, and serialize it back, + * disposing the session whether the mutation succeeds or throws. + * + * A plain function rather than an inline callback: the React Compiler cannot + * reorder across a `finally` and declines any hook body that holds one. + */ +async function mutateComposition( + content: string, + mutate: (session: Composition) => void, +): Promise { + const comp = await openComposition(content, { history: false }); + try { + mutate(comp); + return comp.serialize(); + } finally { + comp.dispose(); + } +} + /** * Apply a variable-schema mutation to an arbitrary composition file (a sub-comp * that isn't the active SDK session) and persist it through the standard @@ -104,15 +128,7 @@ export function useEditVariablesInFile(deps: EditVariablesDeps) { async (path: string, label: string, mutate: (session: Composition) => void): Promise => { const originalContent = await readProjectFile(path); await persistSdkSerialize( - async (onDiskBefore) => { - const comp = await openComposition(onDiskBefore, { history: false }); - try { - mutate(comp); - return comp.serialize(); - } finally { - comp.dispose(); - } - }, + (onDiskBefore) => mutateComposition(onDiskBefore, mutate), path, originalContent, { diff --git a/packages/studio/src/hooks/useProjectSignaturePoll.ts b/packages/studio/src/hooks/useProjectSignaturePoll.ts index 5692252534..a804b6eccc 100644 --- a/packages/studio/src/hooks/useProjectSignaturePoll.ts +++ b/packages/studio/src/hooks/useProjectSignaturePoll.ts @@ -33,8 +33,15 @@ export function useProjectSignaturePoll( ): void { const signatureRef = useRef(currentSignature); const onChangeRef = useRef(onChange); - signatureRef.current = currentSignature; - onChangeRef.current = onChange; + + // Refreshed on commit rather than during render: only the interval callback + // below reads these, and it cannot run before the commit that produced the + // values. Declared above the polling effect so it also runs first on every + // commit, which is what keeps the comparison baseline current. + useEffect(() => { + signatureRef.current = currentSignature; + onChangeRef.current = onChange; + }); useEffect(() => { if (!projectId) return; diff --git a/packages/studio/src/hooks/useRazorSplit.ts b/packages/studio/src/hooks/useRazorSplit.ts index b488807dc0..b24bd914f7 100644 --- a/packages/studio/src/hooks/useRazorSplit.ts +++ b/packages/studio/src/hooks/useRazorSplit.ts @@ -1,4 +1,4 @@ -import { useCallback, useRef } from "react"; +import { useCallback, useEffect, useRef } from "react"; import type { TimelineElement } from "../player"; import { usePlayerStore } from "../player"; import { getTimelineElementLabel } from "../utils/studioHelpers"; @@ -34,7 +34,12 @@ export function useRazorSplit({ isRecordingRef, }: UseRazorSplitOptions) { const projectIdRef = useRef(projectId); - projectIdRef.current = projectId; + + // Refreshed on commit rather than during render. Only `runCut` reads it, and + // it runs from a razor gesture, which cannot fire before the commit. + useEffect(() => { + projectIdRef.current = projectId; + }, [projectId]); const synchronize = useCallback(() => { let failure: unknown; @@ -46,7 +51,10 @@ export function useRazorSplit({ try { reloadPreview(); } catch (error) { - failure ??= error; + // Longhand for `failure ??= error`, which the React Compiler does not + // lower. Same nullish test, so a first failure that is not itself + // nullish still wins. + if (failure === undefined || failure === null) failure = error; } if (failure) throw failure; }, [forceReloadSdkSession, reloadPreview]); diff --git a/packages/studio/src/hooks/useRemoveBackground.ts b/packages/studio/src/hooks/useRemoveBackground.ts index c55a81e255..6147df7251 100644 --- a/packages/studio/src/hooks/useRemoveBackground.ts +++ b/packages/studio/src/hooks/useRemoveBackground.ts @@ -8,6 +8,32 @@ interface RemoveBackgroundOptions { onProgress?: (progress: BackgroundRemovalProgress) => void; } +/** + * Await one accepted job and report it, releasing the in-flight controller + * whichever way it ends. + * + * A plain function rather than a `try`/`finally` inside the hook's callback: + * the React Compiler cannot reorder across a `finally` and declines the whole + * function. `release` is called on success, on failure, and on abort. + */ +async function awaitRemoval( + jobId: string, + options: RemoveBackgroundOptions, + signal: AbortSignal, + release: () => void, + refreshFileTree: () => Promise, + showToast: (message: string, kind?: "info" | "error") => void, +) { + try { + const result = await waitForMediaJob(jobId, options.onProgress, signal); + await refreshFileTree(); + showToast(`Created transparent asset: ${result.outputPath.split("/").pop()}`, "info"); + return result; + } finally { + release(); + } +} + /** * One removal in flight at a time: starting a second one aborts whichever job * is still running, so a stale progress callback can't overwrite a newer @@ -54,16 +80,18 @@ export function useRemoveBackground( abortRef.current?.abort(); const controller = new AbortController(); abortRef.current = controller; - try { - const result = await waitForMediaJob(data.jobId, options.onProgress, controller.signal); - await refreshFileTree(); - showToast(`Created transparent asset: ${result.outputPath.split("/").pop()}`, "info"); - return result; - } finally { - if (abortRef.current === controller) { - abortRef.current = null; - } - } + return awaitRemoval( + data.jobId, + options, + controller.signal, + () => { + if (abortRef.current === controller) { + abortRef.current = null; + } + }, + refreshFileTree, + showToast, + ); }, [projectId, refreshFileTree, showToast], ); diff --git a/packages/studio/src/hooks/useSdkSession.ts b/packages/studio/src/hooks/useSdkSession.ts index 8cd5cea1c0..675fdb595a 100644 --- a/packages/studio/src/hooks/useSdkSession.ts +++ b/packages/studio/src/hooks/useSdkSession.ts @@ -147,12 +147,19 @@ export function useSdkSession( const sessionOwnersRef = useRef(new WeakMap()); const generationRef = useRef(0); const projectIdRef = useRef(projectId); - projectIdRef.current = projectId; const activeCompPathRef = useRef(activeCompPath); - activeCompPathRef.current = activeCompPath; const [reloadToken, setReloadToken] = useState(0); const reloadTokenRef = useRef(reloadToken); - reloadTokenRef.current = reloadToken; + + // What the hook is currently pointed at, refreshed on commit rather than + // during render. Every reader is an effect, a file-watcher callback or the + // ownership check, so none of them can run before this does; it is declared + // first so it also runs before the session effect below on every commit. + useEffect(() => { + projectIdRef.current = projectId; + activeCompPathRef.current = activeCompPath; + reloadTokenRef.current = reloadToken; + }); useEffect( () => diff --git a/packages/studio/src/hooks/useSlideshowTabState.test.ts b/packages/studio/src/hooks/useSlideshowTabState.test.ts index be45f448b2..005cf18f9b 100644 --- a/packages/studio/src/hooks/useSlideshowTabState.test.ts +++ b/packages/studio/src/hooks/useSlideshowTabState.test.ts @@ -15,20 +15,27 @@ afterEach(() => { document.body.innerHTML = ""; }); +/** A stand-in for the preview frame, carrying only the manifest the hook reads. */ +function fakePreviewFrame(scenes: unknown): HTMLIFrameElement { + return { contentWindow: { __clipManifest: { scenes } } } as unknown as HTMLIFrameElement; +} + function renderHook(params: { editingFileContent: string | null | undefined; rightPanelTab: RightPanelTab; + previewIframe?: HTMLIFrameElement | null; }) { const host = document.createElement("div"); document.body.append(host); const root = createRoot(host); const setRightPanelTabCalls: RightPanelTab[] = []; let current: ReturnType | null = null; + const previewIframeRef = { current: params.previewIframe ?? null }; function Harness() { current = useSlideshowTabState({ editingFileContent: params.editingFileContent, - previewIframeRef: { current: null }, + previewIframeRef, refreshKey: 0, rightPanelTab: params.rightPanelTab, setRightPanelTab: (tab) => setRightPanelTabCalls.push(tab), @@ -93,4 +100,46 @@ describe("useSlideshowTabState", () => { expect(harness.setRightPanelTabCalls).toEqual([]); harness.unmount(); }); + + it("derives the scene list from the preview frame's clip manifest", () => { + const harness = renderHook({ + editingFileContent: SLIDESHOW_HTML, + rightPanelTab: "slideshow", + previewIframe: fakePreviewFrame([ + { id: "s1", label: "Intro", start: 0, duration: 2, extra: "dropped" }, + { id: "s2", label: "Outro", start: 2, duration: 3 }, + ]), + }); + expect(harness.getState().slideshowScenes).toEqual([ + { id: "s1", label: "Intro", start: 0, duration: 2 }, + { id: "s2", label: "Outro", start: 2, duration: 3 }, + ]); + harness.unmount(); + }); + + it("reports no scenes when the preview frame has no manifest yet", () => { + const harness = renderHook({ + editingFileContent: SLIDESHOW_HTML, + rightPanelTab: "slideshow", + previewIframe: { contentWindow: {} } as unknown as HTMLIFrameElement, + }); + expect(harness.getState().slideshowScenes).toEqual([]); + harness.unmount(); + }); + + it("reports no scenes when reading the frame throws (cross-origin preview)", () => { + const hostile = {} as HTMLIFrameElement; + Object.defineProperty(hostile, "contentWindow", { + get() { + throw new Error("blocked a frame with origin"); + }, + }); + const harness = renderHook({ + editingFileContent: SLIDESHOW_HTML, + rightPanelTab: "slideshow", + previewIframe: hostile, + }); + expect(harness.getState().slideshowScenes).toEqual([]); + harness.unmount(); + }); }); diff --git a/packages/studio/src/hooks/useSlideshowTabState.ts b/packages/studio/src/hooks/useSlideshowTabState.ts index df7d49ea6e..58f662a68d 100644 --- a/packages/studio/src/hooks/useSlideshowTabState.ts +++ b/packages/studio/src/hooks/useSlideshowTabState.ts @@ -1,9 +1,39 @@ -import { useEffect, useMemo, type MutableRefObject } from "react"; +import { useEffect, useMemo, useState, type MutableRefObject } from "react"; import { SLIDESHOW_ISLAND_TYPE, slideshowIslandRegex } from "@hyperframes/core/slideshow"; import type { SceneInfo } from "../components/panels/SlideshowPanel"; import type { IframeWindow } from "../player/lib/playbackTypes"; import type { RightPanelTab } from "../utils/studioHelpers"; +/** The live scene list the preview is currently playing, or none. */ +function readSlideshowScenes(iframe: HTMLIFrameElement | null): SceneInfo[] { + try { + const win = iframe?.contentWindow as IframeWindow | null; + return (win?.__clipManifest?.scenes ?? []).map((s) => ({ + id: s.id, + label: s.label, + start: s.start, + duration: s.duration, + })); + } catch { + return []; + } +} + +/** Scene lists are short and flat, so equality is a field-by-field walk. */ +function sameScenes(a: SceneInfo[], b: SceneInfo[]): boolean { + if (a.length !== b.length) return false; + return a.every((scene, index) => { + const other = b[index]; + return ( + other !== undefined && + scene.id === other.id && + scene.label === other.label && + scene.start === other.start && + scene.duration === other.duration + ); + }); +} + /** * Derives whether the currently-edited composition is a slideshow (carries * the slideshow JSON island — the same definitive marker the CLI's `present` @@ -35,20 +65,17 @@ export function useSlideshowTabState(params: { return slideshowIslandRegex("i").test(editingFileContent); }, [editingFileContent]); - // Derive scene list from the live clip manifest in the preview iframe. - const slideshowScenes = useMemo(() => { - try { - const win = previewIframeRef.current?.contentWindow as IframeWindow | null; - return (win?.__clipManifest?.scenes ?? []).map((s) => ({ - id: s.id, - label: s.label, - start: s.start, - duration: s.duration, - })); - } catch { - return []; - } - // eslint-disable-next-line react-hooks/exhaustive-deps + // The scene list comes from the preview iframe, which is live mutable state + // rather than a render input: read on commit, not during render. `rightPanelTab` + // and `refreshKey` are re-read triggers (opening the tab, and a preview reload), + // which is why the memo this replaces needed a suppression to keep them. + const [slideshowScenes, setSlideshowScenes] = useState([]); + useEffect(() => { + const next = readSlideshowScenes(previewIframeRef.current); + // Keeping the previous array when nothing moved is what stops this read + // costing a second commit on every tab switch, which the memo it replaces + // never did. + setSlideshowScenes((prev) => (sameScenes(prev, next) ? prev : next)); }, [previewIframeRef, rightPanelTab, refreshKey]); useEffect(() => { diff --git a/packages/studio/src/hooks/useThumbnailLease.ts b/packages/studio/src/hooks/useThumbnailLease.ts index 01ecd82c39..0758ac21de 100644 --- a/packages/studio/src/hooks/useThumbnailLease.ts +++ b/packages/studio/src/hooks/useThumbnailLease.ts @@ -1,4 +1,4 @@ -import { useCallback, useLayoutEffect, useRef, useSyncExternalStore } from "react"; +import { useCallback, useEffect, useLayoutEffect, useRef, useSyncExternalStore } from "react"; import { createThumbnailRequestIdentity, thumbnailScheduler, @@ -14,10 +14,20 @@ export function useThumbnailLease( scheduler: ThumbnailScheduler = thumbnailScheduler, ): ThumbnailSnapshot { const requestRef = useRef(request); - requestRef.current = request; const leaseRef = useRef | null>(null); const identity = request ? createThumbnailRequestIdentity(request) : null; const priority = request?.priority; + + // `subscribe` is keyed on the identity, not on the request object, so that a + // fresh object with the same identity does not release and re-acquire the + // lease every render. It still has to reach the CURRENT request for its + // `load` and `priority`, hence the ref, refreshed on commit: React calls + // `subscribe` from a passive effect, and this effect is declared first, so it + // has already run by then. + useEffect(() => { + requestRef.current = request; + }); + const subscribe = useCallback( (listener: () => void) => { const current = requestRef.current; @@ -31,10 +41,14 @@ export function useThumbnailLease( }, [identity, scheduler], ); - const getSnapshot = useCallback(() => { - const current = requestRef.current; - return current && identity !== null ? scheduler.getSnapshot(current) : IDLE; - }, [identity, scheduler]); + + // Read during render, so it takes the request straight from the arguments: a + // ref would still hold the previous one on the render that changes identity, + // and the scheduler derives the entry it looks up from what it is handed. + const getSnapshot = useCallback( + () => (request && identity !== null ? scheduler.getSnapshot(request) : IDLE), + [request, identity, scheduler], + ); useLayoutEffect(() => { if (priority) leaseRef.current?.updatePriority(priority); diff --git a/packages/studio/src/hooks/useTimelineDeleteOps.ts b/packages/studio/src/hooks/useTimelineDeleteOps.ts index cd47b9adcf..da5b9667cd 100644 --- a/packages/studio/src/hooks/useTimelineDeleteOps.ts +++ b/packages/studio/src/hooks/useTimelineDeleteOps.ts @@ -12,8 +12,8 @@ import { captureDurationRollback, readFileContent } from "./timelineTimingSync"; import { setCompositionDurationToContent } from "../utils/timelineAssetDrop"; import { furthestClipEndFromSource } from "../player/lib/timelineElementHelpers"; -interface UseTimelineDeleteOpsOptions { - projectIdRef: MutableRefObject; +/** What a delete needs from the app, whichever entry point started it. */ +interface TimelineDeleteDeps { activeCompPath: string | null; timelineElements: TimelineElement[]; showToast: (message: string, tone?: "error" | "info") => void; @@ -21,11 +21,126 @@ interface UseTimelineDeleteOpsOptions { recordEdit: (input: RecordEditInput) => Promise; domEditSaveTimestampRef: MutableRefObject; reloadPreview: () => void; - isRecordingRef?: MutableRefObject; forceReloadSdkSession?: () => void; previewIframeRef: RefObject; } +interface UseTimelineDeleteOpsOptions extends TimelineDeleteDeps { + projectIdRef: MutableRefObject; + isRecordingRef?: MutableRefObject; +} + +interface RunTimelineDeleteArgs extends TimelineDeleteDeps { + projectId: string; + targetPath: string; + /** The selected clips that live in `targetPath`; the rest were dropped. */ + sameFile: TimelineElement[]; + label: string; +} + +/** + * One delete pass: remove every clip from the file, shrink the composition to + * the remaining content, persist it as a single history entry, then resettle + * the store and the preview. Throws on any failure; the caller reports it. + * + * A plain function rather than the hook callback's own body: the React Compiler + * declines to lower a `throw` inside a `try`/`catch`, and this is where all of + * them are. + */ +// fallow-ignore-next-line complexity +async function runTimelineDelete({ + projectId, + targetPath, + sameFile, + label, + activeCompPath, + timelineElements, + showToast, + writeProjectFile, + recordEdit, + domEditSaveTimestampRef, + reloadPreview, + forceReloadSdkSession, + previewIframeRef, +}: RunTimelineDeleteArgs): Promise { + const originalContent = await readFileContent(projectId, targetPath); + + // Remove every selected element before saving once. The server rewrites + // the file per call, so `removedContent` after the last one holds them + // all, which is what makes this a single history entry, and a single + // undo, rather than one per clip. + let removedContent = originalContent; + for (const target of sameFile) { + const patchTarget = buildPatchTarget(target); + if (!patchTarget) { + throw new Error(`Timeline element ${target.id} is missing a patchable target`); + } + + const removeResponse = await fetch( + `/api/projects/${projectId}/file-mutations/remove-element/${encodeURIComponent(targetPath)}`, + { + method: "POST", + headers: { "Content-Type": "application/json", ...studioWriteHeaders() }, + body: JSON.stringify({ target: patchTarget }), + }, + ); + if (!removeResponse.ok) { + throw new Error(`Failed to delete ${target.id} from ${targetPath}`); + } + + const removeData = (await removeResponse.json()) as { + changed?: boolean; + content?: string; + }; + if (typeof removeData.content === "string") removedContent = removeData.content; + } + // Content-driven duration: shrink the composition to the furthest + // remaining clip end, read from the post-removal SOURCE (raw + // data-duration), so deleting the last/longest clip removes trailing + // empty space. Measured from the source, not the store, whose + // durations are runtime-truncated. + const deleteContentEnd = furthestClipEndFromSource(removedContent); + const patchedContent = setCompositionDurationToContent(removedContent, deleteContentEnd); + // Optimistically reflect the shrunk length in the readout/seek bar, + // rolling it back if the persist below fails (see captureDurationRollback). + const rollbackDuration = captureDurationRollback(previewIframeRef.current); + if (deleteContentEnd > 0 && targetPath === (activeCompPath || "index.html")) { + usePlayerStore.getState().setDuration(deleteContentEnd); + } + + domEditSaveTimestampRef.current = Date.now(); + try { + await saveProjectFilesWithHistory({ + projectId, + label: "Delete timeline clip", + kind: "timeline", + files: { [targetPath]: patchedContent }, + readFile: async () => originalContent, + // remove-element already wrote the removal, so disk holds THAT, not the + // content read at the top. Undo still goes back to the original. + diskContent: { [targetPath]: removedContent }, + writeFile: writeProjectFile, + recordEdit, + }); + } catch (error) { + rollbackDuration(); + throw error; + } + + const deletedKeys = new Set(sameFile.map((te) => te.key ?? te.id)); + usePlayerStore + .getState() + .setElements(timelineElements.filter((te) => !deletedKeys.has(te.key ?? te.id))); + usePlayerStore.getState().setSelectedElementId(null); + usePlayerStore.getState().setSelectedElementIds(new Set()); + forceReloadSdkSession?.(); + reloadPreview(); + showToast( + `Deleted ${label}. Use Undo to restore ${sameFile.length === 1 ? "it" : "them"}.`, + "info", + ); +} + export function useTimelineDeleteOps({ projectIdRef, activeCompPath, @@ -61,87 +176,24 @@ export function useTimelineDeleteOps({ const sameFile = selection.filter( (candidate) => (candidate.sourceFile || activeCompPath || "index.html") === targetPath, ); - try { - const originalContent = await readFileContent(pid, targetPath); - - // Remove every selected element before saving once. The server rewrites - // the file per call, so `removedContent` after the last one holds them - // all — which is what makes this a single history entry, and a single - // undo, rather than one per clip. - let removedContent = originalContent; - for (const target of sameFile) { - const patchTarget = buildPatchTarget(target); - if (!patchTarget) { - throw new Error(`Timeline element ${target.id} is missing a patchable target`); - } - - const removeResponse = await fetch( - `/api/projects/${pid}/file-mutations/remove-element/${encodeURIComponent(targetPath)}`, - { - method: "POST", - headers: { "Content-Type": "application/json", ...studioWriteHeaders() }, - body: JSON.stringify({ target: patchTarget }), - }, - ); - if (!removeResponse.ok) { - throw new Error(`Failed to delete ${target.id} from ${targetPath}`); - } - - const removeData = (await removeResponse.json()) as { - changed?: boolean; - content?: string; - }; - if (typeof removeData.content === "string") removedContent = removeData.content; - } - // Content-driven duration: shrink the composition to the furthest - // remaining clip end, read from the post-removal SOURCE (raw - // data-duration), so deleting the last/longest clip removes trailing - // empty space. Measured from the source, not the store, whose - // durations are runtime-truncated. - const deleteContentEnd = furthestClipEndFromSource(removedContent); - const patchedContent = setCompositionDurationToContent(removedContent, deleteContentEnd); - // Optimistically reflect the shrunk length in the readout/seek bar, - // rolling it back if the persist below fails (see captureDurationRollback). - const rollbackDuration = captureDurationRollback(previewIframeRef.current); - if (deleteContentEnd > 0 && targetPath === (activeCompPath || "index.html")) { - usePlayerStore.getState().setDuration(deleteContentEnd); - } - - domEditSaveTimestampRef.current = Date.now(); - try { - await saveProjectFilesWithHistory({ - projectId: pid, - label: "Delete timeline clip", - kind: "timeline", - files: { [targetPath]: patchedContent }, - readFile: async () => originalContent, - // remove-element already wrote the removal, so disk holds THAT — not the - // content read at the top. Undo still goes back to the original. - diskContent: { [targetPath]: removedContent }, - writeFile: writeProjectFile, - recordEdit, - }); - } catch (error) { - rollbackDuration(); - throw error; - } - - const deletedKeys = new Set(sameFile.map((te) => te.key ?? te.id)); - usePlayerStore - .getState() - .setElements(timelineElements.filter((te) => !deletedKeys.has(te.key ?? te.id))); - usePlayerStore.getState().setSelectedElementId(null); - usePlayerStore.getState().setSelectedElementIds(new Set()); - forceReloadSdkSession?.(); - reloadPreview(); - showToast( - `Deleted ${label}. Use Undo to restore ${sameFile.length === 1 ? "it" : "them"}.`, - "info", - ); - } catch (error) { + await runTimelineDelete({ + projectId: pid, + targetPath, + sameFile, + label, + activeCompPath, + timelineElements, + showToast, + writeProjectFile, + recordEdit, + domEditSaveTimestampRef, + reloadPreview, + forceReloadSdkSession, + previewIframeRef, + }).catch((error: unknown) => { const message = error instanceof Error ? error.message : "Failed to delete timeline clip"; showToast(message); - } + }); }, [ activeCompPath, diff --git a/packages/studio/src/hooks/useTimelineEditing.ts b/packages/studio/src/hooks/useTimelineEditing.ts index 070c284f46..b43049a360 100644 --- a/packages/studio/src/hooks/useTimelineEditing.ts +++ b/packages/studio/src/hooks/useTimelineEditing.ts @@ -1,5 +1,5 @@ // fallow-ignore-file complexity -import { useCallback, useRef } from "react"; +import { useCallback, useEffect, useRef } from "react"; import type { TimelineElement } from "../player"; import { useRazorSplit } from "./useRazorSplit"; import { useTimelineAssetDropOps } from "./useTimelineAssetDropOps"; @@ -56,9 +56,15 @@ export function useTimelineEditing({ handleDomZIndexReorderCommitRef, }: UseTimelineEditingOptions) { const projectIdRef = useRef(projectId); - projectIdRef.current = projectId; const editQueueRef = useRef(Promise.resolve()); + // Refreshed on commit rather than during render. Every reader is a timeline + // edit handler or a sub-hook that runs from one, so none can see it before + // the commit that produced the value. + useEffect(() => { + projectIdRef.current = projectId; + }, [projectId]); + const enqueueEdit = useCallback( ( element: TimelineElement, diff --git a/packages/studio/src/hooks/useTimelineGroupEditing.ts b/packages/studio/src/hooks/useTimelineGroupEditing.ts index 03abbb6930..21a2f87d85 100644 --- a/packages/studio/src/hooks/useTimelineGroupEditing.ts +++ b/packages/studio/src/hooks/useTimelineGroupEditing.ts @@ -316,28 +316,28 @@ export function useTimelineGroupEditing({ // GSAP read is stale whether or not the position rewrite succeeded — // invalidate on the error path too (matches the single-element path's // `.finally`), or a failed rewrite leaves the editor reading old tweens. - try { - await finishGroupTimingGsapFallback({ - projectId, - iframe: previewIframeRef.current, - reloadPreview, - label: "Move timeline clips", - errorLabel: "Failed to shift GSAP positions", - coalesceKey, - recordEdit, - activeCompPath, - changes, - resolveChangePath: (element) => targetPathFor(element, activeCompPath), - mutateChange: (change, changePath) => { - const delta = change.start - change.element.start; - const domId = change.element.domId; - if (delta === 0 || !domId) return null; - return shiftGsapPositions(projectId, changePath, domId, delta); - }, - }); - } finally { - invalidateGsapCache?.(); - } + // `.finally` on the promise rather than a `try`/`finally` block: the + // React Compiler cannot reorder across a `finally` and declines the + // whole hook. `finishGroupTimingGsapFallback` is async, so it can only + // reject, never throw before returning its promise. + await finishGroupTimingGsapFallback({ + projectId, + iframe: previewIframeRef.current, + reloadPreview, + label: "Move timeline clips", + errorLabel: "Failed to shift GSAP positions", + coalesceKey, + recordEdit, + activeCompPath, + changes, + resolveChangePath: (element) => targetPathFor(element, activeCompPath), + mutateChange: (change, changePath) => { + const delta = change.start - change.element.start; + const domId = change.element.domId; + if (delta === 0 || !domId) return null; + return shiftGsapPositions(projectId, changePath, domId, delta); + }, + }).finally(() => invalidateGsapCache?.()); }).catch((error) => { // Failed persist: revert the optimistic duration readout + live root // alongside the gesture owner's store rollback. @@ -419,38 +419,37 @@ export function useTimelineGroupEditing({ } // See the move path: the timing persist is already on disk, so the GSAP // cache must be invalidated even when the position rewrite throws. - try { - await finishGroupTimingGsapFallback({ - projectId, - iframe: previewIframeRef.current, - reloadPreview, - label: "Resize timeline clips", - errorLabel: "Failed to scale GSAP positions", - coalesceKey, - recordEdit, - activeCompPath, - changes, - resolveChangePath: (element) => targetPathFor(element, activeCompPath), - mutateChange: (change, changePath) => { - const domId = change.element.domId; - const timingChanged = - change.start !== change.element.start || - change.duration !== change.element.duration; - if (!timingChanged || !domId) return null; - return scaleGsapPositions( - projectId, - changePath, - domId, - change.element.start, - change.element.duration, - change.start, - change.duration, - ); - }, - }); - } finally { - invalidateGsapCache?.(); - } + // `.finally` on the promise rather than a `try`/`finally` block: the + // React Compiler cannot reorder across a `finally` and declines the + // whole hook. `finishGroupTimingGsapFallback` is async, so it can only + // reject, never throw before returning its promise. + await finishGroupTimingGsapFallback({ + projectId, + iframe: previewIframeRef.current, + reloadPreview, + label: "Resize timeline clips", + errorLabel: "Failed to scale GSAP positions", + coalesceKey, + recordEdit, + activeCompPath, + changes, + resolveChangePath: (element) => targetPathFor(element, activeCompPath), + mutateChange: (change, changePath) => { + const domId = change.element.domId; + const timingChanged = + change.start !== change.element.start || change.duration !== change.element.duration; + if (!timingChanged || !domId) return null; + return scaleGsapPositions( + projectId, + changePath, + domId, + change.element.start, + change.element.duration, + change.start, + change.duration, + ); + }, + }).finally(() => invalidateGsapCache?.()); }).catch((error) => { // Failed persist: revert the optimistic duration readout + live root // alongside the gesture owner's store rollback. diff --git a/packages/studio/src/hooks/useTimelineSelectionPreviewSync.ts b/packages/studio/src/hooks/useTimelineSelectionPreviewSync.ts index 79752e3f64..d35b36aab2 100644 --- a/packages/studio/src/hooks/useTimelineSelectionPreviewSync.ts +++ b/packages/studio/src/hooks/useTimelineSelectionPreviewSync.ts @@ -84,8 +84,16 @@ export function useTimelineSelectionPreviewSync({ const domEditGroupSelectionsRef = useRef(domEditGroupSelections); const lastSyncedSelectedKeyRef = useRef(""); const missingSelectionKeyRef = useRef(""); - domEditSelectionRef.current = domEditSelection; - domEditGroupSelectionsRef.current = domEditGroupSelections; + + // The canvas selection is read through refs so the effect below does not + // depend on it: depending on it directly would let the preview-to-timeline + // echo cancel an in-flight timeline click. Refreshed on commit rather than + // during render, and declared first so it has run by the time the sync effect + // reads it. + useEffect(() => { + domEditSelectionRef.current = domEditSelection; + domEditGroupSelectionsRef.current = domEditGroupSelections; + }); useEffect(() => { const previousSelectedKey = lastSyncedSelectedKeyRef.current; @@ -180,9 +188,6 @@ export function useTimelineSelectionPreviewSync({ return () => { cancelled = true; }; - // DOM selection changes are read through refs. Depending on them directly - // would let the preview-to-timeline echo cancel an in-flight timeline click. - // eslint-disable-next-line react-hooks/exhaustive-deps }, [ activeCompPath, applyDomSelection, diff --git a/packages/studio/src/hooks/useToast.ts b/packages/studio/src/hooks/useToast.ts index ea7f5add16..951ec38af7 100644 --- a/packages/studio/src/hooks/useToast.ts +++ b/packages/studio/src/hooks/useToast.ts @@ -14,6 +14,19 @@ const MAX_TOASTS = 3; let nextToastId = 1; +/** + * The next id, and advance the counter. + * + * A plain function rather than `nextToastId++` at the call site: the React + * Compiler declines to lower an update expression on a module-level binding, + * and the increment has no reason to live inside the hook. + */ +function takeToastId(): number { + const id = nextToastId; + nextToastId += 1; + return id; +} + /** * Stacked toasts (max 3). Info toasts auto-dismiss after 4s; error toasts * persist until explicitly dismissed so failures can't silently vanish. @@ -51,7 +64,7 @@ export function useToast() { const showToast = useCallback( (message: string, tone: AppToast["tone"] = "error") => { - const id = nextToastId++; + const id = takeToastId(); setToasts((prev) => { const next = [...prev, { id, message, tone }]; // Cap the stack; drop the oldest (and its pending timer). diff --git a/packages/studio/src/player/hooks/usePlaybackKeyboard.ts b/packages/studio/src/player/hooks/usePlaybackKeyboard.ts index 6ebb561679..9df6016eb2 100644 --- a/packages/studio/src/player/hooks/usePlaybackKeyboard.ts +++ b/packages/studio/src/player/hooks/usePlaybackKeyboard.ts @@ -6,7 +6,7 @@ * and iframe listener setup function. Has no side effects of its own. */ -import { useRef, useCallback } from "react"; +import { useRef, useCallback, useEffect } from "react"; import { useCaptionStore } from "../../captions/store"; import { shouldIgnorePlaybackShortcutEvent, SHUTTLE_SPEEDS } from "../lib/playbackShortcuts"; import { canvasNudgeKeysClaimed } from "../../utils/canvasNudgeGate"; @@ -181,8 +181,13 @@ export function usePlaybackKeyboard({ pressedKeysRef.current.delete(e.key.toLowerCase()); }, []); - playbackKeyDownRef.current = handlePlaybackKeyDown; - playbackKeyUpRef.current = handlePlaybackKeyUp; + // Refreshed on commit rather than during render. The window and iframe + // listeners call through these refs so they never have to be re-attached when + // a handler's identity changes, and a key event cannot arrive before commit. + useEffect(() => { + playbackKeyDownRef.current = handlePlaybackKeyDown; + playbackKeyUpRef.current = handlePlaybackKeyUp; + }, [handlePlaybackKeyDown, handlePlaybackKeyUp]); // fallow-ignore-next-line complexity const attachIframeShortcutListeners = useCallback(() => { diff --git a/packages/studio/src/player/hooks/useTimelinePlayer.ts b/packages/studio/src/player/hooks/useTimelinePlayer.ts index 7fb883fadb..4d47bb1547 100644 --- a/packages/studio/src/player/hooks/useTimelinePlayer.ts +++ b/packages/studio/src/player/hooks/useTimelinePlayer.ts @@ -46,6 +46,19 @@ import { applyPreviewVariablesToUrl } from "../../hooks/previewVariablesStore"; import { createPreviewMessageHandler } from "./previewMessageRouter"; import { timelineElementsChanged } from "./timelinePlayerSync"; +/** + * The store's action handles, which never change for the store's lifetime. + * + * Read through a plain function rather than `usePlayerStore.getState()` inline: + * the React Compiler reads the store's own name in a hook body as a hook used + * as a value and declines the file. + */ +function readPlayerStoreActions() { + const { setIsPlaying, setCurrentTime, setDuration, setTimelineReady, setElements } = + usePlayerStore.getState(); + return { setIsPlaying, setCurrentTime, setDuration, setTimelineReady, setElements }; +} + export function useTimelinePlayer() { const iframeRef = useRef(null); const rafRef = useRef(0); @@ -61,7 +74,7 @@ export function useTimelinePlayer() { const staticSeekWarnedRef = useRef(false); const { setIsPlaying, setCurrentTime, setDuration, setTimelineReady, setElements } = - usePlayerStore.getState(); + readPlayerStoreActions(); // The fixture lease belongs at this shared synchronization boundary so every // iframe discovery path has the same owner for deciding whether it may write. @@ -149,7 +162,8 @@ export function useTimelinePlayer() { releaseStaticSeekCache(staticSeekAdapterRef, staticSeekWarnedRef); return adapter; } - if (dur > 0) timelineAdapter ??= adapter; + // `??=` longhand: the React Compiler does not lower logical assignment. + if (dur > 0) timelineAdapter = timelineAdapter ?? adapter; } if (win.__timelines) { @@ -167,7 +181,7 @@ export function useTimelinePlayer() { releaseStaticSeekCache(staticSeekAdapterRef, staticSeekWarnedRef); return adapter; } - if (dur > 0) timelineAdapter ??= adapter; + if (dur > 0) timelineAdapter = timelineAdapter ?? adapter; } } @@ -475,8 +489,12 @@ export function useTimelinePlayer() { applyPreviewVariablesToUrl(url); iframe.src = url.toString(); }, [saveSeekPosition]); + // Refreshed on commit rather than during render; the only reader is the + // visibility handler installed on mount, which fires from a document event. const getAdapterRef = useRef(getAdapter); - getAdapterRef.current = getAdapter; + useEffect(() => { + getAdapterRef.current = getAdapter; + }, [getAdapter]); useMountEffect(() => { const handleWindowKeyDown = (e: KeyboardEvent) => playbackKeyDownRef.current(e); diff --git a/packages/studio/src/styles/compiler-bailouts.json b/packages/studio/src/styles/compiler-bailouts.json index 012d362c8f..dddc9e7f88 100644 --- a/packages/studio/src/styles/compiler-bailouts.json +++ b/packages/studio/src/styles/compiler-bailouts.json @@ -1,5 +1,5 @@ { - "total": 126, + "total": 104, "files": { "src/App.tsx": 1, "src/captions/components/CaptionOverlay.tsx": 1, @@ -87,25 +87,6 @@ "src/hooks/useGsapScriptCommits.ts": 1, "src/hooks/useGsapSelectionHandlers.ts": 1, "src/hooks/useGsapTweenCache.ts": 1, - "src/hooks/useLintModal.ts": 1, - "src/hooks/useLivePlayheadTime.ts": 1, - "src/hooks/useMountEffect.ts": 1, - "src/hooks/useMusicBeatAnalysis.ts": 1, - "src/hooks/usePanelLayout.ts": 1, - "src/hooks/usePersistentEditHistory.ts": 1, - "src/hooks/usePreviewPersistence.ts": 1, - "src/hooks/useProjectCompositionVariables.ts": 1, - "src/hooks/useProjectSignaturePoll.ts": 1, - "src/hooks/useRazorSplit.ts": 1, - "src/hooks/useRemoveBackground.ts": 1, - "src/hooks/useSdkSession.ts": 1, - "src/hooks/useSlideshowTabState.ts": 1, - "src/hooks/useThumbnailLease.ts": 1, - "src/hooks/useTimelineDeleteOps.ts": 1, - "src/hooks/useTimelineEditing.ts": 1, - "src/hooks/useTimelineGroupEditing.ts": 1, - "src/hooks/useTimelineSelectionPreviewSync.ts": 1, - "src/hooks/useToast.ts": 1, "src/player/components/Player.tsx": 1, "src/player/components/PlayerControls.tsx": 1, "src/player/components/Timeline.tsx": 1, @@ -123,9 +104,6 @@ "src/player/components/useTimelineSelectionLifecycle.ts": 1, "src/player/components/useTimelineStackingSync.ts": 1, "src/player/components/useTimelineTrackLayout.ts": 1, - "src/player/components/useTimelineVirtualRows.ts": 1, - "src/player/hooks/usePlaybackKeyboard.ts": 1, - "src/player/hooks/useTimelinePlayer.ts": 1, - "src/webmcp/useStudioAgentTools.ts": 1 + "src/player/components/useTimelineVirtualRows.ts": 1 } } diff --git a/packages/studio/src/webmcp/useStudioAgentTools.ts b/packages/studio/src/webmcp/useStudioAgentTools.ts index 717f9dcd38..ae8cdd72b9 100644 --- a/packages/studio/src/webmcp/useStudioAgentTools.ts +++ b/packages/studio/src/webmcp/useStudioAgentTools.ts @@ -289,7 +289,13 @@ function readNumberInput(input: object, key: string): number { */ export function useStudioAgentTools(deps: StudioAgentToolsDeps): void { const depsRef = useRef(deps); - depsRef.current = deps; + + // Refreshed on commit rather than during render. The tools built below read + // it when the agent calls one, which is long after registration, itself an + // async step inside the mount effect below. + useEffect(() => { + depsRef.current = deps; + }); // eslint-disable-next-line no-restricted-syntax useEffect(() => {