peter
332d24a646
Every one of them is the same shape: a value derived correctly, and a WRITE to the thing it derives from left unattended. Deriving hides a stale value; it does not consume it. PlayerModal — the watch toggle inherited the previous video's auto-watch. Moving the queue-advance sync into render was right for `currentVideoId`, but the same block also synced `status`, and that one had an ordering constraint of its own: useYtProgress's cleanup for the OUTGOING item can still call setWatched(true) near the end of a clip, and cleanups run after the commit. As an effect the sync landed last and corrected it; from render it lands first and the stray write wins. Press Next in the final ten seconds and the NEXT video showed as watched — one click on the toggle then sent onState(next, "new") for an item already new. Fixed by keying the mirror to the id it describes, so a write for another item is ignored rather than displayed, and no ordering has to be argued about at all. ChannelsProvider — a focus-channel intent was masked by the page rather than consumed, and nothing ever cleared it. The manager is a page chunk: it unmounts on every navigation, and its seed effect runs again on each mount, so returning to it re-applied a channel name from earlier in the session over whatever the user had typed since. The consumed marker now lives in the provider, keyed on the token — which also closes the case the old clear-on-leave effect never handled, opening a channel page (that does not change `page` at all). TagFilterChips + ChannelTags x2 — `open = openRaw && hidden > 0` hides the overflow popover when the lane stops overflowing, but floating-ui only reports a close for a real event on a mounted surface, so the raw flag stayed true and the menu came back by itself on the next re-fit — a window resize or the text-size slider is enough, with nobody near it. The flag is now reset in render against the same condition that hides it. Avatar + downloadRow — remembering a failed image BY URL fixed the recycled-row frame but never forgot it, so a transient 429 pinned a channel to its initial for the life of the instance, never re-requesting. The memory is now dropped once `src` moves off it, which keeps both properties. And playerModalHooks' last exhaustive-deps suppression claimed "nothing in the body reads a render value" while reading five. The invariant that is actually load-bearing is that each of them is a per-item snapshot — written down instead. All four fixes are mutation-checked: each new test was seen red, against the fixed line only, naming the real defect.
85 lines
4.1 KiB
TypeScript
85 lines
4.1 KiB
TypeScript
// @vitest-environment jsdom
|
|
import { useLayoutEffect } from "react";
|
|
import { describe, it, expect, afterEach } from "vitest";
|
|
import { render, screen, fireEvent, cleanup } from "@testing-library/react";
|
|
|
|
import Avatar from "./Avatar";
|
|
|
|
afterEach(cleanup);
|
|
|
|
// The failure state is keyed by URL rather than held as a boolean an effect resets, and the reason
|
|
// is a frame the user could see: Avatar is recycled across items in the virtualized feed, so after
|
|
// ONE image 429s (Google's avatar CDNs throttle bursts — see the component's own note), the next
|
|
// item to land in that slot rendered with its new `src` and the previous item's `failed`. The
|
|
// fallback initial was therefore committed and painted over a perfectly good image, and only the
|
|
// post-paint effect put it right.
|
|
//
|
|
// `useLayoutEffect` is what makes that observable instead of a race: layout effects run against the
|
|
// committed DOM BEFORE any passive effect, so the snapshot it takes is exactly the frame the browser
|
|
// would paint — the one the old reset effect had not repaired yet.
|
|
function Probe({ src, painted }: { src: string; painted: string[] }) {
|
|
useLayoutEffect(() => {
|
|
painted.push(document.body.innerHTML);
|
|
});
|
|
return <Avatar src={src} alt="avatar" fallback="Zed" />;
|
|
}
|
|
|
|
describe("Avatar · a failed image must not follow the component to the next item", () => {
|
|
it("shows the new src in the SAME commit that changes it, with no fallback frame between", () => {
|
|
const painted: string[] = [];
|
|
const { rerender } = render(<Probe src="https://cdn.test/a.jpg" painted={painted} />);
|
|
|
|
fireEvent.error(screen.getByRole("img", { name: "avatar" }));
|
|
expect(screen.queryByRole("img")).toBeNull();
|
|
expect(screen.getByText("Z")).toBeTruthy();
|
|
|
|
painted.length = 0;
|
|
rerender(<Probe src="https://cdn.test/b.jpg" painted={painted} />);
|
|
|
|
// The FIRST frame after the src change already carries the image. Reading the last snapshot
|
|
// instead would let a passive-effect repair pass, which is precisely the bug.
|
|
expect(painted.length).toBeGreaterThan(0);
|
|
expect(painted[0]).toContain("https://cdn.test/b.jpg");
|
|
expect(painted[0]).not.toContain(">Z<");
|
|
});
|
|
|
|
// CHARACTERIZATION: the old boolean-plus-effect passes this too (the rerender is wrapped in act(),
|
|
// so its reset effect has run by the time the src is read). The frame-level guard is the test
|
|
// above, which the old shape fails; the retry guard is the test below, which it also fails.
|
|
it("keeps the fallback for the URL that actually failed, not for whatever is current", () => {
|
|
const { rerender } = render(
|
|
<Avatar src="https://cdn.test/a.jpg" alt="avatar" fallback="Zed" />,
|
|
);
|
|
fireEvent.error(screen.getByRole("img", { name: "avatar" }));
|
|
expect(screen.getByText("Z")).toBeTruthy();
|
|
|
|
rerender(<Avatar src="https://cdn.test/b.jpg" alt="avatar" fallback="Zed" />);
|
|
expect(screen.getByRole("img", { name: "avatar" }).getAttribute("src")).toBe(
|
|
"https://cdn.test/b.jpg",
|
|
);
|
|
});
|
|
|
|
it("retries a URL it comes back to — a 429 is transient, and so is the memory of it", () => {
|
|
// The instance is not remounted here on purpose: the player's channel avatar and the Messages
|
|
// thread header both swap `src` on a live instance. A memory that outlived the reason for it
|
|
// would pin a throttled channel to its initial for the rest of the session.
|
|
const { rerender } = render(
|
|
<Avatar src="https://cdn.test/a.jpg" alt="avatar" fallback="Zed" />,
|
|
);
|
|
fireEvent.error(screen.getByRole("img", { name: "avatar" }));
|
|
expect(screen.getByText("Z")).toBeTruthy();
|
|
|
|
rerender(<Avatar src="https://cdn.test/b.jpg" alt="avatar" fallback="Zed" />);
|
|
rerender(<Avatar src="https://cdn.test/a.jpg" alt="avatar" fallback="Zed" />);
|
|
expect(screen.getByRole("img", { name: "avatar" }).getAttribute("src")).toBe(
|
|
"https://cdn.test/a.jpg",
|
|
);
|
|
});
|
|
|
|
it("falls back when there is no src at all", () => {
|
|
render(<Avatar src={null} alt="avatar" fallback="Zed" />);
|
|
expect(screen.queryByRole("img")).toBeNull();
|
|
expect(screen.getByText("Z")).toBeTruthy();
|
|
});
|
|
});
|