Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 105 additions & 0 deletions packages/studio/src/hooks/useLintModal.test.tsx
Original file line number Diff line number Diff line change
@@ -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<typeof useLintModal> | 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();
});
});
6 changes: 4 additions & 2 deletions packages/studio/src/hooks/useLintModal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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],
);
Expand Down
105 changes: 105 additions & 0 deletions packages/studio/src/hooks/useLivePlayheadTime.test.tsx
Original file line number Diff line number Diff line change
@@ -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();
});
});
21 changes: 11 additions & 10 deletions packages/studio/src/hooks/useLivePlayheadTime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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<number | null>(null);

useEffect(() => {
if (!isPlaying) return;
let latest: number | null = null;
let timerId: ReturnType<typeof setTimeout> | 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;
}
11 changes: 8 additions & 3 deletions packages/studio/src/hooks/useMountEffect.ts
Original file line number Diff line number Diff line change
@@ -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).
Expand All @@ -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(), []);
}
Loading
Loading