Skip to content

Commit 328e69a

Browse files
Oseltamiviradibarra
Oseltamivir
and
adibarra
authored
perf(charts): eliminate redundant D3 rebuilds at mount (#444)
* perf(charts): eliminate redundant D3 rebuilds at mount Profiling the live site showed every chart runs 2-3 full (visually identical) D3 teardown/rebuild passes right after mount, each ~300ms of main-thread time. Two unstable identities cause this: - useThemeColors re-set themeColors via setTimeout(0) on mount even though the synchronous useState read already saw the correct computed styles (next-themes applies the theme class pre-hydration). The new object identity invalidates getCssColor -> layers -> full rebuild. Now only an actual resolvedTheme change re-reads colors; the setTimeout(0) is kept for real switches so the <html> class flip settles first. - useResponsiveChartDimensions produced a new dimensions object from the ResizeObserver's initial callback (same width the ref callback just measured) and from same-value height updates. Same-size updates now keep the previous object so React bails out. Both fixes are identity-only: real theme switches and real resizes behave exactly as before. * perf(charts): cache iwanthue palettes per (vendor, theme, count, mode) iwanthue's force-vector clustering (quality 50 x 5 attempts) costs tens of milliseconds per call and showed up repeatedly in the profile's useMemo render paths whenever high-contrast colors recompute (mount, legend toggles, theme switches). The output is fully deterministic (seeded RNG) and independent of the key names - a vendor group's palette depends only on item count, vendor zone/ban mode, theme seed, and lightness bounds - so identical requests across renders, charts, and tabs now share one cached entry. Key space is tiny (vendors x themes x counts x 3 modes); no eviction needed. Existing color-quality tests (brand zones, bans, min distances) pass unchanged, confirming identical output. --------- Co-authored-by: Alec Ibarra <93070681+adibarra@users.noreply.github.com>
1 parent 1a31abd commit 328e69a

6 files changed

Lines changed: 480 additions & 31 deletions

File tree

‎packages/app/src/hooks/useResponsiveChartDimensions.test.ts‎

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
// @vitest-environment jsdom
2+
import { act, createElement } from 'react';
3+
import { createRoot, type Root } from 'react-dom/client';
4+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
5+
6+
import {
7+
useResponsiveChartDimensions,
8+
type UseResponsiveChartDimensionsResult,
9+
} from './useResponsiveChartDimensions';
10+
11+
// Minimal ResizeObserver stand-in — jsdom doesn't implement it. Tests fire
12+
// observations manually via `trigger`.
13+
class MockResizeObserver {
14+
static instances: MockResizeObserver[] = [];
15+
callback: ResizeObserverCallback;
16+
observed: Element[] = [];
17+
disconnected = false;
18+
19+
constructor(callback: ResizeObserverCallback) {
20+
this.callback = callback;
21+
MockResizeObserver.instances.push(this);
22+
}
23+
24+
observe(el: Element) {
25+
this.observed.push(el);
26+
}
27+
28+
disconnect() {
29+
this.disconnected = true;
30+
}
31+
32+
unobserve() {}
33+
34+
trigger(width: number) {
35+
act(() => {
36+
this.callback(
37+
[{ contentRect: { width } } as unknown as ResizeObserverEntry],
38+
this as unknown as ResizeObserver,
39+
);
40+
});
41+
}
42+
}
43+
44+
function renderHook<T>(hook: () => T): {
45+
result: { current: T };
46+
rerender: () => void;
47+
unmount: () => void;
48+
root: Root;
49+
} {
50+
const result = { current: undefined as unknown as T };
51+
function TestComponent() {
52+
result.current = hook();
53+
return null;
54+
}
55+
const container = document.createElement('div');
56+
document.body.append(container);
57+
const root = createRoot(container);
58+
const render = () => {
59+
act(() => {
60+
root.render(createElement(TestComponent));
61+
});
62+
};
63+
render();
64+
return {
65+
result,
66+
rerender: render,
67+
unmount: () => {
68+
act(() => root.unmount());
69+
container.remove();
70+
},
71+
root,
72+
};
73+
}
74+
75+
/** Create a container div whose getBoundingClientRect reports `width`. */
76+
function makeContainer(width: number): HTMLDivElement {
77+
const el = document.createElement('div');
78+
vi.spyOn(el, 'getBoundingClientRect').mockReturnValue({ width } as DOMRect);
79+
return el;
80+
}
81+
82+
beforeEach(() => {
83+
MockResizeObserver.instances = [];
84+
vi.stubGlobal('ResizeObserver', MockResizeObserver);
85+
});
86+
87+
afterEach(() => {
88+
vi.unstubAllGlobals();
89+
vi.restoreAllMocks();
90+
});
91+
92+
describe('useResponsiveChartDimensions', () => {
93+
it('measures the container on attach', () => {
94+
const { result, unmount } = renderHook<UseResponsiveChartDimensionsResult>(() =>
95+
useResponsiveChartDimensions({ height: 600 }),
96+
);
97+
98+
act(() => {
99+
result.current.setContainerRef(makeContainer(800));
100+
});
101+
102+
expect(result.current.dimensions).toEqual({ width: 800, height: 600 });
103+
unmount();
104+
});
105+
106+
it('keeps the dimensions object identity when an observation reports the same size', () => {
107+
const { result, unmount } = renderHook<UseResponsiveChartDimensionsResult>(() =>
108+
useResponsiveChartDimensions({ height: 600 }),
109+
);
110+
111+
act(() => {
112+
result.current.setContainerRef(makeContainer(800));
113+
});
114+
const initial = result.current.dimensions;
115+
116+
// ResizeObserver fires once right after observe() with the width the ref
117+
// callback already measured. The object identity must not change — a new
118+
// identity makes every chart treat it as a resize and fully rebuild.
119+
MockResizeObserver.instances.at(-1)!.trigger(800);
120+
121+
expect(result.current.dimensions).toBe(initial);
122+
unmount();
123+
});
124+
125+
it('updates dimensions when an observation reports a new width', () => {
126+
const { result, unmount } = renderHook<UseResponsiveChartDimensionsResult>(() =>
127+
useResponsiveChartDimensions({ height: 600 }),
128+
);
129+
130+
act(() => {
131+
result.current.setContainerRef(makeContainer(800));
132+
});
133+
const initial = result.current.dimensions;
134+
135+
MockResizeObserver.instances.at(-1)!.trigger(1024);
136+
137+
expect(result.current.dimensions).not.toBe(initial);
138+
expect(result.current.dimensions).toEqual({ width: 1024, height: 600 });
139+
unmount();
140+
});
141+
142+
it('disconnects the previous observer when the container changes', () => {
143+
const { result, unmount } = renderHook<UseResponsiveChartDimensionsResult>(() =>
144+
useResponsiveChartDimensions({ height: 600 }),
145+
);
146+
147+
act(() => {
148+
result.current.setContainerRef(makeContainer(800));
149+
});
150+
const first = MockResizeObserver.instances.at(-1)!;
151+
152+
act(() => {
153+
result.current.setContainerRef(makeContainer(640));
154+
});
155+
156+
expect(first.disconnected).toBe(true);
157+
expect(result.current.dimensions).toEqual({ width: 640, height: 600 });
158+
unmount();
159+
});
160+
161+
it('detaches cleanly when the container is removed', () => {
162+
const { result, unmount } = renderHook<UseResponsiveChartDimensionsResult>(() =>
163+
useResponsiveChartDimensions({ height: 600 }),
164+
);
165+
166+
act(() => {
167+
result.current.setContainerRef(makeContainer(800));
168+
});
169+
const observer = MockResizeObserver.instances.at(-1)!;
170+
171+
act(() => {
172+
result.current.setContainerRef(null);
173+
});
174+
175+
expect(observer.disconnected).toBe(true);
176+
// Last measured dimensions are retained (no reset to 0 on detach).
177+
expect(result.current.dimensions).toEqual({ width: 800, height: 600 });
178+
unmount();
179+
});
180+
});

‎packages/app/src/hooks/useResponsiveChartDimensions.ts‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,17 @@ export function useResponsiveChartDimensions(
3333
const [dimensions, setDimensions] = useState({ width: 0, height });
3434
const resizeObserverRef = useRef<ResizeObserver | null>(null);
3535

36+
// Keep the dimensions object referentially stable when nothing changed.
37+
// ResizeObserver fires once right after observe() with the same width the
38+
// ref callback just measured — without this guard that initial callback
39+
// produces a new object identity, which downstream chart effects treat as
40+
// a resize and answer with a full (visually identical) D3 rebuild.
41+
const updateDimensions = useCallback((width: number, h: number) => {
42+
setDimensions((prev) =>
43+
prev.width === width && prev.height === h ? prev : { width, height: h },
44+
);
45+
}, []);
46+
3647
// ref callback for initial dimension calculation and ResizeObserver setup
3748
const setContainerRef = useCallback(
3849
(element: HTMLDivElement | null) => {
@@ -47,20 +58,20 @@ export function useResponsiveChartDimensions(
4758
if (element) {
4859
// set initial dimensions
4960
const initialWidth = element.getBoundingClientRect().width;
50-
setDimensions({ width: initialWidth, height });
61+
updateDimensions(initialWidth, height);
5162

5263
// set up ResizeObserver
5364
resizeObserverRef.current = new ResizeObserver((entries) => {
5465
if (entries[0]) {
5566
const { width: observedWidth } = entries[0].contentRect;
56-
setDimensions({ width: observedWidth, height });
67+
updateDimensions(observedWidth, height);
5768
}
5869
});
5970

6071
resizeObserverRef.current.observe(element);
6172
}
6273
},
63-
[height],
74+
[height, updateDimensions],
6475
);
6576

6677
// clean up on unmount or height change
@@ -75,7 +86,7 @@ export function useResponsiveChartDimensions(
7586

7687
// update dimensions when height changes
7788
useEffect(() => {
78-
setDimensions((prev) => ({ ...prev, height }));
89+
setDimensions((prev) => (prev.height === height ? prev : { ...prev, height }));
7990
}, [height]);
8091

8192
return {

0 commit comments

Comments
 (0)