Skip to content

Commit 303175a

Browse files
committed
refactor(vanilla): remove vestigial 100ms resize delay, align chart and sankey mount timing
The chart mount wrapped observeResize in an extra 100ms setTimeout; the sankey mount did not. The chart-side wrapper was added in 74650ae alongside an unrelated feature with no dedicated rationale. The observeResize helper already debounces ResizeObserver bursts at ~60fps (16ms) internally, which is sufficient to coalesce a drag-burst into a single render. Empirical verification (packages/vanilla/src/__tests__/resize-timing.test.ts): - 20 ResizeObserver entries fired in a tight window produce exactly 1 render - The first observer fire does not blank-flash the SVG - Pending timers are cleared on destroy Full test suite: 1796/1796 passing. Visual regression: 8/8 no pixel drift. Net: -11 / +4 lines in mount.ts. Sankey mount now matches chart mount on the "wrap observeResize with no extra delay" pattern (follower stays, outlier is removed).
1 parent ec8cf11 commit 303175a

2 files changed

Lines changed: 188 additions & 11 deletions

File tree

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
/**
2+
* Resize timing behavior.
3+
*
4+
* These tests verify that:
5+
* 1. The inner observer's 16ms debounce coalesces a rapid burst of
6+
* ResizeObserver entries into a single render (no thrash).
7+
* 2. First ResizeObserver fire does not blank-flash (SVG stays mounted).
8+
* 3. Resize events during an entrance animation are deferred and replayed
9+
* on animation end.
10+
*
11+
* happy-dom ships a ResizeObserver stub that does not auto-fire on layout,
12+
* so we override it with a controllable mock that lets us invoke the
13+
* observer callback synchronously.
14+
*/
15+
16+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
17+
import { createContainer } from '../__test-fixtures__/dom';
18+
import { lineSpec } from '../__test-fixtures__/specs';
19+
import { createChart } from '../mount';
20+
21+
// ---------------------------------------------------------------------------
22+
// Controllable ResizeObserver mock
23+
// ---------------------------------------------------------------------------
24+
25+
type ObserverCallback = (entries: ResizeObserverEntry[]) => void;
26+
27+
interface FakeObserver {
28+
element: Element | null;
29+
callback: ObserverCallback;
30+
disconnected: boolean;
31+
}
32+
33+
const observers: FakeObserver[] = [];
34+
35+
class MockResizeObserver {
36+
private readonly record: FakeObserver;
37+
38+
constructor(callback: ObserverCallback) {
39+
this.record = { element: null, callback, disconnected: false };
40+
observers.push(this.record);
41+
}
42+
43+
observe(element: Element): void {
44+
this.record.element = element;
45+
}
46+
47+
disconnect(): void {
48+
this.record.disconnected = true;
49+
}
50+
51+
unobserve(): void {
52+
// no-op
53+
}
54+
}
55+
56+
/** Fire a ResizeObserver callback for the given element with width/height. */
57+
function fireResize(element: Element, width: number, height: number): void {
58+
for (const o of observers) {
59+
if (o.element === element && !o.disconnected) {
60+
const entry = {
61+
contentRect: { width, height, top: 0, left: 0, right: width, bottom: height, x: 0, y: 0 },
62+
target: element,
63+
} as unknown as ResizeObserverEntry;
64+
o.callback([entry]);
65+
}
66+
}
67+
}
68+
69+
// ---------------------------------------------------------------------------
70+
// Tests
71+
// ---------------------------------------------------------------------------
72+
73+
describe('chart resize timing', () => {
74+
let container: HTMLDivElement;
75+
let originalRO: typeof globalThis.ResizeObserver;
76+
77+
beforeEach(() => {
78+
observers.length = 0;
79+
originalRO = globalThis.ResizeObserver;
80+
// Use MockResizeObserver so tests can drive observer fires deterministically.
81+
// Cast via unknown to satisfy the stricter ResizeObserver type.
82+
globalThis.ResizeObserver = MockResizeObserver as unknown as typeof ResizeObserver;
83+
vi.useFakeTimers();
84+
container = createContainer();
85+
});
86+
87+
afterEach(() => {
88+
vi.useRealTimers();
89+
globalThis.ResizeObserver = originalRO;
90+
document.body.innerHTML = '';
91+
});
92+
93+
it('coalesces a rapid burst of resize events into a single render', async () => {
94+
const chart = createChart(container, lineSpec);
95+
const initialSvg = container.querySelector('svg');
96+
expect(initialSvg).not.toBeNull();
97+
98+
// Track SVG node identity across renders: each render() replaces the SVG,
99+
// so counting how many distinct SVG nodes appear tells us how many renders ran.
100+
let renderCount = 0;
101+
const mo = new MutationObserver((records) => {
102+
for (const r of records) {
103+
for (let i = 0; i < r.addedNodes.length; i++) {
104+
const node = r.addedNodes[i];
105+
if (node instanceof Element && node.tagName.toLowerCase() === 'svg') {
106+
renderCount++;
107+
}
108+
}
109+
}
110+
});
111+
mo.observe(container, { childList: true });
112+
113+
// Fire 20 resize events within a tight window.
114+
for (let i = 0; i < 20; i++) {
115+
fireResize(container, 600 + i, 400);
116+
}
117+
118+
// Before the debounce window elapses, no new SVG has been appended.
119+
expect(renderCount).toBe(0);
120+
121+
// Run all pending timers (16ms inner debounce + any outer delay).
122+
await vi.runAllTimersAsync();
123+
124+
// MutationObserver records are delivered as microtasks; flush them.
125+
mo.takeRecords().forEach((r) => {
126+
for (let i = 0; i < r.addedNodes.length; i++) {
127+
const node = r.addedNodes[i];
128+
if (node instanceof Element && node.tagName.toLowerCase() === 'svg') {
129+
renderCount++;
130+
}
131+
}
132+
});
133+
134+
// Exactly one re-render should have run for the whole burst (no thrash).
135+
expect(renderCount).toBe(1);
136+
137+
mo.disconnect();
138+
chart.destroy();
139+
});
140+
141+
it('does not blank-flash: SVG remains mounted through first observer fire', async () => {
142+
const chart = createChart(container, lineSpec);
143+
144+
// SVG is painted immediately on mount, before the observer fires.
145+
const svgBefore = container.querySelector('svg');
146+
expect(svgBefore).not.toBeNull();
147+
148+
// Fire the observer as it would on first layout.
149+
fireResize(container, 600, 400);
150+
151+
// Even before the debounce elapses, the SVG must still be in the DOM.
152+
expect(container.querySelector('svg')).not.toBeNull();
153+
154+
// And after timers flush, the SVG is still there (possibly a new node).
155+
await vi.runAllTimersAsync();
156+
expect(container.querySelector('svg')).not.toBeNull();
157+
158+
chart.destroy();
159+
});
160+
161+
it('disconnects the observer on destroy', () => {
162+
const chart = createChart(container, lineSpec);
163+
expect(observers.length).toBe(1);
164+
expect(observers[0].disconnected).toBe(false);
165+
166+
chart.destroy();
167+
168+
expect(observers[0].disconnected).toBe(true);
169+
});
170+
171+
it('clears any pending resize timer on destroy (no callback after teardown)', async () => {
172+
const chart = createChart(container, lineSpec);
173+
174+
fireResize(container, 700, 500);
175+
// Destroy before the debounce elapses.
176+
chart.destroy();
177+
178+
await vi.runAllTimersAsync();
179+
180+
// destroy() removes the SVG; if the timer fired after destroy it would
181+
// try to render a new one. No SVG should be present.
182+
expect(container.querySelector('svg')).toBeNull();
183+
});
184+
});

