peter
6a7ff330f6
Ten more suppressions, and React 19 turns out to supply the missing primitive.
useEffectEvent (stable since React 19.2, which S1 brought in) is the canonical answer to the
"bind the listener once, but read the latest values" pattern these hooks were disabling the
checker for. Three of playerModalHooks' four escape hatches become honest dep lists: the
wheel-volume handler, the native-controls handover, and the fullscreen-change sync. The
contract is no longer prose — the compiler enforces it.
The fourth stays, deliberately, and now says why at the site: the YouTube player's lifecycle
effect must run once per video (re-running destroys and rebuilds the iframe), and YT.Player
takes its callbacks as an object literal, which an effect event may not be passed into.
Fixed, not suppressed:
- useDismiss captured onClose when the popover opened, so a caller that re-rendered with a
different handler while it stayed open kept getting the OLD one called. The disabled dep list
asserted the opposite ("read fresh from this run's closure"). Both the handler and the ref
list are read through refs now; three tests, the two that matter mutation-checked.
- usePref wrapped its seed in a useCallback whose dep list had to be excused. useState calls a
lazy initializer exactly once, so the memo was saving nothing; inlined.
- useCardPager clamped an out-of-range page from an effect. Adjusted during render instead —
React discards that pass rather than committing it, so no out-of-range page is ever painted.
- plexPlayerHooks' auto-skip countdown reads its marker from the ref that already existed, so
the dep list names its real inputs. That also made the set-state-in-effect finding go away:
the progress is genuinely timer-driven, and the checker can now see that.
- useUrlMirror and useChipOverflowFit only needed their stable refs listed.
Two test-harness suppressions (a render-phase probe ref, a module-level setState handle) moved
inline with the reason they exist: both exist to observe a hook from outside without changing
what is being measured.
Suppressions 99 -> 89; exhaustive-deps 35 -> 27.
86 lines
3.2 KiB
TypeScript
86 lines
3.2 KiB
TypeScript
// @vitest-environment jsdom
|
|
//
|
|
// The outside-press half of useDismiss. The hook subscribes its `mousedown` listener once per
|
|
// open/close, which is what keeps a popover from re-subscribing on every parent render — but it
|
|
// means the listener must not CAPTURE the caller's handler or the ref LIST, or it keeps using the
|
|
// versions that existed when the popover opened. Both are read through refs (E27 S2); these tests
|
|
// are what says so.
|
|
import { useRef, useState } from "react";
|
|
import { describe, it, expect, vi, afterEach } from "vitest";
|
|
import { render, screen, fireEvent, cleanup } from "@testing-library/react";
|
|
|
|
import { useDismiss } from "./useDismiss";
|
|
|
|
afterEach(cleanup);
|
|
|
|
/** A panel that stays open while the caller swaps the handler it wants called. */
|
|
function SwappableHandler({ first, second }: { first: () => void; second: () => void }) {
|
|
const panel = useRef<HTMLDivElement>(null);
|
|
const [useSecond, setUseSecond] = useState(false);
|
|
useDismiss(true, useSecond ? second : first, panel);
|
|
return (
|
|
<div>
|
|
<div ref={panel}>panel</div>
|
|
<button onClick={() => setUseSecond(true)}>swap</button>
|
|
<div data-testid="outside">outside</div>
|
|
</div>
|
|
);
|
|
}
|
|
|
|
/** A panel that starts out registering one element and adds a second while it is open. */
|
|
function GrowingRefs({ onClose }: { onClose: () => void }) {
|
|
const panel = useRef<HTMLDivElement>(null);
|
|
const trigger = useRef<HTMLButtonElement>(null);
|
|
const [withTrigger, setWithTrigger] = useState(false);
|
|
useDismiss(true, onClose, withTrigger ? [panel, trigger] : [panel]);
|
|
return (
|
|
<div>
|
|
<div ref={panel}>panel</div>
|
|
<button onClick={() => setWithTrigger(true)}>register trigger</button>
|
|
<button ref={trigger}>trigger</button>
|
|
<div data-testid="outside">outside</div>
|
|
</div>
|
|
);
|
|
}
|
|
|
|
describe("useDismiss · outside press", () => {
|
|
it("closes on a press outside and stays put on a press inside", () => {
|
|
const onClose = vi.fn();
|
|
render(<SwappableHandler first={onClose} second={vi.fn()} />);
|
|
|
|
fireEvent.mouseDown(screen.getByText("panel"));
|
|
expect(onClose).not.toHaveBeenCalled();
|
|
|
|
fireEvent.mouseDown(screen.getByTestId("outside"));
|
|
expect(onClose).toHaveBeenCalledTimes(1);
|
|
});
|
|
|
|
it("calls the handler the caller has NOW, not the one it had when the popover opened", () => {
|
|
const first = vi.fn();
|
|
const second = vi.fn();
|
|
render(<SwappableHandler first={first} second={second} />);
|
|
|
|
fireEvent.click(screen.getByText("swap"));
|
|
fireEvent.mouseDown(screen.getByTestId("outside"));
|
|
|
|
expect(second).toHaveBeenCalledTimes(1);
|
|
expect(first).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("honours a ref the caller started passing after the popover opened", () => {
|
|
const onClose = vi.fn();
|
|
render(<GrowingRefs onClose={onClose} />);
|
|
|
|
fireEvent.mouseDown(screen.getByText("trigger"));
|
|
expect(onClose).toHaveBeenCalledTimes(1); // not registered yet — outside
|
|
|
|
onClose.mockClear();
|
|
fireEvent.click(screen.getByText("register trigger"));
|
|
fireEvent.mouseDown(screen.getByText("trigger"));
|
|
expect(onClose).not.toHaveBeenCalled(); // now it is one of ours
|
|
|
|
fireEvent.mouseDown(screen.getByTestId("outside"));
|
|
expect(onClose).toHaveBeenCalledTimes(1);
|
|
});
|
|
});
|