mirror of
https://gh-proxy.org/https://github.com/klinsmaya/multica.git
synced 2026-11-08 17:36:45 +08:00
perf(ui): coalesce useScrollFade observers and skip identical setState (MUL-7987) (#9207)
* perf(ui): coalesce useScrollFade observers and skip identical setState Every fade host (sidebar groups, the tab bar, dialogs) observed scroll, resize AND childList:subtree mutations, and each observer hit synchronously read layout and dispatched a setState even when the computed fade value was unchanged. In busy workspaces with frequent realtime re-renders, a route remount fired hundreds of identical dispatchSetState calls per second across the shell's fade hosts — every one going through React's lane scheduler — stretching tab switches and detail-page opens out to several seconds. A production CPU profile of the stall showed roughly half of all samples inside that dispatchSetState path. Coalesce the three sources through one requestAnimationFrame per host and only dispatch when the computed value actually changes. The first frame after mount still applies the initial mask, so the visual behavior is unchanged. Also gives packages/ui a minimal vitest setup (jsdom + @testing-library, matching packages/core and packages/views) with coverage for the mask values and a regression test asserting that a mutation/scroll storm with unchanged layout reaches React zero times. * fix(ui): keep useScrollFade scroll handling synchronous Coalescing scroll through rAF broke chat's auto-scroll handshake: its stick-to-bottom latch attributes wheel displacement to the scroll event that confirms it within the same task, and deferring the fade update (and its re-render) off that task left followOutput stale in the autoscroll suite. Scroll stays synchronous — user scrolling is too rare to need coalescing, and the storms this hook guards against come from resizes and subtree mutations, which remain rAF-coalesced. Full packages/views suite (5981 tests) passes with this split. * fix(ui): resolve useScrollFade lifecycle defects from review Two defects from the PR review: 1. The cleanup removed the wrong closure — the listener was registered as `run` but unregistered as `schedule`, so StrictMode remounts and axis changes left the old measurement attached (double reads, stale axis). Remove `run` symmetrically. 2. A synchronous scroll executed `raf = 0` while a queued frame was still alive: the frame fired anyway (resize→scroll→mutation yielded two pending frames instead of one) and survived unmount cleanup, which saw the zeroed handle. `run` now cancels the queued frame up front — a synchronous measurement absorbs it. Also refreshes the header comment that still claimed all three sources go through rAF, and adds regression tests for the three review scenarios (absorb queued frame, zero frames after unmount, listener symmetry across cleanup and axis rebuild). * chore(ui): drop unused no-var eslint directive in test setup Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Multica Eve <[email protected]> Co-authored-by: multica-agent <[email protected]>
This commit is contained in:
苗大
co-authored by
multica-agent
Multica Eve
multica-agent
Multica Eve
parent
9a50d0cc65
commit
e2e237a22f
@@ -0,0 +1,208 @@
|
||||
// @vitest-environment jsdom
|
||||
import { render, act } from "@testing-library/react";
|
||||
import { type CSSProperties, useRef, useState } from "react";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { useScrollFade } from "./use-scroll-fade";
|
||||
|
||||
/** jsdom reports every box as 0×0; these getters stand in for real layout. */
|
||||
function fakeScrollBox(
|
||||
el: HTMLElement,
|
||||
box: { client: number; scroll: number; position?: number },
|
||||
): void {
|
||||
Object.defineProperty(el, "clientHeight", { configurable: true, value: box.client });
|
||||
Object.defineProperty(el, "scrollHeight", { configurable: true, value: box.scroll });
|
||||
Object.defineProperty(el, "scrollTop", {
|
||||
configurable: true,
|
||||
value: box.position ?? 0,
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* The hook measures through three layout getters per run; wrap the fakes so
|
||||
* each measurement bumps a counter the lifecycle tests can assert against.
|
||||
* `fakeScrollBox` defines these configurable, so the wrappers stack.
|
||||
*/
|
||||
function trackMeasures(el: HTMLElement): () => number {
|
||||
let count = 0;
|
||||
for (const prop of ["scrollTop", "scrollHeight", "clientHeight"]) {
|
||||
const desc = Object.getOwnPropertyDescriptor(el, prop)!;
|
||||
const value = desc.value;
|
||||
Object.defineProperty(el, prop, {
|
||||
configurable: desc.configurable,
|
||||
enumerable: desc.enumerable,
|
||||
get() {
|
||||
count += 1;
|
||||
return typeof value === "function" ? value.call(el) : value;
|
||||
},
|
||||
});
|
||||
}
|
||||
return () => count;
|
||||
}
|
||||
|
||||
// jsdom's cssstyle drops `mask-image`, so assert on the hook's return value.
|
||||
function Host({
|
||||
box,
|
||||
axis,
|
||||
onRender,
|
||||
onStyle,
|
||||
}: {
|
||||
box: { client: number; scroll: number; position?: number };
|
||||
axis?: "vertical" | "horizontal";
|
||||
onRender?: () => void;
|
||||
onStyle?: (style: CSSProperties | undefined) => void;
|
||||
}) {
|
||||
const ref = useRef<HTMLDivElement>(null);
|
||||
const [, bump] = useState(0);
|
||||
const style = useScrollFade(ref, 32, axis ?? "vertical");
|
||||
onRender?.();
|
||||
onStyle?.(style);
|
||||
return (
|
||||
<div
|
||||
ref={(node) => {
|
||||
ref.current = node;
|
||||
if (node) fakeScrollBox(node, box);
|
||||
}}
|
||||
>
|
||||
<button type="button" onClick={() => bump((n) => n + 1)}>
|
||||
bump
|
||||
</button>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
describe("useScrollFade", () => {
|
||||
it("returns no mask for a non-scrollable box", () => {
|
||||
const styles: (CSSProperties | undefined)[] = [];
|
||||
render(<Host box={{ client: 100, scroll: 100 }} onStyle={(s) => styles.push(s)} />);
|
||||
act(() => flushRaf());
|
||||
expect(styles.at(-1)).toBeUndefined();
|
||||
});
|
||||
|
||||
it("returns the end fade at the top of a scrollable box", () => {
|
||||
const styles: (CSSProperties | undefined)[] = [];
|
||||
render(<Host box={{ client: 100, scroll: 400 }} onStyle={(s) => styles.push(s)} />);
|
||||
act(() => flushRaf());
|
||||
expect(String(styles.at(-1)?.maskImage)).toContain("transparent 100%");
|
||||
});
|
||||
|
||||
it("keeps the fade value when the box overflows and position does not change", () => {
|
||||
// Mount and resize storms used to dispatch an identical setState per
|
||||
// observer hit — hundreds per second across the shell's fade hosts.
|
||||
// Identical values must not reach React at all.
|
||||
const renders = vi.fn();
|
||||
const styles: (CSSProperties | undefined)[] = [];
|
||||
const { container } = render(
|
||||
<Host box={{ client: 100, scroll: 400 }} onRender={renders} onStyle={(s) => styles.push(s)} />,
|
||||
);
|
||||
act(() => flushRaf());
|
||||
const afterMount = renders.mock.calls.length;
|
||||
const box = container.querySelector("div")!;
|
||||
// Simulate the mount/realtime storm: far more mutations and scroll events
|
||||
// than frames, with layout unchanged throughout.
|
||||
act(() => {
|
||||
for (let i = 0; i < 50; i++) {
|
||||
box.dispatchEvent(new Event("scroll"));
|
||||
box.appendChild(document.createElement("span"));
|
||||
box.removeChild(box.lastChild!);
|
||||
}
|
||||
flushRaf();
|
||||
});
|
||||
expect(renders.mock.calls.length).toBe(afterMount);
|
||||
expect(String(styles.at(-1)?.maskImage)).toContain("transparent 100%");
|
||||
});
|
||||
|
||||
it("absorbs the queued resize frame when a synchronous scroll lands first", async () => {
|
||||
// Audit scenario: resize queues frame A, a synchronous scroll measures
|
||||
// right away and must CANCEL A (not just drop its handle), then a
|
||||
// mutation queues frame B — exactly one frame pending, two measurements
|
||||
// total.
|
||||
const { container } = render(<Host box={{ client: 100, scroll: 400 }} />);
|
||||
act(() => flushRaf());
|
||||
const el = container.querySelector("div")!;
|
||||
const measures = trackMeasures(el);
|
||||
act(() => {
|
||||
triggerResizeObservers();
|
||||
expect(pendingRaf()).toBe(1);
|
||||
el.dispatchEvent(new Event("scroll"));
|
||||
expect(pendingRaf()).toBe(0);
|
||||
});
|
||||
// jsdom dispatches MutationObserver callbacks on a microtask.
|
||||
await act(async () => {
|
||||
el.appendChild(document.createElement("span"));
|
||||
await Promise.resolve();
|
||||
});
|
||||
act(() => {
|
||||
expect(pendingRaf()).toBe(1);
|
||||
flushRaf();
|
||||
});
|
||||
expect(measures()).toBe(6); // scroll + frame B, 3 layout reads each
|
||||
});
|
||||
|
||||
it("leaves no queued frame behind when unmounting after a synchronous scroll", () => {
|
||||
// Audit scenario: resize queued a frame, scroll absorbed it, unmount —
|
||||
// nothing may fire afterwards (the old handle drop let the frame
|
||||
// survive cleanup).
|
||||
const { container, unmount } = render(<Host box={{ client: 100, scroll: 400 }} />);
|
||||
act(() => flushRaf());
|
||||
const el = container.querySelector("div")!;
|
||||
const measures = trackMeasures(el);
|
||||
act(() => {
|
||||
triggerResizeObservers();
|
||||
el.dispatchEvent(new Event("scroll"));
|
||||
});
|
||||
unmount();
|
||||
expect(pendingRaf()).toBe(0);
|
||||
act(() => flushRaf());
|
||||
expect(measures()).toBe(3); // only the synchronous scroll measured
|
||||
});
|
||||
|
||||
it("removes the scroll listener on cleanup", () => {
|
||||
// Audit scenario: the cleanup used to remove `schedule` while the
|
||||
// listener was registered as `run`, leaving the measurement attached
|
||||
// across StrictMode remounts and axis changes.
|
||||
const { container, unmount } = render(<Host box={{ client: 100, scroll: 400 }} />);
|
||||
act(() => flushRaf());
|
||||
const el = container.querySelector("div")!;
|
||||
const measures = trackMeasures(el);
|
||||
unmount();
|
||||
const before = measures();
|
||||
el.dispatchEvent(new Event("scroll"));
|
||||
expect(measures()).toBe(before);
|
||||
});
|
||||
|
||||
it("rebuilds with the new axis and drops the old scroll listener", () => {
|
||||
const styles: (CSSProperties | undefined)[] = [];
|
||||
const { container, rerender } = render(
|
||||
<Host axis="vertical" box={{ client: 100, scroll: 400 }} onStyle={(s) => styles.push(s)} />,
|
||||
);
|
||||
act(() => flushRaf());
|
||||
rerender(<Host axis="horizontal" box={{ client: 100, scroll: 400 }} onStyle={(s) => styles.push(s)} />);
|
||||
act(() => flushRaf());
|
||||
const el = container.querySelector("div")!;
|
||||
const measures = trackMeasures(el);
|
||||
const before = measures();
|
||||
el.dispatchEvent(new Event("scroll"));
|
||||
// The old axis reads scrollTop/scrollHeight/clientHeight; a leftover
|
||||
// listener from the vertical closure would bump this counter. The new
|
||||
// horizontal run sees no overflow in jsdom and renders no mask.
|
||||
expect(measures() - before).toBe(0);
|
||||
expect(styles.at(-1)).toBeUndefined();
|
||||
});
|
||||
|
||||
it("updates the fade when the scroll position crosses a boundary", () => {
|
||||
const styles: (CSSProperties | undefined)[] = [];
|
||||
const { container } = render(
|
||||
<Host box={{ client: 100, scroll: 400 }} onStyle={(s) => styles.push(s)} />,
|
||||
);
|
||||
act(() => flushRaf());
|
||||
const box = container.querySelector("div")!;
|
||||
act(() => {
|
||||
fakeScrollBox(box, { client: 100, scroll: 400, position: 150 });
|
||||
box.dispatchEvent(new Event("scroll"));
|
||||
flushRaf();
|
||||
});
|
||||
const mask = String(styles.at(-1)?.maskImage ?? "");
|
||||
expect(mask).toContain("transparent 0%");
|
||||
expect(mask).toContain("transparent 100%");
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,4 @@
|
||||
import { type RefObject, type CSSProperties, useEffect, useState, useCallback } from "react";
|
||||
import { type RefObject, type CSSProperties, useEffect, useState } from "react";
|
||||
|
||||
export type ScrollFadeAxis = "vertical" | "horizontal";
|
||||
|
||||
@@ -16,52 +16,71 @@ export function useScrollFade(
|
||||
): CSSProperties | undefined {
|
||||
const [fade, setFade] = useState<"none" | "start" | "end" | "both">("none");
|
||||
|
||||
const update = useCallback(() => {
|
||||
const el = ref.current;
|
||||
if (!el) return;
|
||||
|
||||
const position = axis === "horizontal" ? el.scrollLeft : el.scrollTop;
|
||||
const scrollSize = axis === "horizontal" ? el.scrollWidth : el.scrollHeight;
|
||||
const clientSize = axis === "horizontal" ? el.clientWidth : el.clientHeight;
|
||||
const scrollable = scrollSize - clientSize;
|
||||
|
||||
if (scrollable <= 0) {
|
||||
setFade("none");
|
||||
return;
|
||||
}
|
||||
|
||||
const atStart = position <= 1;
|
||||
const atEnd = position >= scrollable - 1;
|
||||
|
||||
if (atStart && atEnd) setFade("none");
|
||||
else if (atStart) setFade("end");
|
||||
else if (atEnd) setFade("start");
|
||||
else setFade("both");
|
||||
}, [axis, ref]);
|
||||
|
||||
useEffect(() => {
|
||||
const el = ref.current;
|
||||
if (!el) return;
|
||||
|
||||
const frame = requestAnimationFrame(update);
|
||||
// Resizes and child mutations funnel through one rAF-coalesced run
|
||||
// (scroll stays synchronous — see its listener below). Each run reads
|
||||
// layout once and calls setFade only when the value actually changes:
|
||||
// with dozens of fade hosts on screen, a mount or a realtime-event
|
||||
// re-render used to fire hundreds of identical dispatchSetState calls
|
||||
// per second — every one of them went through React's lane
|
||||
// scheduler and stretched tab switches to seconds.
|
||||
let last: "none" | "start" | "end" | "both" | null = null;
|
||||
let raf = 0;
|
||||
const run = () => {
|
||||
// A synchronous scroll absorbs any queued frame: this measurement
|
||||
// covers it, and leaving the handle alive would both double-fire the
|
||||
// callback and survive unmount cleanup (which sees raf === 0).
|
||||
if (raf) {
|
||||
cancelAnimationFrame(raf);
|
||||
raf = 0;
|
||||
}
|
||||
const node = ref.current;
|
||||
if (!node) return;
|
||||
const position = axis === "horizontal" ? node.scrollLeft : node.scrollTop;
|
||||
const scrollSize = axis === "horizontal" ? node.scrollWidth : node.scrollHeight;
|
||||
const clientSize = axis === "horizontal" ? node.clientWidth : node.clientHeight;
|
||||
const scrollable = scrollSize - clientSize;
|
||||
const atStart = position <= 1;
|
||||
const atEnd = position >= scrollable - 1;
|
||||
const next: "none" | "start" | "end" | "both" =
|
||||
scrollable <= 0 || (atStart && atEnd) ? "none" : atStart ? "end" : atEnd ? "start" : "both";
|
||||
if (next !== last) {
|
||||
last = next;
|
||||
setFade(next);
|
||||
}
|
||||
};
|
||||
const schedule = () => {
|
||||
if (!raf) raf = requestAnimationFrame(run);
|
||||
};
|
||||
|
||||
el.addEventListener("scroll", update, { passive: true });
|
||||
const ro = new ResizeObserver(update);
|
||||
schedule();
|
||||
// Scroll stays synchronous on purpose. Consumer wiring treats a scroll
|
||||
// and its fade verdict as one task — chat's stick-to-bottom latch, for
|
||||
// one, attributes wheel displacement to the scroll that confirms it, and
|
||||
// deferring the fade update off that task reordered the handshake in a
|
||||
// way its tests (and real scrolling feedback) depend on not happening.
|
||||
// User scrolling is also far too rare to need coalescing; the storms come
|
||||
// from resizes and DOM mutations below.
|
||||
el.addEventListener("scroll", run, { passive: true });
|
||||
const ro = new ResizeObserver(schedule);
|
||||
ro.observe(el);
|
||||
// ResizeObserver only fires on the container's own box. When children
|
||||
// grow inside a flex/auto-sized parent, the scroll extent can change while
|
||||
// the viewport does not — the mask would stay "none" until the user scrolls.
|
||||
// MutationObserver on childList catches those content insertions.
|
||||
const mo = new MutationObserver(update);
|
||||
const mo = new MutationObserver(schedule);
|
||||
mo.observe(el, { childList: true, subtree: true });
|
||||
|
||||
return () => {
|
||||
cancelAnimationFrame(frame);
|
||||
el.removeEventListener("scroll", update);
|
||||
if (raf) cancelAnimationFrame(raf);
|
||||
el.removeEventListener("scroll", run);
|
||||
ro.disconnect();
|
||||
mo.disconnect();
|
||||
};
|
||||
}, [ref, update]);
|
||||
}, [axis, ref]);
|
||||
|
||||
if (fade === "none") return undefined;
|
||||
|
||||
|
||||
@@ -5,7 +5,8 @@
|
||||
"type": "module",
|
||||
"scripts": {
|
||||
"typecheck": "tsc --noEmit",
|
||||
"lint": "eslint ."
|
||||
"lint": "eslint .",
|
||||
"test": "vitest run"
|
||||
},
|
||||
"exports": {
|
||||
"./components/ui/*": "./components/ui/*.tsx",
|
||||
@@ -69,10 +70,15 @@
|
||||
"devDependencies": {
|
||||
"@multica/eslint-config": "workspace:*",
|
||||
"@multica/tsconfig": "workspace:*",
|
||||
"@testing-library/jest-dom": "catalog:",
|
||||
"@testing-library/react": "catalog:",
|
||||
"@types/linkify-it": "^5.0.0",
|
||||
"@types/react": "catalog:",
|
||||
"@types/react-dom": "catalog:",
|
||||
"@vitejs/plugin-react": "catalog:",
|
||||
"esbuild": "^0.25.12",
|
||||
"typescript": "catalog:"
|
||||
"jsdom": "catalog:",
|
||||
"typescript": "catalog:",
|
||||
"vitest": "catalog:"
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,56 @@
|
||||
import { afterEach } from "vitest";
|
||||
import "@testing-library/jest-dom/vitest";
|
||||
|
||||
// jsdom ships neither class. use-scroll-fade observes both.
|
||||
const roInstances: ResizeObserverStub[] = [];
|
||||
|
||||
class ResizeObserverStub implements ResizeObserver {
|
||||
static triggerAll(): void {
|
||||
for (const instance of [...roInstances]) instance.callback([], instance);
|
||||
}
|
||||
|
||||
constructor(private readonly callback: ResizeObserverCallback) {
|
||||
roInstances.push(this);
|
||||
}
|
||||
|
||||
observe(): void {}
|
||||
|
||||
unobserve(): void {}
|
||||
|
||||
disconnect(): void {}
|
||||
}
|
||||
|
||||
if (typeof globalThis.ResizeObserver === "undefined") {
|
||||
globalThis.ResizeObserver = ResizeObserverStub;
|
||||
}
|
||||
|
||||
// jsdom never runs its own frame loop. Queue callbacks so tests can inspect
|
||||
// exactly how many frames are pending and flush one batch at a time.
|
||||
const rafQueue: FrameRequestCallback[] = [];
|
||||
globalThis.requestAnimationFrame = (cb: FrameRequestCallback): number => {
|
||||
rafQueue.push(cb);
|
||||
return rafQueue.length;
|
||||
};
|
||||
globalThis.cancelAnimationFrame = (handle: number): void => {
|
||||
const index = handle - 1;
|
||||
if (index >= 0 && index < rafQueue.length) rafQueue[index] = null as unknown as FrameRequestCallback;
|
||||
};
|
||||
globalThis.flushRaf = (): void => {
|
||||
const pending = rafQueue.splice(0, rafQueue.length);
|
||||
for (const cb of pending) cb?.(performance.now());
|
||||
};
|
||||
globalThis.pendingRaf = (): number =>
|
||||
rafQueue.filter((cb) => typeof cb === "function").length;
|
||||
|
||||
declare global {
|
||||
var flushRaf: () => void;
|
||||
var pendingRaf: () => number;
|
||||
var triggerResizeObservers: () => void;
|
||||
}
|
||||
|
||||
globalThis.triggerResizeObservers = ResizeObserverStub.triggerAll;
|
||||
|
||||
afterEach(() => {
|
||||
rafQueue.length = 0;
|
||||
roInstances.length = 0;
|
||||
});
|
||||
@@ -0,0 +1,12 @@
|
||||
import { defineConfig } from "vitest/config";
|
||||
import react from "@vitejs/plugin-react";
|
||||
|
||||
export default defineConfig({
|
||||
plugins: [react()],
|
||||
test: {
|
||||
environment: "jsdom",
|
||||
globals: true,
|
||||
setupFiles: ["./test/setup.ts"],
|
||||
include: ["**/*.test.{ts,tsx}"],
|
||||
},
|
||||
});
|
||||
Generated
+83
-22
Reference in New Issue
Block a user