Skip to content

Commit 411e1e9

Browse files
authored
Merge pull request #12 from ChartGPU/bug/fix-chart-rendering
Optimize React performance across all wrapper components
2 parents dd1a1ec + 87fa7a8 commit 411e1e9

File tree

6 files changed

+142
-143
lines changed

6 files changed

+142
-143
lines changed

src/ChartGPU.tsx

Lines changed: 52 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import {
44
useState,
55
useImperativeHandle,
66
forwardRef,
7-
useCallback,
87
} from 'react';
98
import { ChartGPU as ChartGPULib } from '@chartgpu/chartgpu';
109
import type {
@@ -15,24 +14,7 @@ import type {
1514
MouseOverParams,
1615
} from './types';
1716
import type { ChartGPUOptions, ChartGPUZoomRangeChangePayload } from '@chartgpu/chartgpu';
18-
19-
/**
20-
* Debounce utility for throttling frequent calls.
21-
*/
22-
function debounce<T extends (...args: any[]) => void>(
23-
fn: T,
24-
delayMs: number
25-
): (...args: Parameters<T>) => void {
26-
let timeoutId: ReturnType<typeof setTimeout> | null = null;
27-
return (...args: Parameters<T>) => {
28-
if (timeoutId !== null) {
29-
clearTimeout(timeoutId);
30-
}
31-
timeoutId = setTimeout(() => {
32-
fn(...args);
33-
}, delayMs);
34-
};
35-
}
17+
import { debounce } from './utils';
3618

3719
/**
3820
* ChartGPU React component.
@@ -84,9 +66,33 @@ export const ChartGPU = forwardRef<ChartGPUHandle, ChartGPUProps>(
8466
const instanceRef = useRef<ChartInstance | null>(null);
8567
const [chart, setChart] = useState<ChartInstance | null>(null);
8668
const mountedRef = useRef<boolean>(false);
87-
const resizeObserverRef = useRef<ResizeObserver | null>(null);
8869
const gpuContextRef = useRef(gpuContext);
8970

71+
// --- Callback refs: keep handlers in refs so effects never re-subscribe ---
72+
const onReadyRef = useRef(onReady);
73+
onReadyRef.current = onReady;
74+
75+
const onClickRef = useRef(onClick);
76+
onClickRef.current = onClick;
77+
78+
const onMouseOverRef = useRef(onMouseOver);
79+
onMouseOverRef.current = onMouseOver;
80+
81+
const onMouseOutRef = useRef(onMouseOut);
82+
onMouseOutRef.current = onMouseOut;
83+
84+
const onCrosshairMoveRef = useRef(onCrosshairMove);
85+
onCrosshairMoveRef.current = onCrosshairMove;
86+
87+
const onDataAppendRef = useRef(onDataAppend);
88+
onDataAppendRef.current = onDataAppend;
89+
90+
const onDeviceLostRef = useRef(onDeviceLost);
91+
onDeviceLostRef.current = onDeviceLost;
92+
93+
const onZoomChangeRef = useRef(onZoomChange);
94+
onZoomChangeRef.current = onZoomChange;
95+
9096
// Expose imperative handle
9197
useImperativeHandle(
9298
ref,
@@ -169,17 +175,6 @@ export const ChartGPU = forwardRef<ChartGPUHandle, ChartGPUProps>(
169175
[]
170176
);
171177

172-
// Build effective options with theme override if provided
173-
const getEffectiveOptions = useCallback((): ChartGPUOptions => {
174-
if (theme !== undefined) {
175-
return {
176-
...options,
177-
theme,
178-
};
179-
}
180-
return options;
181-
}, [options, theme]);
182-
183178
// Initialize chart on mount
184179
useEffect(() => {
185180
if (!containerRef.current) return;
@@ -191,7 +186,7 @@ export const ChartGPU = forwardRef<ChartGPUHandle, ChartGPUProps>(
191186
try {
192187
if (!containerRef.current) return;
193188

194-
const effectiveOptions = getEffectiveOptions();
189+
const effectiveOptions = theme !== undefined ? { ...options, theme } : options;
195190
const ctx = gpuContextRef.current;
196191
chartInstance = ctx
197192
? await ChartGPULib.create(containerRef.current, effectiveOptions, ctx)
@@ -201,7 +196,7 @@ export const ChartGPU = forwardRef<ChartGPUHandle, ChartGPUProps>(
201196
if (mountedRef.current) {
202197
instanceRef.current = chartInstance;
203198
setChart(chartInstance);
204-
onReady?.(chartInstance);
199+
onReadyRef.current?.(chartInstance);
205200
} else {
206201
// Component unmounted during async create - dispose immediately
207202
chartInstance.dispose();
@@ -232,17 +227,17 @@ export const ChartGPU = forwardRef<ChartGPUHandle, ChartGPUProps>(
232227
const instance = chart;
233228
if (!instance || instance.disposed) return;
234229

235-
const effectiveOptions = getEffectiveOptions();
230+
const effectiveOptions = theme !== undefined ? { ...options, theme } : options;
236231
instance.setOption(effectiveOptions);
237-
}, [chart, options, theme, getEffectiveOptions]);
232+
}, [chart, options, theme]);
238233

239234
// Register/unregister click event handler
240235
useEffect(() => {
241236
const instance = chart;
242-
if (!instance || instance.disposed || !onClick) return;
237+
if (!instance || instance.disposed) return;
243238

244239
const handler = (payload: ClickParams) => {
245-
onClick(payload);
240+
onClickRef.current?.(payload);
246241
};
247242

248243
instance.on('click', handler);
@@ -254,15 +249,15 @@ export const ChartGPU = forwardRef<ChartGPUHandle, ChartGPUProps>(
254249
// instance may already be disposed; swallow
255250
}
256251
};
257-
}, [chart, onClick]);
252+
}, [chart]);
258253

259254
// Register/unregister mouseover event handler
260255
useEffect(() => {
261256
const instance = chart;
262-
if (!instance || instance.disposed || !onMouseOver) return;
257+
if (!instance || instance.disposed) return;
263258

264259
const handler = (payload: MouseOverParams) => {
265-
onMouseOver(payload);
260+
onMouseOverRef.current?.(payload);
266261
};
267262

268263
instance.on('mouseover', handler);
@@ -274,15 +269,15 @@ export const ChartGPU = forwardRef<ChartGPUHandle, ChartGPUProps>(
274269
// instance may already be disposed; swallow
275270
}
276271
};
277-
}, [chart, onMouseOver]);
272+
}, [chart]);
278273

279274
// Register/unregister mouseout event handler
280275
useEffect(() => {
281276
const instance = chart;
282-
if (!instance || instance.disposed || !onMouseOut) return;
277+
if (!instance || instance.disposed) return;
283278

284279
const handler = (payload: ClickParams) => {
285-
onMouseOut(payload);
280+
onMouseOutRef.current?.(payload);
286281
};
287282

288283
instance.on('mouseout', handler);
@@ -294,15 +289,15 @@ export const ChartGPU = forwardRef<ChartGPUHandle, ChartGPUProps>(
294289
// instance may already be disposed; swallow
295290
}
296291
};
297-
}, [chart, onMouseOut]);
292+
}, [chart]);
298293

299294
// Register/unregister crosshairMove event handler
300295
useEffect(() => {
301296
const instance = chart;
302-
if (!instance || instance.disposed || !onCrosshairMove) return;
297+
if (!instance || instance.disposed) return;
303298

304299
const handler = (payload: Parameters<NonNullable<ChartGPUProps['onCrosshairMove']>>[0]) => {
305-
onCrosshairMove(payload);
300+
onCrosshairMoveRef.current?.(payload);
306301
};
307302

308303
instance.on('crosshairMove', handler);
@@ -314,15 +309,15 @@ export const ChartGPU = forwardRef<ChartGPUHandle, ChartGPUProps>(
314309
// instance may already be disposed; swallow
315310
}
316311
};
317-
}, [chart, onCrosshairMove]);
312+
}, [chart]);
318313

319314
// Register/unregister dataAppend event handler
320315
useEffect(() => {
321316
const instance = chart;
322-
if (!instance || instance.disposed || !onDataAppend) return;
317+
if (!instance || instance.disposed) return;
323318

324319
const handler = (payload: Parameters<NonNullable<ChartGPUProps['onDataAppend']>>[0]) => {
325-
onDataAppend(payload);
320+
onDataAppendRef.current?.(payload);
326321
};
327322

328323
instance.on('dataAppend', handler);
@@ -334,15 +329,15 @@ export const ChartGPU = forwardRef<ChartGPUHandle, ChartGPUProps>(
334329
// instance may already be disposed; swallow
335330
}
336331
};
337-
}, [chart, onDataAppend]);
332+
}, [chart]);
338333

339334
// Register/unregister deviceLost event handler
340335
useEffect(() => {
341336
const instance = chart;
342-
if (!instance || instance.disposed || !onDeviceLost) return;
337+
if (!instance || instance.disposed) return;
343338

344339
const handler = (payload: Parameters<NonNullable<ChartGPUProps['onDeviceLost']>>[0]) => {
345-
onDeviceLost(payload);
340+
onDeviceLostRef.current?.(payload);
346341
};
347342

348343
instance.on('deviceLost', handler);
@@ -354,7 +349,7 @@ export const ChartGPU = forwardRef<ChartGPUHandle, ChartGPUProps>(
354349
// instance may already be disposed; swallow
355350
}
356351
};
357-
}, [chart, onDeviceLost]);
352+
}, [chart]);
358353

359354
// Set up ResizeObserver for responsive sizing (debounced 100ms)
360355
useEffect(() => {
@@ -373,11 +368,9 @@ export const ChartGPU = forwardRef<ChartGPUHandle, ChartGPUProps>(
373368
});
374369

375370
observer.observe(container);
376-
resizeObserverRef.current = observer;
377371

378372
return () => {
379373
observer.disconnect();
380-
resizeObserverRef.current = null;
381374
};
382375
// Intentionally omitting containerRef.current from dependencies
383376
// eslint-disable-next-line react-hooks/exhaustive-deps
@@ -388,19 +381,19 @@ export const ChartGPU = forwardRef<ChartGPUHandle, ChartGPUProps>(
388381
// so consumers don't need to wait for user interaction to receive the first value.
389382
useEffect(() => {
390383
const instance = chart;
391-
if (!instance || instance.disposed || !onZoomChange) return;
384+
if (!instance || instance.disposed) return;
392385

393386
const handler = (payload: ChartGPUZoomRangeChangePayload) => {
394387
// Map upstream payload to ZoomRange (strip `source`)
395-
onZoomChange({ start: payload.start, end: payload.end });
388+
onZoomChangeRef.current?.({ start: payload.start, end: payload.end });
396389
};
397390

398391
instance.on('zoomRangeChange', handler);
399392

400393
// Hydrate: fire once with the current zoom range (if non-null)
401394
const current = instance.getZoomRange();
402395
if (current) {
403-
onZoomChange({ start: current.start, end: current.end });
396+
onZoomChangeRef.current?.({ start: current.start, end: current.end });
404397
}
405398

406399
return () => {
@@ -410,7 +403,7 @@ export const ChartGPU = forwardRef<ChartGPUHandle, ChartGPUProps>(
410403
// instance may already be disposed; swallow
411404
}
412405
};
413-
}, [chart, onZoomChange]);
406+
}, [chart]);
414407

415408
return (
416409
<div

src/ChartGPUChart.tsx

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
1-
import { useEffect, useRef } from 'react';
1+
import { useCallback, useEffect, useMemo, useRef } from 'react';
22
import type { CSSProperties } from 'react';
33
import type { ChartGPUOptions, ChartGPUInstance } from '@chartgpu/chartgpu';
44
import { ChartGPU } from './ChartGPU';
55

6+
const DEFAULT_STYLE: CSSProperties = {
7+
position: 'relative',
8+
width: '100%',
9+
height: '400px',
10+
};
11+
612
export interface ChartGPUChartProps {
713
/**
814
* Chart configuration options
@@ -71,6 +77,19 @@ export function ChartGPUChart({
7177
}: ChartGPUChartProps): JSX.Element {
7278
const didInitRef = useRef(false);
7379

80+
const onInitRef = useRef(onInit);
81+
onInitRef.current = onInit;
82+
83+
const handleReady = useCallback((instance: ChartGPUInstance) => {
84+
didInitRef.current = true;
85+
onInitRef.current?.(instance);
86+
}, []);
87+
88+
const mergedStyle = useMemo(
89+
() => (style ? { ...DEFAULT_STYLE, ...style } : DEFAULT_STYLE),
90+
[style]
91+
);
92+
7493
useEffect(() => {
7594
return () => {
7695
if (didInitRef.current) {
@@ -82,17 +101,9 @@ export function ChartGPUChart({
82101
return (
83102
<ChartGPU
84103
className={className}
85-
style={{
86-
position: 'relative',
87-
width: '100%',
88-
height: '400px',
89-
...style,
90-
}}
104+
style={mergedStyle}
91105
options={options}
92-
onReady={(instance: ChartGPUInstance) => {
93-
didInitRef.current = true;
94-
onInit?.(instance);
95-
}}
106+
onReady={handleReady}
96107
/>
97108
);
98109
}

0 commit comments

Comments
 (0)