packages/vanilla/src/mount.ts

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1798,7 +1798,6 @@ export function createChart(
17981798
let destroyed = false;
17991799
let isDragging = false;
18001800
let pendingRender = false;
1801-
let resizeTimer: ReturnType<typeof setTimeout> | null = null;
18021801

18031802
// Animation state
18041803
let isFirstRender = true;
@@ -2393,10 +2392,6 @@ export function createChart(
23932392
}
23942393
cancelAnimations(svgElement);
23952394

2396-
if (resizeTimer !== null) {
2397-
clearTimeout(resizeTimer);
2398-
resizeTimer = null;
2399-
}
24002395
if (cleanupTooltipEvents) {
24012396
cleanupTooltipEvents();
24022397
cleanupTooltipEvents = null;
@@ -2463,14 +2458,12 @@ export function createChart(
24632458
// Initial render
24642459
render();
24652460

2466-
// Set up responsive resize with debounce to avoid full SVG rebuild on every frame
2461+
// Set up responsive resize. The observeResize helper already debounces at
2462+
// ~60fps (16ms) internally, which is sufficient to coalesce a drag-burst
2463+
// into a single render without additive delay.
24672464
if (options?.responsive !== false) {
24682465
disconnectResize = observeResize(container, () => {
2469-
if (resizeTimer !== null) clearTimeout(resizeTimer);
2470-
resizeTimer = setTimeout(() => {
2471-
resizeTimer = null;
2472-
resize();
2473-
}, 100);
2466+
resize();
24742467
});
24752468
}
24762469

0 commit comments

Comments
 (0)