Skip to content

[LG-5137]: refactor option updates and improve memory usage #2861

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/charts-oom-fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@lg-charts/core': minor
---

[LG-5137](https://jira.mongodb.org/browse/LG-5137): refactor option updates and improve memory usage
5 changes: 2 additions & 3 deletions charts/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,14 @@
"@lg-charts/colors": "workspace:^",
"@lg-charts/series-provider": "workspace:^",
"echarts": "^5.5.1",
"lodash.debounce": "^4.0.8"
"lodash": "^4.17.21"
},
"peerDependencies": {
"@leafygreen-ui/leafygreen-provider": "workspace:^"
},
"devDependencies": {
"@faker-js/faker": "8.0.2",
"@leafygreen-ui/icon": "workspace:^",
"@types/lodash.debounce": "^4.0.9"
"@leafygreen-ui/icon": "workspace:^"
},
"repository": {
"type": "git",
Expand Down
4 changes: 0 additions & 4 deletions charts/core/src/Chart/config/getDefaultChartOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
} from '@leafygreen-ui/tokens';

import { ChartOptions } from '../Chart.types';
import { TOOLBOX_ID, X_AXIS_ID, Y_AXIS_ID } from '../constants';

const commonAxisOptions = {
/**
Expand Down Expand Up @@ -67,20 +66,17 @@ export const getDefaultChartOptions = (
},

xAxis: {
id: X_AXIS_ID,
type: 'time',
...commonAxisOptions,
},

yAxis: {
id: Y_AXIS_ID,
type: 'value',
...commonAxisOptions,
},

// Sets up zooming
toolbox: {
id: TOOLBOX_ID,
feature: {
dataZoom: {
show: true,
Expand Down
3 changes: 0 additions & 3 deletions charts/core/src/Chart/constants.ts

This file was deleted.

50 changes: 0 additions & 50 deletions charts/core/src/Chart/hooks/updateUtils.spec.ts

This file was deleted.

49 changes: 0 additions & 49 deletions charts/core/src/Chart/hooks/updateUtils.ts

This file was deleted.

173 changes: 92 additions & 81 deletions charts/core/src/Chart/hooks/useChart.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useCallback, useEffect, useRef, useState } from 'react';
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';

import { useEchart } from '../../Echart';
import { EChartEvents } from '../../Echart';
Expand All @@ -14,7 +14,7 @@ export function useChart({
theme,
state,
}: ChartHookProps): ChartInstance {
const initialOptions = getDefaultChartOptions(theme);
const initialOptions = useMemo(() => getDefaultChartOptions(theme), [theme]);

/**
* It is necessary for `useEchart` to know when the container exists
Expand All @@ -23,6 +23,7 @@ export function useChart({
* element only gets populated after render.
*/
const [container, setContainer] = useState<HTMLDivElement | null>(null);

const echart = useEchart({
container,
initialOptions,
Expand All @@ -42,120 +43,130 @@ export function useChart({
} = echart;

useEffect(() => {
if (ready) {
onChartReady();
if (!ready) {
return;
}
onChartReady();
}, [ready, onChartReady]);

useEffect(() => {
if (ready) {
if (groupId) {
addToGroup(groupId);
}

return () => {
removeFromGroup();
};
if (!ready || !groupId) {
return;
}

addToGroup(groupId);

return () => {
removeFromGroup();
};
}, [ready, groupId, addToGroup, removeFromGroup]);

// SETUP AND ENABLE ZOOM
useEffect(() => {
if (ready) {
setupZoomSelect({
xAxis: zoomSelect?.xAxis,
yAxis: zoomSelect?.yAxis,
});

if (zoomSelect?.xAxis || zoomSelect?.yAxis) {
function enableZoomOnRender() {
enableZoom();
/**
* Enabling zoom triggers a render, so once we enable it, we want to
* remove the handler or else there will be an infinite loop of
* render -> enable -> render -> etc.
*/
off('rendered', enableZoomOnRender);
}
if (!ready) {
return;
}

on('rendered', enableZoomOnRender);
setupZoomSelect({
xAxis: zoomSelect?.xAxis,
yAxis: zoomSelect?.yAxis,
});

if (zoomSelect?.xAxis || zoomSelect?.yAxis) {
function enableZoomOnRender() {
enableZoom();
/**
* Enabling zoom triggers a render, so once we enable it, we want to
* remove the handler or else there will be an infinite loop of
* render -> enable -> render -> etc.
*/
off('rendered', enableZoomOnRender);
}

on('rendered', enableZoomOnRender);
}
}, [enableZoom, off, on, ready, setupZoomSelect, zoomSelect]);

useEffect(() => {
if (ready && onZoomSelect) {
on(EChartEvents.ZoomSelect, zoomEventResponse => {
onZoomSelect(zoomEventResponse);
});
if (!ready || !onZoomSelect) {
return;
}
on(EChartEvents.ZoomSelect, zoomEventResponse => {
onZoomSelect(zoomEventResponse);
});
}, [ready, onZoomSelect, on]);

// We want to hide the tooltip when it's hovered over any `EventMarkerPoint`
useEffect(() => {
if (ready) {
on('mouseover', e => {
if (e.componentType === 'markPoint') {
hideTooltip();
on('mousemove', hideTooltip);
}
});

// Stop hiding once the mouse leaves the `EventMarkerPoint`
on('mouseout', e => {
if (e.componentType === 'markPoint') {
off('mousemove', hideTooltip);
}
});
if (!ready) {
return;
}

on('mouseover', e => {
if (e.componentType === 'markPoint') {
hideTooltip();
on('mousemove', hideTooltip);
}
});

// Stop hiding once the mouse leaves the `EventMarkerPoint`
on('mouseout', e => {
if (e.componentType === 'markPoint') {
off('mousemove', hideTooltip);
}
});
}, [echart, hideTooltip, off, on, ready]);

const initialRenderRef = useRef(true);

const handleResize = useCallback(() => {
if (ready) {
// Skip the first resize event, as it's triggered by the initial render
if (initialRenderRef.current) {
initialRenderRef.current = false;
return;
}
if (!ready) {
return;
}

if (zoomSelect && (zoomSelect.xAxis || zoomSelect.yAxis)) {
/**
* If the chart has been resized, the chart appears to reset zoom, which
* disables it. We need to re-enable it after the resize however, doing so
* immediately doesn't work. To work around this, we listen for the `finished`
* event, which is triggered after the chart has been rendered, and then
* execute the re-enable zoom logic after all tasks on the queue have been
* processed.
*
* TODO(LG-4818): Investigate why this is necessary
*/
function reEnableZoom() {
function reEnableZoomCallback() {
enableZoom();
off('finished', reEnableZoom);
}
setTimeout(reEnableZoomCallback, 0);
}
// Skip the first resize event, as it's triggered by the initial render
if (initialRenderRef.current) {
initialRenderRef.current = false;
return;
}

on('finished', reEnableZoom);
if (zoomSelect && (zoomSelect.xAxis || zoomSelect.yAxis)) {
/**
* If the chart has been resized, the chart appears to reset zoom, which
* disables it. We need to re-enable it after the resize however, doing so
* immediately doesn't work. To work around this, we listen for the `finished`
* event, which is triggered after the chart has been rendered, and then
* execute the re-enable zoom logic after all tasks on the queue have been
* processed.
*
* TODO(LG-4818): Investigate why this is necessary
*/
function reEnableZoom() {
function reEnableZoomCallback() {
enableZoom();
off('finished', reEnableZoom);
}
setTimeout(reEnableZoomCallback, 0);
}

resize();
on('finished', reEnableZoom);
}

resize();
}, [enableZoom, off, on, ready, resize, zoomSelect]);

useEffect(() => {
if (ready && container) {
const resizeObserver = new ResizeObserver(handleResize);
resizeObserver.observe(container);

return () => {
resizeObserver.disconnect();
};
if (!ready || !container) {
return;
}
}, [ready, container, handleResize]);

const resizeObserver = new ResizeObserver(handleResize);
resizeObserver.observe(container);

return () => {
resizeObserver.disconnect();
};
}, [container, ready, handleResize]);

return {
...echart,
Expand Down
1 change: 0 additions & 1 deletion charts/core/src/Chart/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,3 @@ export {
type SeriesOption,
type ZoomSelectionEvent,
} from './Chart.types';
export { TOOLBOX_ID, X_AXIS_ID, Y_AXIS_ID } from './constants';
Loading