From a8b020deeb58bda44976d23de23211ea0f3aa639 Mon Sep 17 00:00:00 2001 From: Terrence Keane Date: Wed, 11 Dec 2024 14:01:01 -0500 Subject: [PATCH 01/21] Revert "LG-4662: Fix CommonJS imports (#2587)" This reverts commit 6e37cfa6e92c444a6e99c9d227a106b7662164d9. --- charts/core/src/Chart/hooks/useChart.spec.ts | 18 +++++++++++- charts/core/src/Chart/hooks/useChart.ts | 30 +++++++++++++++++++- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/charts/core/src/Chart/hooks/useChart.spec.ts b/charts/core/src/Chart/hooks/useChart.spec.ts index 2a211bc9d5..2ede70a6c9 100644 --- a/charts/core/src/Chart/hooks/useChart.spec.ts +++ b/charts/core/src/Chart/hooks/useChart.spec.ts @@ -6,7 +6,7 @@ import { SeriesOption } from '../Chart.types'; import { useChart } from './useChart'; -jest.mock('echarts', () => ({ +jest.mock('echarts/core', () => ({ init: jest.fn(() => ({ setOption: jest.fn(), dispose: jest.fn(), @@ -16,6 +16,22 @@ jest.mock('echarts', () => ({ use: jest.fn(), })); +jest.mock('echarts/charts', () => ({ + LineChart: jest.fn(), +})); + +jest.mock('echarts/components', () => ({ + TitleComponent: jest.fn(), + TooltipComponent: jest.fn(), + GridComponent: jest.fn(), + LegendComponent: jest.fn(), + ToolboxComponent: jest.fn(), +})); + +jest.mock('echarts/renderers', () => ({ + CanvasRenderer: jest.fn(), +})); + describe('@lg-echarts/core/hooks/useChart', () => { test('should return an object with the correct properties', () => { const { result } = renderHook(() => useChart({ theme: 'dark' })); diff --git a/charts/core/src/Chart/hooks/useChart.ts b/charts/core/src/Chart/hooks/useChart.ts index ef9b7df5f3..09ef5b8c89 100644 --- a/charts/core/src/Chart/hooks/useChart.ts +++ b/charts/core/src/Chart/hooks/useChart.ts @@ -1,5 +1,16 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; -import * as echarts from 'echarts'; +import { LineChart as EchartsLineChart } from 'echarts/charts'; +import { + DataZoomComponent, + DataZoomInsideComponent, + GridComponent, + LegendComponent, + TitleComponent, + ToolboxComponent, + TooltipComponent, +} from 'echarts/components'; +import * as echarts from 'echarts/core'; +import { CanvasRenderer } from 'echarts/renderers'; import debounce from 'lodash.debounce'; import { ChartOptions, SeriesOption } from '../../Chart/Chart.types'; @@ -9,6 +20,23 @@ import { getDefaultChartOptions } from '../config'; import { addSeries, removeSeries, updateOptions } from './updateUtils'; import { ChartHookProps, ZoomSelectionEvent } from './useChart.types'; +/** + * Register the required components. By using separate imports, we can avoid + * importing the entire echarts library which will reduce the bundle size. + * Must be added to if additional functionality is supported. + */ +echarts.use([ + TitleComponent, + TooltipComponent, + GridComponent, + LegendComponent, + EchartsLineChart, + CanvasRenderer, + ToolboxComponent, + DataZoomComponent, + DataZoomInsideComponent, +]); + /** * Creates a generic Apache ECharts options object with default values for those not set * that are in line with the designs and needs of the design system. From 30ea8ac4f44d90403b0a4ebf46ce09601951d5d2 Mon Sep 17 00:00:00 2001 From: Terrence Keane Date: Thu, 12 Dec 2024 14:53:22 -0500 Subject: [PATCH 02/21] Working state --- charts/core/src/Chart/hooks/useChart.ts | 114 ++++------------- charts/core/src/Echarts/getEchartsInstance.ts | 57 +++++++++ charts/core/src/Echarts/useEchartsInstance.ts | 115 ++++++++++++++++++ 3 files changed, 199 insertions(+), 87 deletions(-) create mode 100644 charts/core/src/Echarts/getEchartsInstance.ts create mode 100644 charts/core/src/Echarts/useEchartsInstance.ts diff --git a/charts/core/src/Chart/hooks/useChart.ts b/charts/core/src/Chart/hooks/useChart.ts index 09ef5b8c89..6507988d0c 100644 --- a/charts/core/src/Chart/hooks/useChart.ts +++ b/charts/core/src/Chart/hooks/useChart.ts @@ -1,16 +1,4 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; -import { LineChart as EchartsLineChart } from 'echarts/charts'; -import { - DataZoomComponent, - DataZoomInsideComponent, - GridComponent, - LegendComponent, - TitleComponent, - ToolboxComponent, - TooltipComponent, -} from 'echarts/components'; -import * as echarts from 'echarts/core'; -import { CanvasRenderer } from 'echarts/renderers'; import debounce from 'lodash.debounce'; import { ChartOptions, SeriesOption } from '../../Chart/Chart.types'; @@ -19,23 +7,7 @@ import { getDefaultChartOptions } from '../config'; import { addSeries, removeSeries, updateOptions } from './updateUtils'; import { ChartHookProps, ZoomSelectionEvent } from './useChart.types'; - -/** - * Register the required components. By using separate imports, we can avoid - * importing the entire echarts library which will reduce the bundle size. - * Must be added to if additional functionality is supported. - */ -echarts.use([ - TitleComponent, - TooltipComponent, - GridComponent, - LegendComponent, - EchartsLineChart, - CanvasRenderer, - ToolboxComponent, - DataZoomComponent, - DataZoomInsideComponent, -]); +import { useEchartsInstance } from '../../Echarts/useEchartsInstance'; /** * Creates a generic Apache ECharts options object with default values for those not set @@ -43,13 +15,13 @@ echarts.use([ */ export function useChart({ theme, - onChartReady, + onChartReady = () => {}, zoomSelect, onZoomSelect, groupId, }: ChartHookProps) { const chartRef = useRef(null); - const chartInstanceRef = useRef(); + const { chart, echarts } = useEchartsInstance(chartRef.current); const [chartOptions, setChartOptions] = useState( getDefaultChartOptions(theme), ); @@ -91,15 +63,13 @@ export function useChart({ } /** - * If start is not 0% or end is not 100%, that means that the 'dataZoom' - * event was triggered by an actual zoom. Since we don't want to actually - * zoom on the current data, but rather provide the new values to the passed - * in handler, we dispatch an action to essentially override the zoom. + * If start is not 0% or end is not 100%, the 'dataZoom' event was triggered by a zoom. + * We override the zoom to provide new values to the handler. */ const isZoomed = params?.start !== 0 || params?.end !== 100; - if (chartInstanceRef.current && isZoomed) { - chartInstanceRef.current.dispatchAction({ + if (chart && isZoomed) { + chart.dispatchAction({ type: 'dataZoom', start: 0, // percentage of starting position end: 100, // percentage of ending position @@ -107,83 +77,54 @@ export function useChart({ } } - /** - * Meant to be called once on initial render - */ - function onInitialRender() { - if (!chartInstanceRef.current) { - return; - } - - const chartInstance = chartInstanceRef.current; - - if (zoomSelect?.xAxis || zoomSelect?.yAxis) { - /** - * Zooming is built into echart via the toolbar. By default, a user - * has to click the "dataZoom" button to enable zooming. We however hide - * this button and want it turned on by default. This is done by dispatching - * an action to enable the "dataZoomSelect" feature, as if it were clicked. - */ - chartInstance.dispatchAction({ - type: 'takeGlobalCursor', - key: 'dataZoomSelect', - dataZoomSelectActive: true, - }); - - chartInstance.on('dataZoom', handleDataZoom); - } - - if (onChartReady) { - onChartReady(); - } - - /** - * This is only done on initial render because what's inside itself - * triggers a render which creates an infinite loop if not removed. - */ - chartInstance.off('rendered', onInitialRender); - } - // Initialize the chart useEffect(() => { - const chartInstance = echarts.init(chartRef.current); - chartInstanceRef.current = chartInstance; - chartInstance.setOption(chartOptions); + if (!chart || !echarts) return; + + chart.setOption(chartOptions); // Connects a chart to a group which allows for synchronized tooltips if (groupId) { - chartInstance.group = groupId; + chart.group = groupId; echarts.connect(groupId); } // ECharts does not automatically resize when the window resizes. const resizeHandler = () => { - chartInstance.resize(); + chart.resize(); }; window.addEventListener('resize', resizeHandler); - chartInstance.on('rendered', onInitialRender); + chart.on('rendered', function onInitialRender() { + // Enable zooming by default by dispatching the "dataZoomSelect" action. + chart.dispatchAction({ + type: 'takeGlobalCursor', + key: 'dataZoomSelect', + dataZoomSelectActive: zoomSelect?.xAxis || zoomSelect?.yAxis, + }); + + chart.on('dataZoom', handleDataZoom); + onChartReady(); + chart.off('rendered', onInitialRender); + }); return () => { window.removeEventListener('resize', resizeHandler); - chartInstance.dispose(); + chart.dispose(); }; - }, [chartOptions, onChartReady, zoomSelect]); + }, [chart, echarts]); // Set which axis zoom is enabled on useEffect(() => { // `0` index enables zoom on that index, `'none'` disables zoom on that index let xAxisIndex: number | string = 0; let yAxisIndex: number | string = 0; - if (!zoomSelect?.xAxis) { xAxisIndex = 'none'; } - if (!zoomSelect?.yAxis) { yAxisIndex = 'none'; } - updateChartOptions({ toolbox: { feature: { @@ -204,7 +145,7 @@ export function useChart({ * This is needed to ensure that series get removed properly. * See issue: https://github.com/apache/echarts/issues/6202 * */ - chartInstanceRef.current?.setOption(chartOptions, true); + chart?.setOption(chartOptions, true); }, 50), [], ); @@ -259,6 +200,5 @@ export function useChart({ addChartSeries, removeChartSeries, chartRef, - chartInstanceRef, }; } diff --git a/charts/core/src/Echarts/getEchartsInstance.ts b/charts/core/src/Echarts/getEchartsInstance.ts new file mode 100644 index 0000000000..b5d4627799 --- /dev/null +++ b/charts/core/src/Echarts/getEchartsInstance.ts @@ -0,0 +1,57 @@ +// Create a singleton promise to handle the initialization +let initPromise: Promise | null = null; +let echartsCore: any; +let echartsCharts: any; +let echartsRenders: any; +let echartsComponents: any; + +async function initializeEcharts() { + // If already initialized, return immediately + if (echartsCore) { + return; + } + + // If initialization is in progress, wait for it + if (initPromise) { + return initPromise; + } + + // Start initialization and store the promise + initPromise = (async () => { + const [ + echartsCoreResolved, + echartsChartsResolved, + echartsRendersResolved, + echartsComponentsResolved, + ] = await Promise.all([ + import('echarts/core'), + import('echarts/charts'), + import('echarts/renderers'), + import('echarts/components'), + ]); + + echartsCore = echartsCoreResolved; + echartsCharts = echartsChartsResolved; + echartsRenders = echartsRendersResolved; + echartsComponents = echartsComponentsResolved; + + echartsCore.use([ + echartsCharts.LineChart, + echartsRenders.CanvasRenderer, + echartsComponents.TitleComponent, + echartsComponents.TooltipComponent, + echartsComponents.GridComponent, + echartsComponents.LegendComponent, + echartsComponents.ToolboxComponent, + echartsComponents.DataZoomComponent, + echartsComponents.DataZoomInsideComponent, + ]); + })(); + + return initPromise; +} + +export async function getEchartsInstance(container: HTMLElement | null) { + await initializeEcharts(); + return { instance: echartsCore.init(container), core: echartsCore }; +} diff --git a/charts/core/src/Echarts/useEchartsInstance.ts b/charts/core/src/Echarts/useEchartsInstance.ts new file mode 100644 index 0000000000..c3a13b81f2 --- /dev/null +++ b/charts/core/src/Echarts/useEchartsInstance.ts @@ -0,0 +1,115 @@ +import { useEffect, useRef, useState } from 'react'; + +// Type for the ECharts instance +type EchartsType = any; + +interface UseEchartsResult { + chart: EchartsType | null; + isLoading: boolean; + error: Error | null; + echarts: any; +} + +// Singleton promise for initialization +let initPromise: Promise | null = null; +let echartsCore: any; +let echartsCharts: any; +let echartsRenders: any; +let echartsComponents: any; + +async function initializeEcharts() { + // If already initialized, return immediately + if (echartsCore) { + return; + } + + // If initialization is in progress, wait for it + if (initPromise) { + return initPromise; + } + + // Start initialization and store the promise + initPromise = (async () => { + try { + const [ + echartsCoreResolved, + echartsChartsResolved, + echartsRendersResolved, + echartsComponentsResolved, + ] = await Promise.all([ + import('echarts/core'), + import('echarts/charts'), + import('echarts/renderers'), + import('echarts/components'), + ]); + + echartsCore = echartsCoreResolved; + echartsCharts = echartsChartsResolved; + echartsRenders = echartsRendersResolved; + echartsComponents = echartsComponentsResolved; + + echartsCore.use([ + echartsCharts.LineChart, + echartsRenders.CanvasRenderer, + echartsComponents.TitleComponent, + echartsComponents.TooltipComponent, + echartsComponents.GridComponent, + echartsComponents.LegendComponent, + echartsComponents.ToolboxComponent, + echartsComponents.DataZoomComponent, + echartsComponents.DataZoomInsideComponent, + ]); + } catch (error) { + // Ensure we clear the promise if initialization fails + initPromise = null; + throw error; + } + })(); + + return initPromise; +} + +export function useEchartsInstance( + container: HTMLDivElement | null, +): UseEchartsResult { + const [chart, setChart] = useState(null); + const [isLoading, setIsLoading] = useState(true); + const [error, setError] = useState(null); + + useEffect(() => { + async function setupChart() { + try { + setIsLoading(true); + setError(null); + + await initializeEcharts(); + + if (container) { + const newChart = echartsCore.init(container); + console.log('post init'); + setChart(newChart); + } + } catch (err) { + console.error(err); + setError( + err instanceof Error + ? err + : new Error('Failed to initialize ECharts'), + ); + } finally { + setIsLoading(false); + } + } + + setupChart(); + + // Cleanup function + return () => { + if (chart) { + chart.dispose(); + } + }; + }, [container]); + + return { chart, echarts: echartsCore, isLoading, error }; +} From 8a2f3eaeecad7274ece7c0f332f302281e906fb4 Mon Sep 17 00:00:00 2001 From: Terrence Keane Date: Fri, 13 Dec 2024 09:05:43 -0500 Subject: [PATCH 03/21] Move adding group logic to echart --- charts/core/src/Chart.stories.tsx | 20 +++++++++++-- charts/core/src/Chart/hooks/useChart.ts | 14 ++++----- .../{useEchartsInstance.ts => useEchart.ts} | 30 +++++++------------ 3 files changed, 36 insertions(+), 28 deletions(-) rename charts/core/src/Echarts/{useEchartsInstance.ts => useEchart.ts} (83%) diff --git a/charts/core/src/Chart.stories.tsx b/charts/core/src/Chart.stories.tsx index 5c1f3b09a4..a23849f7e9 100644 --- a/charts/core/src/Chart.stories.tsx +++ b/charts/core/src/Chart.stories.tsx @@ -442,12 +442,22 @@ export const WithSameGroupIds: StoryObj = { renderHeader, headerTitle, headerShowDivider, + zoomSelectXAxis, + zoomSelectYAxis, + zoomSelectCallback, }) => { return (
- + {renderHeader && (
)} @@ -482,7 +492,13 @@ export const WithSameGroupIds: StoryObj = { ))} - + {renderHeader && (
)} diff --git a/charts/core/src/Chart/hooks/useChart.ts b/charts/core/src/Chart/hooks/useChart.ts index 6507988d0c..e03b31807b 100644 --- a/charts/core/src/Chart/hooks/useChart.ts +++ b/charts/core/src/Chart/hooks/useChart.ts @@ -7,7 +7,7 @@ import { getDefaultChartOptions } from '../config'; import { addSeries, removeSeries, updateOptions } from './updateUtils'; import { ChartHookProps, ZoomSelectionEvent } from './useChart.types'; -import { useEchartsInstance } from '../../Echarts/useEchartsInstance'; +import { useEchart } from '../../Echarts/useEchart'; /** * Creates a generic Apache ECharts options object with default values for those not set @@ -21,7 +21,7 @@ export function useChart({ groupId, }: ChartHookProps) { const chartRef = useRef(null); - const { chart, echarts } = useEchartsInstance(chartRef.current); + const { chart, core, addToGroup } = useEchart(chartRef.current); const [chartOptions, setChartOptions] = useState( getDefaultChartOptions(theme), ); @@ -79,14 +79,14 @@ export function useChart({ // Initialize the chart useEffect(() => { - if (!chart || !echarts) return; + if (!chart || !core) { + return; + } chart.setOption(chartOptions); - // Connects a chart to a group which allows for synchronized tooltips if (groupId) { - chart.group = groupId; - echarts.connect(groupId); + addToGroup(groupId); } // ECharts does not automatically resize when the window resizes. @@ -112,7 +112,7 @@ export function useChart({ window.removeEventListener('resize', resizeHandler); chart.dispose(); }; - }, [chart, echarts]); + }, [chart, core]); // Set which axis zoom is enabled on useEffect(() => { diff --git a/charts/core/src/Echarts/useEchartsInstance.ts b/charts/core/src/Echarts/useEchart.ts similarity index 83% rename from charts/core/src/Echarts/useEchartsInstance.ts rename to charts/core/src/Echarts/useEchart.ts index c3a13b81f2..62f95ff34c 100644 --- a/charts/core/src/Echarts/useEchartsInstance.ts +++ b/charts/core/src/Echarts/useEchart.ts @@ -1,16 +1,8 @@ import { useEffect, useRef, useState } from 'react'; -// Type for the ECharts instance -type EchartsType = any; +type EchartsType = any; // has to be any since no types exist until import -interface UseEchartsResult { - chart: EchartsType | null; - isLoading: boolean; - error: Error | null; - echarts: any; -} - -// Singleton promise for initialization +// Singleton promise for initialization to prevent duplicatation let initPromise: Promise | null = null; let echartsCore: any; let echartsCharts: any; @@ -69,15 +61,13 @@ async function initializeEcharts() { return initPromise; } -export function useEchartsInstance( - container: HTMLDivElement | null, -): UseEchartsResult { +export function useEchart(container: HTMLDivElement | null) { const [chart, setChart] = useState(null); const [isLoading, setIsLoading] = useState(true); const [error, setError] = useState(null); useEffect(() => { - async function setupChart() { + (async function setupChart() { try { setIsLoading(true); setError(null); @@ -86,7 +76,6 @@ export function useEchartsInstance( if (container) { const newChart = echartsCore.init(container); - console.log('post init'); setChart(newChart); } } catch (err) { @@ -99,9 +88,7 @@ export function useEchartsInstance( } finally { setIsLoading(false); } - } - - setupChart(); + })(); // Cleanup function return () => { @@ -111,5 +98,10 @@ export function useEchartsInstance( }; }, [container]); - return { chart, echarts: echartsCore, isLoading, error }; + function addToGroup(groupId: string) { + chart.group = groupId; + echartsCore.connect(groupId); + } + + return { chart, addToGroup, core: echartsCore, isLoading, error }; } From ab255d3ca51982b495cc8099b26a42e4103a30b2 Mon Sep 17 00:00:00 2001 From: Terrence Keane Date: Fri, 13 Dec 2024 10:47:08 -0500 Subject: [PATCH 04/21] Working state --- charts/core/src/Chart/hooks/useChart.ts | 158 +++------------ charts/core/src/Echarts/getEchartsInstance.ts | 57 ------ charts/core/src/Echarts/useEchart.ts | 188 +++++++++++++++++- 3 files changed, 208 insertions(+), 195 deletions(-) delete mode 100644 charts/core/src/Echarts/getEchartsInstance.ts diff --git a/charts/core/src/Chart/hooks/useChart.ts b/charts/core/src/Chart/hooks/useChart.ts index e03b31807b..54c3defd01 100644 --- a/charts/core/src/Chart/hooks/useChart.ts +++ b/charts/core/src/Chart/hooks/useChart.ts @@ -7,24 +7,26 @@ import { getDefaultChartOptions } from '../config'; import { addSeries, removeSeries, updateOptions } from './updateUtils'; import { ChartHookProps, ZoomSelectionEvent } from './useChart.types'; -import { useEchart } from '../../Echarts/useEchart'; +import { useEchart } from '../../echarts/useEchart'; -/** - * Creates a generic Apache ECharts options object with default values for those not set - * that are in line with the designs and needs of the design system. - */ export function useChart({ - theme, onChartReady = () => {}, zoomSelect, onZoomSelect, groupId, }: ChartHookProps) { const chartRef = useRef(null); - const { chart, core, addToGroup } = useEchart(chartRef.current); - const [chartOptions, setChartOptions] = useState( - getDefaultChartOptions(theme), - ); + const { + chart, + isLoading, + chartOptions, + on, + addToGroup, + setupZoomSelect, + addChartSeries, + removeChartSeries, + updateChartOptions, + } = useEchart(chartRef.current); function handleDataZoom(params: any) { if (onZoomSelect) { @@ -61,138 +63,28 @@ export function useChart({ ); } } - - /** - * If start is not 0% or end is not 100%, the 'dataZoom' event was triggered by a zoom. - * We override the zoom to provide new values to the handler. - */ - const isZoomed = params?.start !== 0 || params?.end !== 100; - - if (chart && isZoomed) { - chart.dispatchAction({ - type: 'dataZoom', - start: 0, // percentage of starting position - end: 100, // percentage of ending position - }); - } } - // Initialize the chart useEffect(() => { - if (!chart || !core) { - return; - } - - chart.setOption(chartOptions); - - if (groupId) { - addToGroup(groupId); - } - - // ECharts does not automatically resize when the window resizes. - const resizeHandler = () => { - chart.resize(); - }; - window.addEventListener('resize', resizeHandler); - - chart.on('rendered', function onInitialRender() { - // Enable zooming by default by dispatching the "dataZoomSelect" action. - chart.dispatchAction({ - type: 'takeGlobalCursor', - key: 'dataZoomSelect', - dataZoomSelectActive: zoomSelect?.xAxis || zoomSelect?.yAxis, - }); - - chart.on('dataZoom', handleDataZoom); + if (chart) { onChartReady(); - chart.off('rendered', onInitialRender); - }); - - return () => { - window.removeEventListener('resize', resizeHandler); - chart.dispose(); - }; - }, [chart, core]); + } + }, [chart]); - // Set which axis zoom is enabled on useEffect(() => { - // `0` index enables zoom on that index, `'none'` disables zoom on that index - let xAxisIndex: number | string = 0; - let yAxisIndex: number | string = 0; - if (!zoomSelect?.xAxis) { - xAxisIndex = 'none'; - } - if (!zoomSelect?.yAxis) { - yAxisIndex = 'none'; + if (chart) { + if (groupId) { + addToGroup(groupId); + } } - updateChartOptions({ - toolbox: { - feature: { - dataZoom: { - xAxisIndex, - yAxisIndex, - }, - }, - }, - }); - }, [zoomSelect]); - - const updateChartRef = useMemo( - () => - debounce((chartOptions: Partial) => { - /** - * The second argument is `true` to merge the new options with the existing ones. - * This is needed to ensure that series get removed properly. - * See issue: https://github.com/apache/echarts/issues/6202 - * */ - chart?.setOption(chartOptions, true); - }, 50), - [], - ); - - const addChartSeries = useCallback( - (data: SeriesOption) => { - setChartOptions(currentOptions => { - const updatedOptions = addSeries(currentOptions, data); - updateChartRef(updatedOptions); - return updatedOptions; - }); - }, - [updateChartRef], - ); - - const removeChartSeries = useCallback( - (name: string) => { - setChartOptions(currentOptions => { - const updatedOptions = removeSeries(currentOptions, name); - updateChartRef(updatedOptions); - return updatedOptions; - }); - }, - [updateChartRef], - ); - - const updateChartOptions = useCallback( - (options: Omit, 'series'>) => { - setChartOptions(currentOptions => { - const updatedOptions = updateOptions(currentOptions, options); - updateChartRef(updatedOptions); - return updatedOptions; - }); - }, - [updateChartRef], - ); + }, [chart, groupId]); useEffect(() => { - setChartOptions(currentOptions => { - const updatedOptions = { - ...currentOptions, - color: chartSeriesColors[theme], - }; - updateChartRef(updatedOptions); - return updatedOptions; - }); - }, [theme]); + if (chart) { + setupZoomSelect({ xAxis: zoomSelect?.xAxis, yAxis: zoomSelect?.yAxis }); + on('zoomSelect', handleDataZoom); + } + }, [chart, zoomSelect]); return { chartOptions, diff --git a/charts/core/src/Echarts/getEchartsInstance.ts b/charts/core/src/Echarts/getEchartsInstance.ts deleted file mode 100644 index b5d4627799..0000000000 --- a/charts/core/src/Echarts/getEchartsInstance.ts +++ /dev/null @@ -1,57 +0,0 @@ -// Create a singleton promise to handle the initialization -let initPromise: Promise | null = null; -let echartsCore: any; -let echartsCharts: any; -let echartsRenders: any; -let echartsComponents: any; - -async function initializeEcharts() { - // If already initialized, return immediately - if (echartsCore) { - return; - } - - // If initialization is in progress, wait for it - if (initPromise) { - return initPromise; - } - - // Start initialization and store the promise - initPromise = (async () => { - const [ - echartsCoreResolved, - echartsChartsResolved, - echartsRendersResolved, - echartsComponentsResolved, - ] = await Promise.all([ - import('echarts/core'), - import('echarts/charts'), - import('echarts/renderers'), - import('echarts/components'), - ]); - - echartsCore = echartsCoreResolved; - echartsCharts = echartsChartsResolved; - echartsRenders = echartsRendersResolved; - echartsComponents = echartsComponentsResolved; - - echartsCore.use([ - echartsCharts.LineChart, - echartsRenders.CanvasRenderer, - echartsComponents.TitleComponent, - echartsComponents.TooltipComponent, - echartsComponents.GridComponent, - echartsComponents.LegendComponent, - echartsComponents.ToolboxComponent, - echartsComponents.DataZoomComponent, - echartsComponents.DataZoomInsideComponent, - ]); - })(); - - return initPromise; -} - -export async function getEchartsInstance(container: HTMLElement | null) { - await initializeEcharts(); - return { instance: echartsCore.init(container), core: echartsCore }; -} diff --git a/charts/core/src/Echarts/useEchart.ts b/charts/core/src/Echarts/useEchart.ts index 62f95ff34c..651544177c 100644 --- a/charts/core/src/Echarts/useEchart.ts +++ b/charts/core/src/Echarts/useEchart.ts @@ -1,4 +1,16 @@ -import { useEffect, useRef, useState } from 'react'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import debounce from 'lodash.debounce'; + +import { useDarkMode } from '@leafygreen-ui/leafygreen-provider'; + +import { getDefaultChartOptions } from '../Chart/config'; +import { ChartOptions, SeriesOption } from '../Chart'; +import { + addSeries, + removeSeries, + updateOptions, +} from '../Chart/hooks/updateUtils'; +import { chartSeriesColors } from '../Chart/chartSeriesColors'; type EchartsType = any; // has to be any since no types exist until import @@ -63,21 +75,36 @@ async function initializeEcharts() { export function useEchart(container: HTMLDivElement | null) { const [chart, setChart] = useState(null); - const [isLoading, setIsLoading] = useState(true); + const [isReady, setisReady] = useState(true); const [error, setError] = useState(null); + const { theme } = useDarkMode(); + const [chartOptions, setChartOptions] = useState( + getDefaultChartOptions(theme), + ); + + // ECharts does not automatically resize when the window resizes. + const resizeHandler = () => { + if (chart) { + chart.resize(); + } + }; useEffect(() => { (async function setupChart() { try { - setIsLoading(true); + setisReady(true); setError(null); await initializeEcharts(); if (container) { const newChart = echartsCore.init(container); + newChart.setOption(chartOptions); + window.addEventListener('resize', resizeHandler); setChart(newChart); } + + console.log('is ready'); } catch (err) { console.error(err); setError( @@ -86,22 +113,173 @@ export function useEchart(container: HTMLDivElement | null) { : new Error('Failed to initialize ECharts'), ); } finally { - setIsLoading(false); + setisReady(false); } })(); // Cleanup function return () => { if (chart) { + window.removeEventListener('resize', resizeHandler); chart.dispose(); } }; }, [container]); + useEffect(() => { + setChartOptions(currentOptions => { + const updatedOptions = { + ...currentOptions, + color: chartSeriesColors[theme], + }; + setEchartOptions(updatedOptions); + return updatedOptions; + }); + }, [theme]); + function addToGroup(groupId: string) { chart.group = groupId; echartsCore.connect(groupId); } - return { chart, addToGroup, core: echartsCore, isLoading, error }; + function removeFromGroup() { + chart.group = null; + } + + function setupZoomSelect({ + xAxis, + yAxis, + }: { + xAxis?: boolean; + yAxis?: boolean; + }) { + if (chart) { + // `0` index enables zoom on that index, `'none'` disables zoom on that index + let xAxisIndex: number | string = 0; + let yAxisIndex: number | string = 0; + + if (!xAxis) { + xAxisIndex = 'none'; + } + + if (!yAxis) { + yAxisIndex = 'none'; + } + + updateChartOptions({ + toolbox: { + feature: { + dataZoom: { + xAxisIndex, + yAxisIndex, + }, + }, + }, + }); + + chart.on('rendered', () => { + chart.dispatchAction({ + type: 'takeGlobalCursor', + key: 'dataZoomSelect', + dataZoomSelectActive: xAxis || yAxis, + }); + }); + + chart.on('dataZoom', (params: any) => { + /** + * If start is not 0% or end is not 100%, the 'dataZoom' event was triggered by a zoom. + * We override the zoom to prevent it from actually zooming. + */ + const isZoomed = params?.start !== 0 || params?.end !== 100; + + if (chart && isZoomed) { + chart.dispatchAction({ + type: 'dataZoom', + start: 0, // percentage of starting position + end: 100, // percentage of ending position + }); + } + }); + } + } + + // TODO: update type + function on(action: 'zoomSelect' | string, callback: any) { + switch (action) { + case 'zoomSelect': { + chart.on('dataZoom', (params: any) => { + callback(params); // TODO: Add logic + }); + break; + } + default: { + chart.on(action, callback); + } + } + } + + function off(action: string, callback: any) { + chart.off(action, callback); + } + + const setEchartOptions = useMemo( + () => + debounce((chartOptions: Partial) => { + /** + * The second argument is `true` to merge the new options with the existing ones. + * This is needed to ensure that series get removed properly. + * See issue: https://github.com/apache/echarts/issues/6202 + * */ + chart?.setOption(chartOptions, true); + }, 50), + [], + ); + + const addChartSeries = useCallback( + (data: SeriesOption) => { + setChartOptions(currentOptions => { + const updatedOptions = addSeries(currentOptions, data); + setEchartOptions(updatedOptions); + return updatedOptions; + }); + }, + [setEchartOptions], + ); + + const removeChartSeries = useCallback( + (name: string) => { + setChartOptions(currentOptions => { + const updatedOptions = removeSeries(currentOptions, name); + setEchartOptions(updatedOptions); + return updatedOptions; + }); + }, + [setEchartOptions], + ); + + const updateChartOptions = useCallback( + (options: Omit, 'series'>) => { + setChartOptions(currentOptions => { + const updatedOptions = updateOptions(currentOptions, options); + setEchartOptions(updatedOptions); + return updatedOptions; + }); + }, + [setEchartOptions], + ); + + return { + chart, + chartOptions, + addToGroup, + removeFromGroup, + setupZoomSelect, + on, + off, + addChartSeries, + removeChartSeries, + updateChartOptions, + isReady, + error, + }; } From 0f2e3ed22ce7fd31c865a3bf7d67aebb92139b04 Mon Sep 17 00:00:00 2001 From: Terrence Keane Date: Fri, 13 Dec 2024 10:47:51 -0500 Subject: [PATCH 05/21] Working state --- charts/core/src/Chart/hooks/useChart.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/charts/core/src/Chart/hooks/useChart.ts b/charts/core/src/Chart/hooks/useChart.ts index 54c3defd01..0acbfc049e 100644 --- a/charts/core/src/Chart/hooks/useChart.ts +++ b/charts/core/src/Chart/hooks/useChart.ts @@ -18,7 +18,6 @@ export function useChart({ const chartRef = useRef(null); const { chart, - isLoading, chartOptions, on, addToGroup, From d874a1023f9f94cb17e48765567a3930f23b00ef Mon Sep 17 00:00:00 2001 From: Terrence Keane Date: Fri, 13 Dec 2024 10:55:07 -0500 Subject: [PATCH 06/21] Small fix --- charts/core/src/Chart/hooks/useChart.ts | 8 +------- charts/core/src/Echarts/useEchart.ts | 10 +++++----- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/charts/core/src/Chart/hooks/useChart.ts b/charts/core/src/Chart/hooks/useChart.ts index 0acbfc049e..c46a1b720e 100644 --- a/charts/core/src/Chart/hooks/useChart.ts +++ b/charts/core/src/Chart/hooks/useChart.ts @@ -1,11 +1,5 @@ -import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; -import debounce from 'lodash.debounce'; +import { useEffect, useRef } from 'react'; -import { ChartOptions, SeriesOption } from '../../Chart/Chart.types'; -import { chartSeriesColors } from '../chartSeriesColors'; -import { getDefaultChartOptions } from '../config'; - -import { addSeries, removeSeries, updateOptions } from './updateUtils'; import { ChartHookProps, ZoomSelectionEvent } from './useChart.types'; import { useEchart } from '../../echarts/useEchart'; diff --git a/charts/core/src/Echarts/useEchart.ts b/charts/core/src/Echarts/useEchart.ts index 651544177c..2110afc71a 100644 --- a/charts/core/src/Echarts/useEchart.ts +++ b/charts/core/src/Echarts/useEchart.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { useCallback, useEffect, useMemo, useState } from 'react'; import debounce from 'lodash.debounce'; import { useDarkMode } from '@leafygreen-ui/leafygreen-provider'; @@ -75,7 +75,7 @@ async function initializeEcharts() { export function useEchart(container: HTMLDivElement | null) { const [chart, setChart] = useState(null); - const [isReady, setisReady] = useState(true); + const [isReady, setIsReady] = useState(true); const [error, setError] = useState(null); const { theme } = useDarkMode(); const [chartOptions, setChartOptions] = useState( @@ -92,7 +92,7 @@ export function useEchart(container: HTMLDivElement | null) { useEffect(() => { (async function setupChart() { try { - setisReady(true); + setIsReady(true); setError(null); await initializeEcharts(); @@ -113,7 +113,7 @@ export function useEchart(container: HTMLDivElement | null) { : new Error('Failed to initialize ECharts'), ); } finally { - setisReady(false); + setIsReady(false); } })(); @@ -208,7 +208,7 @@ export function useEchart(container: HTMLDivElement | null) { switch (action) { case 'zoomSelect': { chart.on('dataZoom', (params: any) => { - callback(params); // TODO: Add logic + callback(params); // TODO: Add logic to parse data }); break; } From 621f3203739d58e7e0ad8ed0516bde6946c6240f Mon Sep 17 00:00:00 2001 From: Terrence Keane Date: Fri, 13 Dec 2024 10:55:21 -0500 Subject: [PATCH 07/21] typo --- charts/core/src/Echarts/useEchart.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/core/src/Echarts/useEchart.ts b/charts/core/src/Echarts/useEchart.ts index 2110afc71a..38f0866350 100644 --- a/charts/core/src/Echarts/useEchart.ts +++ b/charts/core/src/Echarts/useEchart.ts @@ -14,7 +14,7 @@ import { chartSeriesColors } from '../Chart/chartSeriesColors'; type EchartsType = any; // has to be any since no types exist until import -// Singleton promise for initialization to prevent duplicatation +// Singleton promise for initialization to prevent duplication let initPromise: Promise | null = null; let echartsCore: any; let echartsCharts: any; From 76323cd0869ca66131500beb26d3783417f117fb Mon Sep 17 00:00:00 2001 From: Terrence Keane Date: Fri, 13 Dec 2024 11:16:18 -0500 Subject: [PATCH 08/21] Shorten names --- charts/core/src/Chart/hooks/useChart.ts | 40 +++++------ charts/core/src/Echarts/useEchart.ts | 92 +++++++++++-------------- 2 files changed, 58 insertions(+), 74 deletions(-) diff --git a/charts/core/src/Chart/hooks/useChart.ts b/charts/core/src/Chart/hooks/useChart.ts index c46a1b720e..d3efb62ea7 100644 --- a/charts/core/src/Chart/hooks/useChart.ts +++ b/charts/core/src/Chart/hooks/useChart.ts @@ -10,16 +10,7 @@ export function useChart({ groupId, }: ChartHookProps) { const chartRef = useRef(null); - const { - chart, - chartOptions, - on, - addToGroup, - setupZoomSelect, - addChartSeries, - removeChartSeries, - updateChartOptions, - } = useEchart(chartRef.current); + const chart = useEchart(chartRef.current); function handleDataZoom(params: any) { if (onZoomSelect) { @@ -59,31 +50,34 @@ export function useChart({ } useEffect(() => { - if (chart) { + if (chart.instance) { onChartReady(); } - }, [chart]); + }, [chart.instance]); useEffect(() => { - if (chart) { + if (chart.instance) { if (groupId) { - addToGroup(groupId); + chart.addToGroup(groupId); } } - }, [chart, groupId]); + }, [chart.instance, groupId]); useEffect(() => { - if (chart) { - setupZoomSelect({ xAxis: zoomSelect?.xAxis, yAxis: zoomSelect?.yAxis }); - on('zoomSelect', handleDataZoom); + if (chart.instance) { + chart.setupZoomSelect({ + xAxis: zoomSelect?.xAxis, + yAxis: zoomSelect?.yAxis, + }); + chart.on('zoomSelect', handleDataZoom); } - }, [chart, zoomSelect]); + }, [chart.instance, zoomSelect]); return { - chartOptions, - updateChartOptions, - addChartSeries, - removeChartSeries, + chartOptions: chart.options, + updateChartOptions: chart.updateOptions, + addChartSeries: chart.addSeries, + removeChartSeries: chart.removeSeries, chartRef, }; } diff --git a/charts/core/src/Echarts/useEchart.ts b/charts/core/src/Echarts/useEchart.ts index 38f0866350..755e3bef69 100644 --- a/charts/core/src/Echarts/useEchart.ts +++ b/charts/core/src/Echarts/useEchart.ts @@ -5,11 +5,7 @@ import { useDarkMode } from '@leafygreen-ui/leafygreen-provider'; import { getDefaultChartOptions } from '../Chart/config'; import { ChartOptions, SeriesOption } from '../Chart'; -import { - addSeries, - removeSeries, - updateOptions, -} from '../Chart/hooks/updateUtils'; +import * as updateUtils from '../Chart/hooks/updateUtils'; import { chartSeriesColors } from '../Chart/chartSeriesColors'; type EchartsType = any; // has to be any since no types exist until import @@ -74,37 +70,31 @@ async function initializeEcharts() { } export function useEchart(container: HTMLDivElement | null) { - const [chart, setChart] = useState(null); - const [isReady, setIsReady] = useState(true); + const [chartInstance, setChartInstance] = useState(null); const [error, setError] = useState(null); const { theme } = useDarkMode(); - const [chartOptions, setChartOptions] = useState( - getDefaultChartOptions(theme), - ); + const [options, setOptions] = useState(getDefaultChartOptions(theme)); // ECharts does not automatically resize when the window resizes. const resizeHandler = () => { - if (chart) { - chart.resize(); + if (chartInstance) { + chartInstance.resize(); } }; useEffect(() => { (async function setupChart() { try { - setIsReady(true); setError(null); await initializeEcharts(); if (container) { const newChart = echartsCore.init(container); - newChart.setOption(chartOptions); + newChart.setOption(options); window.addEventListener('resize', resizeHandler); - setChart(newChart); + setChartInstance(newChart); } - - console.log('is ready'); } catch (err) { console.error(err); setError( @@ -112,22 +102,20 @@ export function useEchart(container: HTMLDivElement | null) { ? err : new Error('Failed to initialize ECharts'), ); - } finally { - setIsReady(false); } })(); // Cleanup function return () => { - if (chart) { + if (chartInstance) { window.removeEventListener('resize', resizeHandler); - chart.dispose(); + chartInstance.dispose(); } }; }, [container]); useEffect(() => { - setChartOptions(currentOptions => { + setOptions(currentOptions => { const updatedOptions = { ...currentOptions, color: chartSeriesColors[theme], @@ -138,12 +126,12 @@ export function useEchart(container: HTMLDivElement | null) { }, [theme]); function addToGroup(groupId: string) { - chart.group = groupId; + chartInstance.group = groupId; echartsCore.connect(groupId); } function removeFromGroup() { - chart.group = null; + chartInstance.group = null; } function setupZoomSelect({ @@ -153,7 +141,7 @@ export function useEchart(container: HTMLDivElement | null) { xAxis?: boolean; yAxis?: boolean; }) { - if (chart) { + if (chartInstance) { // `0` index enables zoom on that index, `'none'` disables zoom on that index let xAxisIndex: number | string = 0; let yAxisIndex: number | string = 0; @@ -166,7 +154,7 @@ export function useEchart(container: HTMLDivElement | null) { yAxisIndex = 'none'; } - updateChartOptions({ + updateOptions({ toolbox: { feature: { dataZoom: { @@ -177,23 +165,23 @@ export function useEchart(container: HTMLDivElement | null) { }, }); - chart.on('rendered', () => { - chart.dispatchAction({ + chartInstance.on('rendered', () => { + chartInstance.dispatchAction({ type: 'takeGlobalCursor', key: 'dataZoomSelect', dataZoomSelectActive: xAxis || yAxis, }); }); - chart.on('dataZoom', (params: any) => { + chartInstance.on('dataZoom', (params: any) => { /** * If start is not 0% or end is not 100%, the 'dataZoom' event was triggered by a zoom. * We override the zoom to prevent it from actually zooming. */ const isZoomed = params?.start !== 0 || params?.end !== 100; - if (chart && isZoomed) { - chart.dispatchAction({ + if (chartInstance && isZoomed) { + chartInstance.dispatchAction({ type: 'dataZoom', start: 0, // percentage of starting position end: 100, // percentage of ending position @@ -207,38 +195,38 @@ export function useEchart(container: HTMLDivElement | null) { function on(action: 'zoomSelect' | string, callback: any) { switch (action) { case 'zoomSelect': { - chart.on('dataZoom', (params: any) => { + chartInstance.on('dataZoom', (params: any) => { callback(params); // TODO: Add logic to parse data }); break; } default: { - chart.on(action, callback); + chartInstance.on(action, callback); } } } function off(action: string, callback: any) { - chart.off(action, callback); + chartInstance.off(action, callback); } const setEchartOptions = useMemo( () => - debounce((chartOptions: Partial) => { + debounce((options: Partial) => { /** * The second argument is `true` to merge the new options with the existing ones. * This is needed to ensure that series get removed properly. * See issue: https://github.com/apache/echarts/issues/6202 * */ - chart?.setOption(chartOptions, true); + chartInstance?.setOption(options, true); }, 50), [], ); - const addChartSeries = useCallback( + const addSeries = useCallback( (data: SeriesOption) => { - setChartOptions(currentOptions => { - const updatedOptions = addSeries(currentOptions, data); + setOptions(currentOptions => { + const updatedOptions = updateUtils.addSeries(currentOptions, data); setEchartOptions(updatedOptions); return updatedOptions; }); @@ -246,10 +234,10 @@ export function useEchart(container: HTMLDivElement | null) { [setEchartOptions], ); - const removeChartSeries = useCallback( + const removeSeries = useCallback( (name: string) => { - setChartOptions(currentOptions => { - const updatedOptions = removeSeries(currentOptions, name); + setOptions(currentOptions => { + const updatedOptions = updateUtils.removeSeries(currentOptions, name); setEchartOptions(updatedOptions); return updatedOptions; }); @@ -257,10 +245,13 @@ export function useEchart(container: HTMLDivElement | null) { [setEchartOptions], ); - const updateChartOptions = useCallback( + const updateOptions = useCallback( (options: Omit, 'series'>) => { - setChartOptions(currentOptions => { - const updatedOptions = updateOptions(currentOptions, options); + setOptions(currentOptions => { + const updatedOptions = updateUtils.updateOptions( + currentOptions, + options, + ); setEchartOptions(updatedOptions); return updatedOptions; }); @@ -269,17 +260,16 @@ export function useEchart(container: HTMLDivElement | null) { ); return { - chart, - chartOptions, + instance: chartInstance, + options, addToGroup, removeFromGroup, setupZoomSelect, on, off, - addChartSeries, - removeChartSeries, - updateChartOptions, - isReady, + addSeries, + removeSeries, + updateOptions, error, }; } From 16cfee20c4a2f4f3f530571f1e79b36ec151ce95 Mon Sep 17 00:00:00 2001 From: Terrence Keane Date: Fri, 13 Dec 2024 13:01:09 -0500 Subject: [PATCH 09/21] Improve typing --- charts/core/src/Chart/hooks/useChart.ts | 28 +++--- charts/core/src/Echarts/echarts.types.ts | 85 +++++++++++++++++ charts/core/src/Echarts/useEchart.ts | 116 +++++++++++++---------- 3 files changed, 164 insertions(+), 65 deletions(-) create mode 100644 charts/core/src/Echarts/echarts.types.ts diff --git a/charts/core/src/Chart/hooks/useChart.ts b/charts/core/src/Chart/hooks/useChart.ts index d3efb62ea7..7263acea61 100644 --- a/charts/core/src/Chart/hooks/useChart.ts +++ b/charts/core/src/Chart/hooks/useChart.ts @@ -10,7 +10,7 @@ export function useChart({ groupId, }: ChartHookProps) { const chartRef = useRef(null); - const chart = useEchart(chartRef.current); + const echart = useEchart(chartRef.current); function handleDataZoom(params: any) { if (onZoomSelect) { @@ -50,34 +50,34 @@ export function useChart({ } useEffect(() => { - if (chart.instance) { + if (echart.ready) { onChartReady(); } - }, [chart.instance]); + }, [echart.ready]); useEffect(() => { - if (chart.instance) { + if (echart.ready) { if (groupId) { - chart.addToGroup(groupId); + echart.addToGroup(groupId); } } - }, [chart.instance, groupId]); + }, [echart.ready, groupId]); useEffect(() => { - if (chart.instance) { - chart.setupZoomSelect({ + if (echart.ready) { + echart.setupZoomSelect({ xAxis: zoomSelect?.xAxis, yAxis: zoomSelect?.yAxis, }); - chart.on('zoomSelect', handleDataZoom); + echart.on('zoomselect', handleDataZoom); } - }, [chart.instance, zoomSelect]); + }, [echart.ready, zoomSelect]); return { - chartOptions: chart.options, - updateChartOptions: chart.updateOptions, - addChartSeries: chart.addSeries, - removeChartSeries: chart.removeSeries, + chartOptions: echart.options, + updateChartOptions: echart.updateOptions, + addChartSeries: echart.addSeries, + removeChartSeries: echart.removeSeries, chartRef, }; } diff --git a/charts/core/src/Echarts/echarts.types.ts b/charts/core/src/Echarts/echarts.types.ts new file mode 100644 index 0000000000..fb6d958854 --- /dev/null +++ b/charts/core/src/Echarts/echarts.types.ts @@ -0,0 +1,85 @@ +import { ChartOptions, SeriesOption } from '../Chart'; + +type EChartsEvents = + // Mouse Events + | 'click' // Triggered when clicking on a series, data item, or other graphical element + | 'dblclick' // Triggered when double clicking + | 'mousedown' // Triggered when pressing down the mouse button + | 'mouseup' // Triggered when releasing the mouse button + | 'mouseover' // Triggered when mouse hovers over a component + | 'mouseout' // Triggered when mouse leaves a component + | 'mousemove' // Triggered when mouse moves within a component + | 'globalout' // Triggered when mouse leaves the chart area + | 'contextmenu' // Triggered when right clicking + + // Drag Events + | 'dragstart' // Triggered when starting to drag a component + | 'drag' // Triggered while dragging + | 'dragend' // Triggered when ending a drag operation + + // Brush Events + | 'brushselected' // Triggered when selecting an area using the brush tool + | 'brushEnd' // Triggered when brush selection ends + | 'brush' // Triggered during brush selection + + // Legend Events + | 'legendselectchanged' // Triggered when legend selection changes + | 'legendselected' // Triggered when clicking legend item + | 'legendunselected' // Triggered when unselecting legend item + | 'legendselectall' // Triggered when selecting all legend items + | 'legendinverseselect' // Triggered when inversely selecting legend items + | 'legendscroll' // Triggered when scrolling legend + + // Dataoom Events + | 'datazoom' // Triggered when data zoom area changes + | 'datarangeselected' // Triggered when selecting a range in continuous visual map + | 'timelinechanged' // Triggered when timeline changes + | 'timelineplaychanged' // Triggered when timeline play state changes + | 'restore' // Triggered when restoring chart to initial state + + // Map Events + | 'georoam' // Triggered when roaming (zooming/panning) in geo coordinate system + | 'geoselected' // Triggered when selecting areas in geo coordinate system + | 'geounselected' // Triggered when unselecting areas in geo coordinate system + + // Axis Events + | 'axisareaselected' // Triggered when selecting an axis area + | 'focusnodeadjacency' // Triggered when focusing on adjacent nodes (graph) + | 'unfocusnodeadjacency' // Triggered when unfocusing adjacent nodes (graph) + + // Rendering Events + | 'rendered' // Triggered after chart rendering finished + | 'finished' // Triggered after chart animation finished + + // Chart Action Events + | 'magictypechanged' // Triggered when changing chart type using magic type switcher + | 'tooltipshown' // Triggered when tooltip is shown + | 'tooltiphidden' // Triggered when tooltip is hidden + | 'selectchanged' // Triggered when selection state changes + | 'globalcursortaken'; // Triggered when global cursor is taken + +export interface setupZoomSelectProps { + xAxis?: boolean; + yAxis?: boolean; +} + +export interface EChartsInstance { + _echartsInstance: any; + ready: boolean; + options: Partial; + updateOptions: (options: Omit, 'series'>) => void; + on: ( + event: EChartsEvents | 'zoomselect', + callback: (params: any) => void, + ) => void; + off: ( + event: EChartsEvents | 'zoomselect', + callback: (params: any) => void, + ) => void; + addSeries: (series: SeriesOption) => void; + removeSeries: (name: string) => void; + addToGroup: (groupId: string) => void; + removeFromGroup: () => void; + setupZoomSelect: (setupZoomSelectProps: setupZoomSelectProps) => void; + error: Error | null; +} diff --git a/charts/core/src/Echarts/useEchart.ts b/charts/core/src/Echarts/useEchart.ts index 755e3bef69..5b4f8457f5 100644 --- a/charts/core/src/Echarts/useEchart.ts +++ b/charts/core/src/Echarts/useEchart.ts @@ -4,11 +4,10 @@ import debounce from 'lodash.debounce'; import { useDarkMode } from '@leafygreen-ui/leafygreen-provider'; import { getDefaultChartOptions } from '../Chart/config'; -import { ChartOptions, SeriesOption } from '../Chart'; +import { ChartOptions } from '../Chart'; import * as updateUtils from '../Chart/hooks/updateUtils'; import { chartSeriesColors } from '../Chart/chartSeriesColors'; - -type EchartsType = any; // has to be any since no types exist until import +import { EChartsInstance } from './echarts.types'; // Singleton promise for initialization to prevent duplication let initPromise: Promise | null = null; @@ -69,16 +68,24 @@ async function initializeEcharts() { return initPromise; } -export function useEchart(container: HTMLDivElement | null) { - const [chartInstance, setChartInstance] = useState(null); - const [error, setError] = useState(null); +/** + * Wrapper around the ECharts library. Instantiates and returns an ECharts instance. + * Provides helper methods to hide ECharts specific logic and provide a clean API + * for interacting with a chart. + */ +export function useEchart(container: HTMLDivElement | null): EChartsInstance { + const [echartsInstance, setEchartsInstance] = useState(null); // has to be any since no types exist until import + const [error, setError] = useState(null); + const [ready, setReady] = useState(false); const { theme } = useDarkMode(); - const [options, setOptions] = useState(getDefaultChartOptions(theme)); + const [options, setOptions] = useState( + getDefaultChartOptions(theme), + ); // ECharts does not automatically resize when the window resizes. const resizeHandler = () => { - if (chartInstance) { - chartInstance.resize(); + if (echartsInstance) { + echartsInstance.resize(); } }; @@ -93,9 +100,11 @@ export function useEchart(container: HTMLDivElement | null) { const newChart = echartsCore.init(container); newChart.setOption(options); window.addEventListener('resize', resizeHandler); - setChartInstance(newChart); + setEchartsInstance(newChart); + setReady(true); } } catch (err) { + setReady(false); console.error(err); setError( err instanceof Error @@ -107,9 +116,9 @@ export function useEchart(container: HTMLDivElement | null) { // Cleanup function return () => { - if (chartInstance) { + if (echartsInstance) { window.removeEventListener('resize', resizeHandler); - chartInstance.dispose(); + echartsInstance.dispose(); } }; }, [container]); @@ -125,23 +134,20 @@ export function useEchart(container: HTMLDivElement | null) { }); }, [theme]); - function addToGroup(groupId: string) { - chartInstance.group = groupId; + const addToGroup: EChartsInstance['addToGroup'] = groupId => { + echartsInstance.group = groupId; echartsCore.connect(groupId); - } + }; - function removeFromGroup() { - chartInstance.group = null; - } + const removeFromGroup: EChartsInstance['removeFromGroup'] = () => { + echartsInstance.group = null; + }; - function setupZoomSelect({ + const setupZoomSelect: EChartsInstance['setupZoomSelect'] = ({ xAxis, yAxis, - }: { - xAxis?: boolean; - yAxis?: boolean; - }) { - if (chartInstance) { + }) => { + if (echartsInstance) { // `0` index enables zoom on that index, `'none'` disables zoom on that index let xAxisIndex: number | string = 0; let yAxisIndex: number | string = 0; @@ -165,23 +171,23 @@ export function useEchart(container: HTMLDivElement | null) { }, }); - chartInstance.on('rendered', () => { - chartInstance.dispatchAction({ + echartsInstance.on('rendered', () => { + echartsInstance.dispatchAction({ type: 'takeGlobalCursor', key: 'dataZoomSelect', dataZoomSelectActive: xAxis || yAxis, }); }); - chartInstance.on('dataZoom', (params: any) => { + echartsInstance.on('dataZoom', (params: any) => { /** * If start is not 0% or end is not 100%, the 'dataZoom' event was triggered by a zoom. * We override the zoom to prevent it from actually zooming. */ const isZoomed = params?.start !== 0 || params?.end !== 100; - if (chartInstance && isZoomed) { - chartInstance.dispatchAction({ + if (echartsInstance && isZoomed) { + echartsInstance.dispatchAction({ type: 'dataZoom', start: 0, // percentage of starting position end: 100, // percentage of ending position @@ -189,26 +195,33 @@ export function useEchart(container: HTMLDivElement | null) { } }); } - } + }; - // TODO: update type - function on(action: 'zoomSelect' | string, callback: any) { + const on: EChartsInstance['on'] = (action, callback) => { switch (action) { - case 'zoomSelect': { - chartInstance.on('dataZoom', (params: any) => { + case 'zoomselect': { + echartsInstance.on('datazoom', (params: any) => { callback(params); // TODO: Add logic to parse data }); break; } default: { - chartInstance.on(action, callback); + echartsInstance.on(action, callback); } } - } + }; - function off(action: string, callback: any) { - chartInstance.off(action, callback); - } + const off: EChartsInstance['off'] = (action, callback) => { + switch (action) { + case 'zoomselect': { + echartsInstance.off('datazoom', callback); + break; + } + default: { + echartsInstance.on(action, callback); + } + } + }; const setEchartOptions = useMemo( () => @@ -218,13 +231,13 @@ export function useEchart(container: HTMLDivElement | null) { * This is needed to ensure that series get removed properly. * See issue: https://github.com/apache/echarts/issues/6202 * */ - chartInstance?.setOption(options, true); + echartsInstance?.setOption(options, true); }, 50), [], ); - const addSeries = useCallback( - (data: SeriesOption) => { + const addSeries: EChartsInstance['addSeries'] = useCallback( + data => { setOptions(currentOptions => { const updatedOptions = updateUtils.addSeries(currentOptions, data); setEchartOptions(updatedOptions); @@ -234,8 +247,8 @@ export function useEchart(container: HTMLDivElement | null) { [setEchartOptions], ); - const removeSeries = useCallback( - (name: string) => { + const removeSeries: EChartsInstance['removeSeries'] = useCallback( + name => { setOptions(currentOptions => { const updatedOptions = updateUtils.removeSeries(currentOptions, name); setEchartOptions(updatedOptions); @@ -245,8 +258,8 @@ export function useEchart(container: HTMLDivElement | null) { [setEchartOptions], ); - const updateOptions = useCallback( - (options: Omit, 'series'>) => { + const updateOptions: EChartsInstance['updateOptions'] = useCallback( + options => { setOptions(currentOptions => { const updatedOptions = updateUtils.updateOptions( currentOptions, @@ -260,16 +273,17 @@ export function useEchart(container: HTMLDivElement | null) { ); return { - instance: chartInstance, + _echartsInstance: echartsInstance, + ready, options, - addToGroup, - removeFromGroup, - setupZoomSelect, + updateOptions, on, off, addSeries, removeSeries, - updateOptions, + addToGroup, + removeFromGroup, + setupZoomSelect, error, }; } From 5db200d2c7c138d0712aafedcee3729c8bfa16b2 Mon Sep 17 00:00:00 2001 From: Terrence Keane Date: Mon, 16 Dec 2024 12:44:33 -0500 Subject: [PATCH 10/21] Working order --- charts/core/src/Chart/hooks/useChart.ts | 53 ++----- charts/core/src/Echarts/echarts.types.ts | 22 ++- charts/core/src/Echarts/useEchart.ts | 191 ++++++++++++++++------- 3 files changed, 162 insertions(+), 104 deletions(-) diff --git a/charts/core/src/Chart/hooks/useChart.ts b/charts/core/src/Chart/hooks/useChart.ts index 7263acea61..3f729a6b26 100644 --- a/charts/core/src/Chart/hooks/useChart.ts +++ b/charts/core/src/Chart/hooks/useChart.ts @@ -1,6 +1,6 @@ import { useEffect, useRef } from 'react'; -import { ChartHookProps, ZoomSelectionEvent } from './useChart.types'; +import { ChartHookProps } from './useChart.types'; import { useEchart } from '../../echarts/useEchart'; export function useChart({ @@ -12,43 +12,6 @@ export function useChart({ const chartRef = useRef(null); const echart = useEchart(chartRef.current); - function handleDataZoom(params: any) { - if (onZoomSelect) { - if (zoomSelect?.xAxis && zoomSelect?.yAxis) { - if (params.batch) { - const { startValue: xStart, endValue: xEnd } = params.batch[0]; - const { startValue: yStart, endValue: yEnd } = params.batch[1]; - onZoomSelect({ - xAxis: { startValue: xStart, endValue: xEnd }, - yAxis: { startValue: yStart, endValue: yEnd }, - }); - } - } else if (zoomSelect?.xAxis || zoomSelect?.yAxis) { - const axis = zoomSelect?.xAxis ? 'xAxis' : 'yAxis'; - const zoomEventResponse: ZoomSelectionEvent = {}; - - if (params.startValue && params.endValue) { - zoomEventResponse[axis] = { - startValue: params.startValue, - endValue: params.endValue, - }; - onZoomSelect(zoomEventResponse); - } else if (params.batch) { - const { startValue, endValue } = params.batch[0]; - zoomEventResponse[axis] = { - startValue, - endValue, - }; - onZoomSelect(zoomEventResponse); - } - } else { - console.error( - 'zoomSelect configuration provided without any axes props. Either xAxis or yAxis must be set.', - ); - } - } - } - useEffect(() => { if (echart.ready) { onChartReady(); @@ -69,10 +32,22 @@ export function useChart({ xAxis: zoomSelect?.xAxis, yAxis: zoomSelect?.yAxis, }); - echart.on('zoomselect', handleDataZoom); + if (onZoomSelect) { + echart.on('zoomselect', zoomEventResponse => { + onZoomSelect(zoomEventResponse); + }); + } } }, [echart.ready, zoomSelect]); + useEffect(() => { + if (echart.ready && onZoomSelect) { + echart.on('zoomselect', zoomEventResponse => { + onZoomSelect(zoomEventResponse); + }); + } + }, [echart.ready, onZoomSelect]); + return { chartOptions: echart.options, updateChartOptions: echart.updateOptions, diff --git a/charts/core/src/Echarts/echarts.types.ts b/charts/core/src/Echarts/echarts.types.ts index fb6d958854..a07702a4f2 100644 --- a/charts/core/src/Echarts/echarts.types.ts +++ b/charts/core/src/Echarts/echarts.types.ts @@ -58,24 +58,30 @@ type EChartsEvents = | 'selectchanged' // Triggered when selection state changes | 'globalcursortaken'; // Triggered when global cursor is taken +export interface ZoomSelectionEvent { + xAxis?: { startValue: number; endValue: number }; + yAxis?: { startValue: number; endValue: number }; +} + export interface setupZoomSelectProps { xAxis?: boolean; yAxis?: boolean; } +type EChartsEventHandlerType = { + // Regular events use the original param type + (event: EChartsEvents, callback: (params: any) => void): void; + // Specific override for 'zoomselect' + (event: 'zoomselect', callback: (params: ZoomSelectionEvent) => void): void; +}; + export interface EChartsInstance { _echartsInstance: any; ready: boolean; options: Partial; updateOptions: (options: Omit, 'series'>) => void; - on: ( - event: EChartsEvents | 'zoomselect', - callback: (params: any) => void, - ) => void; - off: ( - event: EChartsEvents | 'zoomselect', - callback: (params: any) => void, - ) => void; + on: EChartsEventHandlerType; + off: EChartsEventHandlerType; addSeries: (series: SeriesOption) => void; removeSeries: (name: string) => void; addToGroup: (groupId: string) => void; diff --git a/charts/core/src/Echarts/useEchart.ts b/charts/core/src/Echarts/useEchart.ts index 5b4f8457f5..5ba678b538 100644 --- a/charts/core/src/Echarts/useEchart.ts +++ b/charts/core/src/Echarts/useEchart.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo, useState } from 'react'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import debounce from 'lodash.debounce'; import { useDarkMode } from '@leafygreen-ui/leafygreen-provider'; @@ -7,7 +7,7 @@ import { getDefaultChartOptions } from '../Chart/config'; import { ChartOptions } from '../Chart'; import * as updateUtils from '../Chart/hooks/updateUtils'; import { chartSeriesColors } from '../Chart/chartSeriesColors'; -import { EChartsInstance } from './echarts.types'; +import { EChartsInstance, ZoomSelectionEvent } from './echarts.types'; // Singleton promise for initialization to prevent duplication let initPromise: Promise | null = null; @@ -82,6 +82,9 @@ export function useEchart(container: HTMLDivElement | null): EChartsInstance { getDefaultChartOptions(theme), ); + // Keep track of active handlers + const activeHandlers = useRef(new Map()); + // ECharts does not automatically resize when the window resizes. const resizeHandler = () => { if (echartsInstance) { @@ -143,22 +146,43 @@ export function useEchart(container: HTMLDivElement | null): EChartsInstance { echartsInstance.group = null; }; - const setupZoomSelect: EChartsInstance['setupZoomSelect'] = ({ + const clearDataZoom = useCallback( + (params: any) => { + /** + * If start is not 0% or end is not 100%, the 'dataZoom' event was triggered by a zoom. + * We override the zoom to prevent it from actually zooming. + */ + const isZoomed = params?.start !== 0 || params?.end !== 100; + + if (echartsInstance && isZoomed) { + echartsInstance.dispatchAction({ + type: 'dataZoom', + start: 0, // percentage of starting position + end: 100, // percentage of ending position + }); + } + }, + [echartsInstance], + ); + + const setupZoomSelect: EChartsInstance['setupZoomSelect'] = async ({ xAxis, yAxis, }) => { + function enableZoom() { + echartsInstance.dispatchAction({ + type: 'takeGlobalCursor', + key: 'dataZoomSelect', + dataZoomSelectActive: xAxis || yAxis, + }); + // This will trigger a render so we need to remove the handler to prevent a loop + echartsInstance.off('rendered', enableZoom); + } + if (echartsInstance) { // `0` index enables zoom on that index, `'none'` disables zoom on that index - let xAxisIndex: number | string = 0; - let yAxisIndex: number | string = 0; - - if (!xAxis) { - xAxisIndex = 'none'; - } - - if (!yAxis) { - yAxisIndex = 'none'; - } + let xAxisIndex: number | string = xAxis ? 0 : 'none'; + let yAxisIndex: number | string = yAxis ? 0 : 'none'; updateOptions({ toolbox: { @@ -171,57 +195,110 @@ export function useEchart(container: HTMLDivElement | null): EChartsInstance { }, }); - echartsInstance.on('rendered', () => { - echartsInstance.dispatchAction({ - type: 'takeGlobalCursor', - key: 'dataZoomSelect', - dataZoomSelectActive: xAxis || yAxis, - }); - }); - - echartsInstance.on('dataZoom', (params: any) => { - /** - * If start is not 0% or end is not 100%, the 'dataZoom' event was triggered by a zoom. - * We override the zoom to prevent it from actually zooming. - */ - const isZoomed = params?.start !== 0 || params?.end !== 100; - - if (echartsInstance && isZoomed) { - echartsInstance.dispatchAction({ - type: 'dataZoom', - start: 0, // percentage of starting position - end: 100, // percentage of ending position - }); - } - }); + echartsInstance.on('rendered', enableZoom); + echartsInstance.off('dataZoom', clearDataZoom); // prevent adding dupes + echartsInstance.on('dataZoom', clearDataZoom); } }; - const on: EChartsInstance['on'] = (action, callback) => { - switch (action) { - case 'zoomselect': { - echartsInstance.on('datazoom', (params: any) => { - callback(params); // TODO: Add logic to parse data - }); - break; + const off: EChartsInstance['off'] = useCallback( + (action, callback) => { + if (!echartsInstance) return; + + switch (action) { + case 'zoomselect': { + echartsInstance.off('datazoom', callback); + // Remove from active handlers + activeHandlers.current.delete(`${action}-${callback.toString()}`); + break; + } + default: { + echartsInstance.off(action, callback); + activeHandlers.current.delete(`${action}-${callback.toString()}`); + } } - default: { - echartsInstance.on(action, callback); + }, + [echartsInstance], + ); + + const on: EChartsInstance['on'] = useCallback( + (action, callback) => { + if (!echartsInstance) return; + + // Create a unique key for this handler + const handlerKey = `${action}-${callback.toString()}`; + + // If this exact handler is already registered, skip + if (activeHandlers.current.has(handlerKey)) { + return; } - } - }; - const off: EChartsInstance['off'] = (action, callback) => { - switch (action) { - case 'zoomselect': { - echartsInstance.off('datazoom', callback); - break; + switch (action) { + case 'zoomselect': { + const zoomHandler = (params: any) => { + const zoomSelectionEvent: ZoomSelectionEvent = {}; + + if ( + params.startValue !== undefined && + params.endValue !== undefined + ) { + // Handle single axis zoom + const axis = params.dataZoomId?.includes('y') ? 'yAxis' : 'xAxis'; + zoomSelectionEvent[axis] = { + startValue: params.startValue, + endValue: params.endValue, + }; + callback(zoomSelectionEvent); + } else if (params.batch) { + // Handle batch zoom (multiple axes) + params.batch.forEach((batchItem: any) => { + if (batchItem.dataZoomId?.includes('y')) { + zoomSelectionEvent.yAxis = { + startValue: batchItem.startValue, + endValue: batchItem.endValue, + }; + } else { + zoomSelectionEvent.xAxis = { + startValue: batchItem.startValue, + endValue: batchItem.endValue, + }; + } + }); + callback(zoomSelectionEvent); + } + }; + + // Store the wrapper function so we can remove it later + activeHandlers.current.set(handlerKey, zoomHandler); + echartsInstance.on('datazoom', zoomHandler); + break; + } + default: { + activeHandlers.current.set(handlerKey, callback); + echartsInstance.on(action, callback); + } } - default: { - echartsInstance.on(action, callback); + }, + [echartsInstance], + ); + + // Clean up all handlers when the echartsInstance changes + useEffect(() => { + return () => { + if (echartsInstance) { + // Remove all registered handlers + activeHandlers.current.forEach((handler, key) => { + const [action] = key.split('-'); + if (action === 'zoomselect') { + echartsInstance.off('datazoom', handler); + } else { + echartsInstance.off(action, handler); + } + }); + activeHandlers.current.clear(); } - } - }; + }; + }, [echartsInstance]); const setEchartOptions = useMemo( () => @@ -233,7 +310,7 @@ export function useEchart(container: HTMLDivElement | null): EChartsInstance { * */ echartsInstance?.setOption(options, true); }, 50), - [], + [echartsInstance], ); const addSeries: EChartsInstance['addSeries'] = useCallback( From dee7ded16c312e4723d64f7cab0f06ad197eb796 Mon Sep 17 00:00:00 2001 From: Terrence Keane Date: Mon, 16 Dec 2024 14:36:16 -0500 Subject: [PATCH 11/21] Re-org --- charts/core/src/Echarts/useEchart.ts | 239 ++++++++++++++------------- 1 file changed, 121 insertions(+), 118 deletions(-) diff --git a/charts/core/src/Echarts/useEchart.ts b/charts/core/src/Echarts/useEchart.ts index 5ba678b538..e883a7ce95 100644 --- a/charts/core/src/Echarts/useEchart.ts +++ b/charts/core/src/Echarts/useEchart.ts @@ -85,6 +85,55 @@ export function useEchart(container: HTMLDivElement | null): EChartsInstance { // Keep track of active handlers const activeHandlers = useRef(new Map()); + const setEchartOptions = useMemo( + () => + debounce((options: Partial) => { + /** + * The second argument is `true` to merge the new options with the existing ones. + * This is needed to ensure that series get removed properly. + * See issue: https://github.com/apache/echarts/issues/6202 + * */ + echartsInstance?.setOption(options, true); + }, 50), + [echartsInstance], + ); + + const addSeries: EChartsInstance['addSeries'] = useCallback( + data => { + setOptions(currentOptions => { + const updatedOptions = updateUtils.addSeries(currentOptions, data); + setEchartOptions(updatedOptions); + return updatedOptions; + }); + }, + [setEchartOptions], + ); + + const removeSeries: EChartsInstance['removeSeries'] = useCallback( + name => { + setOptions(currentOptions => { + const updatedOptions = updateUtils.removeSeries(currentOptions, name); + setEchartOptions(updatedOptions); + return updatedOptions; + }); + }, + [setEchartOptions], + ); + + const updateOptions: EChartsInstance['updateOptions'] = useCallback( + options => { + setOptions(currentOptions => { + const updatedOptions = updateUtils.updateOptions( + currentOptions, + options, + ); + setEchartOptions(updatedOptions); + return updatedOptions; + }); + }, + [setEchartOptions], + ); + // ECharts does not automatically resize when the window resizes. const resizeHandler = () => { if (echartsInstance) { @@ -92,59 +141,18 @@ export function useEchart(container: HTMLDivElement | null): EChartsInstance { } }; - useEffect(() => { - (async function setupChart() { - try { - setError(null); - - await initializeEcharts(); - - if (container) { - const newChart = echartsCore.init(container); - newChart.setOption(options); - window.addEventListener('resize', resizeHandler); - setEchartsInstance(newChart); - setReady(true); - } - } catch (err) { - setReady(false); - console.error(err); - setError( - err instanceof Error - ? err - : new Error('Failed to initialize ECharts'), - ); - } - })(); - - // Cleanup function - return () => { - if (echartsInstance) { - window.removeEventListener('resize', resizeHandler); - echartsInstance.dispose(); - } - }; - }, [container]); - - useEffect(() => { - setOptions(currentOptions => { - const updatedOptions = { - ...currentOptions, - color: chartSeriesColors[theme], - }; - setEchartOptions(updatedOptions); - return updatedOptions; - }); - }, [theme]); - - const addToGroup: EChartsInstance['addToGroup'] = groupId => { - echartsInstance.group = groupId; - echartsCore.connect(groupId); - }; + const addToGroup: EChartsInstance['addToGroup'] = useCallback( + groupId => { + echartsInstance.group = groupId; + echartsCore.connect(groupId); + }, + [echartsCore, echartsInstance], + ); - const removeFromGroup: EChartsInstance['removeFromGroup'] = () => { - echartsInstance.group = null; - }; + const removeFromGroup: EChartsInstance['removeFromGroup'] = + useCallback(() => { + echartsInstance.group = null; + }, [echartsInstance]); const clearDataZoom = useCallback( (params: any) => { @@ -165,21 +173,20 @@ export function useEchart(container: HTMLDivElement | null): EChartsInstance { [echartsInstance], ); - const setupZoomSelect: EChartsInstance['setupZoomSelect'] = async ({ - xAxis, - yAxis, - }) => { - function enableZoom() { - echartsInstance.dispatchAction({ - type: 'takeGlobalCursor', - key: 'dataZoomSelect', - dataZoomSelectActive: xAxis || yAxis, - }); - // This will trigger a render so we need to remove the handler to prevent a loop - echartsInstance.off('rendered', enableZoom); - } + const setupZoomSelect: EChartsInstance['setupZoomSelect'] = useCallback( + async ({ xAxis, yAxis }) => { + if (!echartsInstance) return; + + function enableZoom() { + echartsInstance.dispatchAction({ + type: 'takeGlobalCursor', + key: 'dataZoomSelect', + dataZoomSelectActive: xAxis || yAxis, + }); + // This will trigger a render so we need to remove the handler to prevent a loop + echartsInstance.off('rendered', enableZoom); + } - if (echartsInstance) { // `0` index enables zoom on that index, `'none'` disables zoom on that index let xAxisIndex: number | string = xAxis ? 0 : 'none'; let yAxisIndex: number | string = yAxis ? 0 : 'none'; @@ -198,8 +205,9 @@ export function useEchart(container: HTMLDivElement | null): EChartsInstance { echartsInstance.on('rendered', enableZoom); echartsInstance.off('dataZoom', clearDataZoom); // prevent adding dupes echartsInstance.on('dataZoom', clearDataZoom); - } - }; + }, + [echartsInstance, updateOptions], + ); const off: EChartsInstance['off'] = useCallback( (action, callback) => { @@ -222,7 +230,7 @@ export function useEchart(container: HTMLDivElement | null): EChartsInstance { ); const on: EChartsInstance['on'] = useCallback( - (action, callback) => { + async (action, callback) => { if (!echartsInstance) return; // Create a unique key for this handler @@ -282,6 +290,50 @@ export function useEchart(container: HTMLDivElement | null): EChartsInstance { [echartsInstance], ); + useEffect(() => { + (async function setupChart() { + try { + setError(null); + + await initializeEcharts(); + + if (container) { + const newChart = echartsCore.init(container); + newChart.setOption(options); + window.addEventListener('resize', resizeHandler); + setEchartsInstance(newChart); + setReady(true); + } + } catch (err) { + setReady(false); + console.error(err); + setError( + err instanceof Error + ? err + : new Error('Failed to initialize ECharts'), + ); + } + })(); + + return () => { + if (echartsInstance) { + window.removeEventListener('resize', resizeHandler); + echartsInstance.dispose(); + } + }; + }, [container]); + + useEffect(() => { + setOptions(currentOptions => { + const updatedOptions = { + ...currentOptions, + color: chartSeriesColors[theme], + }; + setEchartOptions(updatedOptions); + return updatedOptions; + }); + }, [theme]); + // Clean up all handlers when the echartsInstance changes useEffect(() => { return () => { @@ -300,55 +352,6 @@ export function useEchart(container: HTMLDivElement | null): EChartsInstance { }; }, [echartsInstance]); - const setEchartOptions = useMemo( - () => - debounce((options: Partial) => { - /** - * The second argument is `true` to merge the new options with the existing ones. - * This is needed to ensure that series get removed properly. - * See issue: https://github.com/apache/echarts/issues/6202 - * */ - echartsInstance?.setOption(options, true); - }, 50), - [echartsInstance], - ); - - const addSeries: EChartsInstance['addSeries'] = useCallback( - data => { - setOptions(currentOptions => { - const updatedOptions = updateUtils.addSeries(currentOptions, data); - setEchartOptions(updatedOptions); - return updatedOptions; - }); - }, - [setEchartOptions], - ); - - const removeSeries: EChartsInstance['removeSeries'] = useCallback( - name => { - setOptions(currentOptions => { - const updatedOptions = updateUtils.removeSeries(currentOptions, name); - setEchartOptions(updatedOptions); - return updatedOptions; - }); - }, - [setEchartOptions], - ); - - const updateOptions: EChartsInstance['updateOptions'] = useCallback( - options => { - setOptions(currentOptions => { - const updatedOptions = updateUtils.updateOptions( - currentOptions, - options, - ); - setEchartOptions(updatedOptions); - return updatedOptions; - }); - }, - [setEchartOptions], - ); - return { _echartsInstance: echartsInstance, ready, From 4e7141bb6026c721aa41d8d7be1f1df6f445d27c Mon Sep 17 00:00:00 2001 From: Terrence Keane Date: Mon, 16 Dec 2024 16:17:24 -0500 Subject: [PATCH 12/21] Add instance check --- charts/core/src/Echarts/useEchart.ts | 80 +++++++++++++++------------- 1 file changed, 43 insertions(+), 37 deletions(-) diff --git a/charts/core/src/Echarts/useEchart.ts b/charts/core/src/Echarts/useEchart.ts index e883a7ce95..7a3f5c7f20 100644 --- a/charts/core/src/Echarts/useEchart.ts +++ b/charts/core/src/Echarts/useEchart.ts @@ -69,8 +69,8 @@ async function initializeEcharts() { } /** - * Wrapper around the ECharts library. Instantiates and returns an ECharts instance. - * Provides helper methods to hide ECharts specific logic and provide a clean API + * Wrapper around the ECharts library. Instantiates an ECharts instance. + * Provides helper methods to hide ECharts specific logic and give a cleaner API * for interacting with a chart. */ export function useEchart(container: HTMLDivElement | null): EChartsInstance { @@ -85,16 +85,28 @@ export function useEchart(container: HTMLDivElement | null): EChartsInstance { // Keep track of active handlers const activeHandlers = useRef(new Map()); + const withInstanceCheck = void>(fn: T) => { + return (...args: Parameters) => { + if (!echartsInstance) { + return; + } + fn(...args); + }; + }; + const setEchartOptions = useMemo( () => - debounce((options: Partial) => { - /** - * The second argument is `true` to merge the new options with the existing ones. - * This is needed to ensure that series get removed properly. - * See issue: https://github.com/apache/echarts/issues/6202 - * */ - echartsInstance?.setOption(options, true); - }, 50), + debounce( + withInstanceCheck((options: Partial) => { + /** + * The second argument is `true` to merge the new options with the existing ones. + * This is needed to ensure that series get removed properly. + * See issue: https://github.com/apache/echarts/issues/6202 + * */ + echartsInstance?.setOption(options, true); + }), + 50, + ), [echartsInstance], ); @@ -135,49 +147,47 @@ export function useEchart(container: HTMLDivElement | null): EChartsInstance { ); // ECharts does not automatically resize when the window resizes. - const resizeHandler = () => { - if (echartsInstance) { - echartsInstance.resize(); - } - }; + const resizeHandler = withInstanceCheck(() => { + echartsInstance.resize(); + }); const addToGroup: EChartsInstance['addToGroup'] = useCallback( - groupId => { + withInstanceCheck(groupId => { echartsInstance.group = groupId; echartsCore.connect(groupId); - }, + }), [echartsCore, echartsInstance], ); - const removeFromGroup: EChartsInstance['removeFromGroup'] = - useCallback(() => { + const removeFromGroup: EChartsInstance['removeFromGroup'] = useCallback( + withInstanceCheck(() => { echartsInstance.group = null; - }, [echartsInstance]); + }), + [echartsInstance], + ); const clearDataZoom = useCallback( - (params: any) => { + withInstanceCheck((params: any) => { /** * If start is not 0% or end is not 100%, the 'dataZoom' event was triggered by a zoom. * We override the zoom to prevent it from actually zooming. */ const isZoomed = params?.start !== 0 || params?.end !== 100; - if (echartsInstance && isZoomed) { + if (isZoomed) { echartsInstance.dispatchAction({ type: 'dataZoom', start: 0, // percentage of starting position end: 100, // percentage of ending position }); } - }, + }), [echartsInstance], ); const setupZoomSelect: EChartsInstance['setupZoomSelect'] = useCallback( - async ({ xAxis, yAxis }) => { - if (!echartsInstance) return; - - function enableZoom() { + withInstanceCheck(({ xAxis, yAxis }) => { + const enableZoom = withInstanceCheck(() => { echartsInstance.dispatchAction({ type: 'takeGlobalCursor', key: 'dataZoomSelect', @@ -185,7 +195,7 @@ export function useEchart(container: HTMLDivElement | null): EChartsInstance { }); // This will trigger a render so we need to remove the handler to prevent a loop echartsInstance.off('rendered', enableZoom); - } + }); // `0` index enables zoom on that index, `'none'` disables zoom on that index let xAxisIndex: number | string = xAxis ? 0 : 'none'; @@ -205,14 +215,12 @@ export function useEchart(container: HTMLDivElement | null): EChartsInstance { echartsInstance.on('rendered', enableZoom); echartsInstance.off('dataZoom', clearDataZoom); // prevent adding dupes echartsInstance.on('dataZoom', clearDataZoom); - }, + }), [echartsInstance, updateOptions], ); const off: EChartsInstance['off'] = useCallback( - (action, callback) => { - if (!echartsInstance) return; - + withInstanceCheck((action, callback) => { switch (action) { case 'zoomselect': { echartsInstance.off('datazoom', callback); @@ -225,14 +233,12 @@ export function useEchart(container: HTMLDivElement | null): EChartsInstance { activeHandlers.current.delete(`${action}-${callback.toString()}`); } } - }, + }), [echartsInstance], ); const on: EChartsInstance['on'] = useCallback( - async (action, callback) => { - if (!echartsInstance) return; - + withInstanceCheck((action, callback) => { // Create a unique key for this handler const handlerKey = `${action}-${callback.toString()}`; @@ -286,7 +292,7 @@ export function useEchart(container: HTMLDivElement | null): EChartsInstance { echartsInstance.on(action, callback); } } - }, + }), [echartsInstance], ); From 03a651fd061481ab14c4b0a4e664b0248831bfe1 Mon Sep 17 00:00:00 2001 From: Terrence Keane Date: Tue, 17 Dec 2024 16:09:11 -0500 Subject: [PATCH 13/21] Share ChartInstance --- charts/core/src/Chart/Chart.tsx | 17 +++-------------- charts/core/src/Chart/hooks/useChart.ts | 11 ++++------- charts/core/src/Chart/hooks/useChart.types.ts | 6 ++++++ charts/core/src/ChartContext/ChartContext.tsx | 16 ++-------------- .../core/src/ChartContext/ChartContext.types.ts | 6 ++---- charts/core/src/Echarts/useEchart.ts | 1 + charts/core/src/Grid/Grid.tsx | 10 ++++++---- charts/core/src/Line/Line.tsx | 10 ++++++---- charts/core/src/Tooltip/Tooltip.tsx | 12 +++++++----- charts/core/src/XAxis/XAxis.tsx | 12 +++++++----- charts/core/src/YAxis/YAxis.tsx | 10 ++++++---- 11 files changed, 50 insertions(+), 61 deletions(-) diff --git a/charts/core/src/Chart/Chart.tsx b/charts/core/src/Chart/Chart.tsx index 65d218063f..7b352d7962 100644 --- a/charts/core/src/Chart/Chart.tsx +++ b/charts/core/src/Chart/Chart.tsx @@ -29,13 +29,7 @@ export function Chart({ ...rest }: ChartProps) { const { theme } = useDarkMode(darkModeProp); - const { - chartOptions, - updateChartOptions, - addChartSeries, - removeChartSeries, - chartRef, - } = useChart({ + const chart = useChart({ theme, onChartReady, zoomSelect, @@ -45,12 +39,7 @@ export function Chart({ return ( - +
{/** @@ -62,7 +51,7 @@ export function Chart({ {children}
void; } + +export interface ChartInstance extends EChartsInstance { + ref: MutableRefObject; +} diff --git a/charts/core/src/ChartContext/ChartContext.tsx b/charts/core/src/ChartContext/ChartContext.tsx index f8def77f25..fdbf2f0411 100644 --- a/charts/core/src/ChartContext/ChartContext.tsx +++ b/charts/core/src/ChartContext/ChartContext.tsx @@ -8,22 +8,10 @@ export const ChartContext = createContext( export const ChartProvider = ({ children, - chartOptions, - updateChartOptions, - addChartSeries, - removeChartSeries, + chart, }: PropsWithChildren) => { return ( - - {children} - + {children} ); }; diff --git a/charts/core/src/ChartContext/ChartContext.types.ts b/charts/core/src/ChartContext/ChartContext.types.ts index 0176f6218a..7c6a09edb6 100644 --- a/charts/core/src/ChartContext/ChartContext.types.ts +++ b/charts/core/src/ChartContext/ChartContext.types.ts @@ -1,8 +1,6 @@ import { ChartOptions, SeriesOption } from '../Chart/Chart.types'; +import { ChartInstance } from '../Chart/hooks/useChart.types'; export interface ChartContextType { - chartOptions: any; - updateChartOptions: (newOptions: Partial) => void; - addChartSeries: (series: SeriesOption) => void; - removeChartSeries: (name: string) => void; + chart: ChartInstance; } diff --git a/charts/core/src/Echarts/useEchart.ts b/charts/core/src/Echarts/useEchart.ts index 7a3f5c7f20..f4b5d95218 100644 --- a/charts/core/src/Echarts/useEchart.ts +++ b/charts/core/src/Echarts/useEchart.ts @@ -88,6 +88,7 @@ export function useEchart(container: HTMLDivElement | null): EChartsInstance { const withInstanceCheck = void>(fn: T) => { return (...args: Parameters) => { if (!echartsInstance) { + console.error('Echart instance not initialized'); return; } fn(...args); diff --git a/charts/core/src/Grid/Grid.tsx b/charts/core/src/Grid/Grid.tsx index 4dfbcd8de1..1e66e7ffbc 100644 --- a/charts/core/src/Grid/Grid.tsx +++ b/charts/core/src/Grid/Grid.tsx @@ -15,10 +15,12 @@ const unsetGridOptions = { }; export function Grid({ horizontal = true, vertical = true }: GridProps) { - const { updateChartOptions } = useChartContext(); + const { chart } = useChartContext(); const { theme } = useDarkMode(); useEffect(() => { + if (!chart.ready) return; + const updatedOptions: Partial = {}; const getUpdatedLineOptions = (show: boolean) => ({ splitLine: { @@ -31,18 +33,18 @@ export function Grid({ horizontal = true, vertical = true }: GridProps) { }); updatedOptions.xAxis = getUpdatedLineOptions(!!vertical); updatedOptions.yAxis = getUpdatedLineOptions(!!horizontal); - updateChartOptions(updatedOptions); + chart.updateOptions(updatedOptions); return () => { /** * Hides the grid lines when the component is unmounted. */ - updateChartOptions({ + chart.updateOptions({ xAxis: unsetGridOptions, yAxis: unsetGridOptions, }); }; - }, [horizontal, vertical, theme]); + }, [chart.ready, horizontal, vertical, theme]); return null; } diff --git a/charts/core/src/Line/Line.tsx b/charts/core/src/Line/Line.tsx index 91e9d62702..835e9f8acb 100644 --- a/charts/core/src/Line/Line.tsx +++ b/charts/core/src/Line/Line.tsx @@ -6,10 +6,12 @@ import { defaultLineOptions } from './config'; import { LineProps } from './Line.types'; export function Line({ name, data }: LineProps) { - const { addChartSeries, removeChartSeries } = useChartContext(); + const { chart } = useChartContext(); useEffect(() => { - addChartSeries({ + if (!chart.ready) return; + + chart.addSeries({ ...defaultLineOptions, name, data, @@ -20,9 +22,9 @@ export function Line({ name, data }: LineProps) { * Remove the series when the component unmounts to make sure the series * is removed when a `Line` is hidden. */ - removeChartSeries(name); + chart.removeSeries(name); }; - }, [name, data]); + }, [chart.ready, name, data]); return null; } diff --git a/charts/core/src/Tooltip/Tooltip.tsx b/charts/core/src/Tooltip/Tooltip.tsx index 5e2e9df735..28e80ad6cc 100644 --- a/charts/core/src/Tooltip/Tooltip.tsx +++ b/charts/core/src/Tooltip/Tooltip.tsx @@ -21,11 +21,13 @@ export function Tooltip({ sortKey = SortKey.Value, valueFormatter, }: TooltipProps) { - const { updateChartOptions } = useChartContext(); + const { chart } = useChartContext(); const { theme } = useDarkMode(); useEffect(() => { - updateChartOptions({ + if (!chart.ready) return; + + chart.updateOptions({ tooltip: { show: true, backgroundColor: @@ -39,7 +41,7 @@ export function Tooltip({ enterable: false, hideDelay: 0, valueFormatter: valueFormatter - ? value => { + ? (value: any) => { if (typeof value === 'number' || typeof value === 'string') { return valueFormatter(value); } @@ -63,13 +65,13 @@ export function Tooltip({ }); return () => { - updateChartOptions({ + chart.updateOptions({ tooltip: { show: false, }, }); }; - }, [theme, sortDirection, sortKey, valueFormatter, updateChartOptions]); + }, [chart.ready, theme, sortDirection, sortKey, valueFormatter]); return null; } diff --git a/charts/core/src/XAxis/XAxis.tsx b/charts/core/src/XAxis/XAxis.tsx index 8c3f586e20..a8d3f798a5 100644 --- a/charts/core/src/XAxis/XAxis.tsx +++ b/charts/core/src/XAxis/XAxis.tsx @@ -98,21 +98,23 @@ const unsetAxisOptions = { * */ export function XAxis({ type, label, formatter }: XAxisProps) { - const { updateChartOptions } = useChartContext(); + const { chart } = useChartContext(); const { theme } = useDarkMode(); useEffect(() => { - updateChartOptions(getOptions({ type, label, formatter, theme })); + if (!chart.ready) return; + + chart.updateOptions(getOptions({ type, label, formatter, theme })); return () => { /** * Hides the axis when the component is unmounted. - */ - updateChartOptions({ + */ chart.updateOptions; + chart.updateOptions({ xAxis: unsetAxisOptions, }); }; - }, [type, label, formatter, theme, updateChartOptions]); + }, [type, label, formatter, theme, chart.ready]); return null; } diff --git a/charts/core/src/YAxis/YAxis.tsx b/charts/core/src/YAxis/YAxis.tsx index e94334ec89..2eb2e089ea 100644 --- a/charts/core/src/YAxis/YAxis.tsx +++ b/charts/core/src/YAxis/YAxis.tsx @@ -97,21 +97,23 @@ const unsetAxisOptions = { * */ export function YAxis({ type, label, formatter }: YAxisProps) { - const { updateChartOptions } = useChartContext(); + const { chart } = useChartContext(); const { theme } = useDarkMode(); useEffect(() => { - updateChartOptions(getOptions({ type, label, formatter, theme })); + if (!chart.ready) return; + + chart.updateOptions(getOptions({ type, label, formatter, theme })); return () => { /** * Hides the axis when the component is unmounted. */ - updateChartOptions({ + chart.updateOptions({ yAxis: unsetAxisOptions, }); }; - }, [type, label, formatter, theme, updateChartOptions]); + }, [type, label, formatter, theme, chart.ready]); return null; } From 1861aa03546f55e22a6b6ae45bcbc8025f216c84 Mon Sep 17 00:00:00 2001 From: Terrence Keane Date: Wed, 18 Dec 2024 15:01:29 -0500 Subject: [PATCH 14/21] Fix imports --- charts/core/src/Chart/hooks/useChart.ts | 4 ++-- charts/core/src/Chart/hooks/useChart.types.ts | 2 +- charts/core/src/ChartContext/ChartContext.types.ts | 1 - charts/core/src/Echarts/index.ts | 6 ++++++ 4 files changed, 9 insertions(+), 4 deletions(-) create mode 100644 charts/core/src/Echarts/index.ts diff --git a/charts/core/src/Chart/hooks/useChart.ts b/charts/core/src/Chart/hooks/useChart.ts index 1dae1760ce..62aee516c3 100644 --- a/charts/core/src/Chart/hooks/useChart.ts +++ b/charts/core/src/Chart/hooks/useChart.ts @@ -1,7 +1,7 @@ import { useEffect, useRef } from 'react'; -import { ChartHookProps, ChartInstance } from './useChart.types'; -import { useEchart } from '../../echarts/useEchart'; +import type { ChartHookProps, ChartInstance } from './useChart.types'; +import { useEchart } from '../../Echarts'; export function useChart({ onChartReady = () => {}, diff --git a/charts/core/src/Chart/hooks/useChart.types.ts b/charts/core/src/Chart/hooks/useChart.types.ts index 28ac200b5b..d5ad0d31c7 100644 --- a/charts/core/src/Chart/hooks/useChart.types.ts +++ b/charts/core/src/Chart/hooks/useChart.types.ts @@ -1,5 +1,5 @@ import { Theme } from '@leafygreen-ui/lib'; -import { EChartsInstance } from '../../echarts/echarts.types'; +import type { EChartsInstance } from '../../Echarts'; import { MutableRefObject } from 'react'; export interface ZoomSelectionEvent { diff --git a/charts/core/src/ChartContext/ChartContext.types.ts b/charts/core/src/ChartContext/ChartContext.types.ts index 7c6a09edb6..946ce5430a 100644 --- a/charts/core/src/ChartContext/ChartContext.types.ts +++ b/charts/core/src/ChartContext/ChartContext.types.ts @@ -1,4 +1,3 @@ -import { ChartOptions, SeriesOption } from '../Chart/Chart.types'; import { ChartInstance } from '../Chart/hooks/useChart.types'; export interface ChartContextType { diff --git a/charts/core/src/Echarts/index.ts b/charts/core/src/Echarts/index.ts new file mode 100644 index 0000000000..3be205da97 --- /dev/null +++ b/charts/core/src/Echarts/index.ts @@ -0,0 +1,6 @@ +export { useEchart } from './useEchart'; +export type { + ZoomSelectionEvent, + setupZoomSelectProps, + EChartsInstance, +} from './echarts.types'; From c99f5a95fab5d901984dedda2a2f4f9941949c11 Mon Sep 17 00:00:00 2001 From: Terrence Keane Date: Wed, 18 Dec 2024 15:56:13 -0500 Subject: [PATCH 15/21] Tests --- charts/core/src/Chart/Chart.spec.tsx | 35 ++- charts/core/src/Chart/hooks/useChart.spec.ts | 162 ++++++------ charts/core/src/Chart/hooks/useChart.ts | 12 +- charts/core/src/Chart/hooks/useChart.types.ts | 2 +- .../Echart.types.ts} | 7 +- charts/core/src/{Echarts => Echart}/index.ts | 2 +- charts/core/src/Echart/useEchart.spec.ts | 231 ++++++++++++++++++ .../core/src/{Echarts => Echart}/useEchart.ts | 13 +- 8 files changed, 368 insertions(+), 96 deletions(-) rename charts/core/src/{Echarts/echarts.types.ts => Echart/Echart.types.ts} (95%) rename charts/core/src/{Echarts => Echart}/index.ts (82%) create mode 100644 charts/core/src/Echart/useEchart.spec.ts rename charts/core/src/{Echarts => Echart}/useEchart.ts (98%) diff --git a/charts/core/src/Chart/Chart.spec.tsx b/charts/core/src/Chart/Chart.spec.tsx index 63212f41e1..e10edcb94b 100644 --- a/charts/core/src/Chart/Chart.spec.tsx +++ b/charts/core/src/Chart/Chart.spec.tsx @@ -2,6 +2,7 @@ import React from 'react'; import { render, screen } from '@testing-library/react'; import { ChartProvider } from '../ChartContext'; +import { useChart } from './hooks'; import { Chart } from './Chart'; @@ -9,13 +10,15 @@ jest.mock('../ChartContext', () => ({ ChartProvider: jest.fn(({ children }) =>
{children}
), })); +const mockChartInstance = { + chartOptions: {}, + updateChartOptions: jest.fn(), + addChartSeries: jest.fn(), + removeChartSeries: jest.fn(), +}; + jest.mock('./hooks', () => ({ - useChart: jest.fn(() => ({ - chartOptions: {}, - updateChartOptions: jest.fn(), - addChartSeries: jest.fn(), - removeChartSeries: jest.fn(), - })), + useChart: jest.fn(() => mockChartInstance), })); /** @@ -24,23 +27,29 @@ jest.mock('./hooks', () => ({ * Chromatic tests for rendering logic. */ describe('lg-charts/core/Chart', () => { - it('renders the echart container', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + test('renders the echart container', () => { render(); expect( screen.getByTestId('lg-charts-core-chart-echart'), ).toBeInTheDocument(); }); - it('passes the correct props to ChartProvider', () => { + test('passes the chart instance to ChartProvider', () => { render(); + + // Verify useChart was called + expect(useChart).toHaveBeenCalled(); + + // Verify ChartProvider was called with the correct chart instance expect(ChartProvider).toHaveBeenCalledWith( expect.objectContaining({ - chartOptions: {}, - updateChartOptions: expect.any(Function), - addChartSeries: expect.any(Function), - removeChartSeries: expect.any(Function), + chart: mockChartInstance, }), - expect.anything(), + expect.any(Object), ); }); }); diff --git a/charts/core/src/Chart/hooks/useChart.spec.ts b/charts/core/src/Chart/hooks/useChart.spec.ts index 2ede70a6c9..77d46cc12f 100644 --- a/charts/core/src/Chart/hooks/useChart.spec.ts +++ b/charts/core/src/Chart/hooks/useChart.spec.ts @@ -1,97 +1,119 @@ import { act } from 'react-dom/test-utils'; - -import { renderHook, waitForState } from '@leafygreen-ui/testing-lib'; - -import { SeriesOption } from '../Chart.types'; - +import { renderHook } from '@leafygreen-ui/testing-lib'; import { useChart } from './useChart'; -jest.mock('echarts/core', () => ({ - init: jest.fn(() => ({ - setOption: jest.fn(), - dispose: jest.fn(), - resize: jest.fn(), +// Mock the useEchart hook +jest.mock('../../Echart', () => ({ + useEchart: jest.fn(() => ({ + ready: false, + addToGroup: jest.fn(), + setupZoomSelect: jest.fn(), on: jest.fn(), })), - use: jest.fn(), })); -jest.mock('echarts/charts', () => ({ - LineChart: jest.fn(), -})); +describe('@lg-echarts/core/hooks/useChart', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); -jest.mock('echarts/components', () => ({ - TitleComponent: jest.fn(), - TooltipComponent: jest.fn(), - GridComponent: jest.fn(), - LegendComponent: jest.fn(), - ToolboxComponent: jest.fn(), -})); + test('call `onChartReady` when EchartInstance is ready', async () => { + const onChartReady = jest.fn(); + const { useEchart } = require('../../Echart'); -jest.mock('echarts/renderers', () => ({ - CanvasRenderer: jest.fn(), -})); + // Initially not ready + (useEchart as jest.Mock).mockReturnValue({ + ready: false, + addToGroup: jest.fn(), + setupZoomSelect: jest.fn(), + on: jest.fn(), + }); -describe('@lg-echarts/core/hooks/useChart', () => { - test('should return an object with the correct properties', () => { - const { result } = renderHook(() => useChart({ theme: 'dark' })); - expect(result.current).toEqual( - expect.objectContaining({ - chartRef: expect.any(Object), - chartInstanceRef: expect.any(Object), - chartOptions: expect.any(Object), - addChartSeries: expect.any(Function), - removeChartSeries: expect.any(Function), - updateChartOptions: expect.any(Function), - }), + const { rerender } = renderHook(() => + useChart({ theme: 'dark', onChartReady }), ); + + expect(onChartReady).not.toHaveBeenCalled(); + + // Update to ready state + (useEchart as jest.Mock).mockReturnValue({ + ready: true, + addToGroup: jest.fn(), + setupZoomSelect: jest.fn(), + on: jest.fn(), + }); + + rerender(); + + expect(onChartReady).toHaveBeenCalledTimes(1); }); - test('should properly update state on addChartSeries call', async () => { - const { result } = renderHook(() => useChart({ theme: 'dark' })); - const series: SeriesOption = { - name: 'test', - data: [1, 2, 3], - }; - act(() => { - result.current.addChartSeries(series); + test('should correctly configure zoom select', async () => { + const { useEchart } = require('../../Echart'); + const setupZoomSelect = jest.fn(); + + (useEchart as jest.Mock).mockReturnValue({ + ready: true, + addToGroup: jest.fn(), + setupZoomSelect, + on: jest.fn(), }); - await waitForState(() => { - expect(result.current.chartOptions.series).toEqual([series]); + + const zoomSelect = { + xAxis: true, + yAxis: false, + }; + + renderHook(() => useChart({ theme: 'dark', zoomSelect })); + + expect(setupZoomSelect).toHaveBeenCalledWith({ + xAxis: true, + yAxis: false, }); }); - test('should properly update state on removeChartSeries call', async () => { - const { result } = renderHook(() => useChart({ theme: 'dark' })); - const series: SeriesOption = { - name: 'test', - data: [1, 2, 3], - }; - act(() => { - result.current.addChartSeries(series); + test('should call `onZoomSelect` on the zoomselect event', async () => { + const onZoomSelect = jest.fn(); + const { useEchart } = require('../../Echart'); + const on = jest.fn(); + + (useEchart as jest.Mock).mockReturnValue({ + ready: true, + addToGroup: jest.fn(), + setupZoomSelect: jest.fn(), + on, }); + + renderHook(() => useChart({ theme: 'dark', onZoomSelect })); + + expect(on).toHaveBeenCalledWith('zoomselect', expect.any(Function)); + + // Simulate zoom select event + const zoomEventResponse = { start: 0, end: 100 }; + const zoomSelectHandler = on.mock.calls[0][1]; act(() => { - result.current.removeChartSeries('test'); - }); - await waitForState(() => { - expect(result.current.chartOptions.series).toEqual([]); + zoomSelectHandler(zoomEventResponse); }); + + expect(onZoomSelect).toHaveBeenCalledWith(zoomEventResponse); }); - test('should properly update state on updateChartOptions call', async () => { - const { result } = renderHook(() => useChart({ theme: 'dark' })); - const options = { - title: { - show: true, - text: 'test', - }, + test('should return the chart instance', () => { + const { useEchart } = require('../../Echart'); + const mockEchartInstance = { + ready: true, + addToGroup: jest.fn(), + setupZoomSelect: jest.fn(), + on: jest.fn(), }; - act(() => { - result.current.updateChartOptions(options); - }); - await waitForState(() => { - expect(result.current.chartOptions.title).toEqual(options.title); + (useEchart as jest.Mock).mockReturnValue(mockEchartInstance); + + const { result } = renderHook(() => useChart({ theme: 'dark' })); + + expect(result.current).toEqual({ + ...mockEchartInstance, + ref: expect.any(Object), }); }); }); diff --git a/charts/core/src/Chart/hooks/useChart.ts b/charts/core/src/Chart/hooks/useChart.ts index 62aee516c3..bde4e98314 100644 --- a/charts/core/src/Chart/hooks/useChart.ts +++ b/charts/core/src/Chart/hooks/useChart.ts @@ -1,16 +1,19 @@ import { useEffect, useRef } from 'react'; import type { ChartHookProps, ChartInstance } from './useChart.types'; -import { useEchart } from '../../Echarts'; +import { useEchart } from '../../Echart'; +import { getDefaultChartOptions } from '../config'; export function useChart({ onChartReady = () => {}, zoomSelect, onZoomSelect, groupId, + theme, }: ChartHookProps): ChartInstance { const chartRef = useRef(null); - const echart = useEchart(chartRef.current); + const initialOptions = getDefaultChartOptions(theme); + const echart = useEchart({ container: chartRef.current, initialOptions }); useEffect(() => { if (echart.ready) { @@ -32,11 +35,6 @@ export function useChart({ xAxis: zoomSelect?.xAxis, yAxis: zoomSelect?.yAxis, }); - if (onZoomSelect) { - echart.on('zoomselect', zoomEventResponse => { - onZoomSelect(zoomEventResponse); - }); - } } }, [echart.ready, zoomSelect]); diff --git a/charts/core/src/Chart/hooks/useChart.types.ts b/charts/core/src/Chart/hooks/useChart.types.ts index d5ad0d31c7..d6ce942946 100644 --- a/charts/core/src/Chart/hooks/useChart.types.ts +++ b/charts/core/src/Chart/hooks/useChart.types.ts @@ -1,5 +1,5 @@ import { Theme } from '@leafygreen-ui/lib'; -import type { EChartsInstance } from '../../Echarts'; +import type { EChartsInstance } from '../../Echart'; import { MutableRefObject } from 'react'; export interface ZoomSelectionEvent { diff --git a/charts/core/src/Echarts/echarts.types.ts b/charts/core/src/Echart/Echart.types.ts similarity index 95% rename from charts/core/src/Echarts/echarts.types.ts rename to charts/core/src/Echart/Echart.types.ts index a07702a4f2..ae69e6a57d 100644 --- a/charts/core/src/Echarts/echarts.types.ts +++ b/charts/core/src/Echart/Echart.types.ts @@ -1,4 +1,4 @@ -import { ChartOptions, SeriesOption } from '../Chart'; +import type { ChartOptions, SeriesOption } from '../Chart'; type EChartsEvents = // Mouse Events @@ -89,3 +89,8 @@ export interface EChartsInstance { setupZoomSelect: (setupZoomSelectProps: setupZoomSelectProps) => void; error: Error | null; } + +export interface EChartHookProps { + container: HTMLDivElement | null; + initialOptions?: Partial; +} diff --git a/charts/core/src/Echarts/index.ts b/charts/core/src/Echart/index.ts similarity index 82% rename from charts/core/src/Echarts/index.ts rename to charts/core/src/Echart/index.ts index 3be205da97..d995352a86 100644 --- a/charts/core/src/Echarts/index.ts +++ b/charts/core/src/Echart/index.ts @@ -3,4 +3,4 @@ export type { ZoomSelectionEvent, setupZoomSelectProps, EChartsInstance, -} from './echarts.types'; +} from './Echart.types'; diff --git a/charts/core/src/Echart/useEchart.spec.ts b/charts/core/src/Echart/useEchart.spec.ts new file mode 100644 index 0000000000..5cb5a5c1ed --- /dev/null +++ b/charts/core/src/Echart/useEchart.spec.ts @@ -0,0 +1,231 @@ +import { act } from 'react-dom/test-utils'; +import { renderHook } from '@leafygreen-ui/testing-lib'; +import { type SeriesOption } from '../Chart'; +import { useEchart } from './useEchart'; + +// Mock echarts instance creation with all required methods +const mockEchartsInstance = { + setOption: jest.fn(), + dispose: jest.fn(), + resize: jest.fn(), + on: jest.fn(), + off: jest.fn(), + dispatchAction: jest.fn(), + group: null, +}; + +// Mock implementations +jest.mock('echarts/core', () => ({ + init: jest.fn(() => mockEchartsInstance), + use: jest.fn(), + connect: jest.fn(), +})); + +jest.mock('echarts/charts', () => ({ + LineChart: jest.fn(), +})); + +jest.mock('echarts/components', () => ({ + TitleComponent: jest.fn(), + TooltipComponent: jest.fn(), + GridComponent: jest.fn(), + LegendComponent: jest.fn(), + ToolboxComponent: jest.fn(), + DataZoomComponent: jest.fn(), + DataZoomInsideComponent: jest.fn(), +})); + +jest.mock('echarts/renderers', () => ({ + CanvasRenderer: jest.fn(), +})); + +// Helper function to wait for hook initialization and state updates +const setupHook = async () => { + const mockContainer = document.createElement('div'); + let result: ReturnType< + typeof renderHook, void> + >['result']; + + await act(async () => { + const hookResult = renderHook(() => + useEchart({ container: mockContainer }), + ); + result = hookResult.result; + // Wait for all state updates to complete + await Promise.resolve(); + }); + + return { result: result! }; +}; + +describe('@lg-echarts/core/hooks/useChart', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + test('should return a chart instance with the correct properties', async () => { + const { result } = await setupHook(); + + expect(result.current).toHaveProperty('_echartsInstance'); + expect(result.current).toHaveProperty('ready'); + expect(result.current).toHaveProperty('options'); + expect(result.current).toHaveProperty('updateOptions'); + expect(result.current).toHaveProperty('on'); + expect(result.current).toHaveProperty('off'); + expect(result.current).toHaveProperty('addSeries'); + expect(result.current).toHaveProperty('removeSeries'); + expect(result.current).toHaveProperty('addToGroup'); + expect(result.current).toHaveProperty('removeFromGroup'); + expect(result.current).toHaveProperty('setupZoomSelect'); + expect(result.current).toHaveProperty('error'); + }); + + test('should properly update state on addSeries call', async () => { + const { result } = await setupHook(); + + const newSeries: SeriesOption = { + name: 'test-series', + data: [[1, 2]], + type: 'line', + }; + + await act(async () => { + result.current.addSeries(newSeries); + await Promise.resolve(); + }); + + expect(result.current.options.series).toContainEqual( + expect.objectContaining(newSeries), + ); + }); + + test('should properly update state on removeSeries call', async () => { + const { result } = await setupHook(); + + const series: SeriesOption = { + name: 'test-series', + data: [[1, 2]], + type: 'line', + }; + + await act(async () => { + result.current.addSeries(series); + result.current.removeSeries('test-series'); + await Promise.resolve(); + }); + + expect(result.current.options.series).not.toContainEqual( + expect.objectContaining(series), + ); + }); + + test('should properly update state on updateChartOptions call', async () => { + const { result } = await setupHook(); + + const newOptions = { + grid: { top: 100 }, + }; + + await act(async () => { + result.current.updateOptions(newOptions); + await Promise.resolve(); + }); + + expect(result.current.options.grid).toEqual( + expect.objectContaining(newOptions.grid), + ); + }); + + test('should add event handler on call of `on`', async () => { + const { result } = await setupHook(); + + const mockCallback = jest.fn(); + + await act(async () => { + result.current.on('click', mockCallback); + await Promise.resolve(); + }); + + expect(result.current._echartsInstance.on).toHaveBeenCalledWith( + 'click', + mockCallback, + ); + }); + + test('should remove event handler on call of `off`', async () => { + const { result } = await setupHook(); + + const mockCallback = jest.fn(); + + await act(async () => { + result.current.off('click', mockCallback); + await Promise.resolve(); + }); + + expect(result.current._echartsInstance.off).toHaveBeenCalledWith( + 'click', + mockCallback, + ); + }); + + test('should add chart to group and connect group on call to `addToGroup`', async () => { + const { result } = await setupHook(); + + await act(async () => { + result.current.addToGroup('test-group'); + await Promise.resolve(); + }); + + expect(result.current._echartsInstance.group).toBe('test-group'); + }); + + test('should remove chart from group on call to `removeFromGroup`', async () => { + const { result } = await setupHook(); + + await act(async () => { + result.current.addToGroup('test-group'); + result.current.removeFromGroup(); + await Promise.resolve(); + }); + + expect(result.current._echartsInstance.group).toBeNull(); + }); + + test('should setup zoom select on call to `setupZoomSelect`', async () => { + const { result } = await setupHook(); + + // Store the 'rendered' event handler when it's registered + let renderedHandler: () => void; + mockEchartsInstance.on.mockImplementation( + (event: string, handler: () => void) => { + if (event === 'rendered') { + renderedHandler = handler; + } + }, + ); + + await act(async () => { + result.current.setupZoomSelect({ xAxis: true, yAxis: false }); + await Promise.resolve(); + }); + + // Trigger the 'rendered' event handler + await act(async () => { + renderedHandler(); + await Promise.resolve(); + }); + + expect(result.current._echartsInstance.dispatchAction).toHaveBeenCalledWith( + { + type: 'takeGlobalCursor', + key: 'dataZoomSelect', + dataZoomSelectActive: true, + }, + ); + }); + + test('should set `ready` to true when chart instance is available', async () => { + const { result } = await setupHook(); + expect(result.current.ready).toBe(true); + }); +}); diff --git a/charts/core/src/Echarts/useEchart.ts b/charts/core/src/Echart/useEchart.ts similarity index 98% rename from charts/core/src/Echarts/useEchart.ts rename to charts/core/src/Echart/useEchart.ts index f4b5d95218..327a9d1c7b 100644 --- a/charts/core/src/Echarts/useEchart.ts +++ b/charts/core/src/Echart/useEchart.ts @@ -7,7 +7,11 @@ import { getDefaultChartOptions } from '../Chart/config'; import { ChartOptions } from '../Chart'; import * as updateUtils from '../Chart/hooks/updateUtils'; import { chartSeriesColors } from '../Chart/chartSeriesColors'; -import { EChartsInstance, ZoomSelectionEvent } from './echarts.types'; +import { + EChartHookProps, + EChartsInstance, + ZoomSelectionEvent, +} from './Echart.types'; // Singleton promise for initialization to prevent duplication let initPromise: Promise | null = null; @@ -73,13 +77,16 @@ async function initializeEcharts() { * Provides helper methods to hide ECharts specific logic and give a cleaner API * for interacting with a chart. */ -export function useEchart(container: HTMLDivElement | null): EChartsInstance { +export function useEchart({ + container, + initialOptions, +}: EChartHookProps): EChartsInstance { const [echartsInstance, setEchartsInstance] = useState(null); // has to be any since no types exist until import const [error, setError] = useState(null); const [ready, setReady] = useState(false); const { theme } = useDarkMode(); const [options, setOptions] = useState( - getDefaultChartOptions(theme), + initialOptions || {}, ); // Keep track of active handlers From 3a764f7656ab926bb0b18e932eea984c5c8e5452 Mon Sep 17 00:00:00 2001 From: Terrence Keane Date: Wed, 18 Dec 2024 15:59:03 -0500 Subject: [PATCH 16/21] Lint --- charts/core/src/Chart/Chart.spec.tsx | 2 +- charts/core/src/Chart/hooks/useChart.spec.ts | 2 ++ charts/core/src/Chart/hooks/useChart.ts | 3 ++- charts/core/src/Chart/hooks/useChart.types.ts | 4 +++- charts/core/src/Echart/Echart.types.ts | 8 ++++---- charts/core/src/Echart/index.ts | 6 +++--- charts/core/src/Echart/useEchart.spec.ts | 3 +++ charts/core/src/Echart/useEchart.ts | 12 +++++++++--- 8 files changed, 27 insertions(+), 13 deletions(-) diff --git a/charts/core/src/Chart/Chart.spec.tsx b/charts/core/src/Chart/Chart.spec.tsx index e10edcb94b..df500f2ead 100644 --- a/charts/core/src/Chart/Chart.spec.tsx +++ b/charts/core/src/Chart/Chart.spec.tsx @@ -2,9 +2,9 @@ import React from 'react'; import { render, screen } from '@testing-library/react'; import { ChartProvider } from '../ChartContext'; -import { useChart } from './hooks'; import { Chart } from './Chart'; +import { useChart } from './hooks'; jest.mock('../ChartContext', () => ({ ChartProvider: jest.fn(({ children }) =>
{children}
), diff --git a/charts/core/src/Chart/hooks/useChart.spec.ts b/charts/core/src/Chart/hooks/useChart.spec.ts index 77d46cc12f..490f74c41b 100644 --- a/charts/core/src/Chart/hooks/useChart.spec.ts +++ b/charts/core/src/Chart/hooks/useChart.spec.ts @@ -1,5 +1,7 @@ import { act } from 'react-dom/test-utils'; + import { renderHook } from '@leafygreen-ui/testing-lib'; + import { useChart } from './useChart'; // Mock the useEchart hook diff --git a/charts/core/src/Chart/hooks/useChart.ts b/charts/core/src/Chart/hooks/useChart.ts index bde4e98314..2220fc3763 100644 --- a/charts/core/src/Chart/hooks/useChart.ts +++ b/charts/core/src/Chart/hooks/useChart.ts @@ -1,9 +1,10 @@ import { useEffect, useRef } from 'react'; -import type { ChartHookProps, ChartInstance } from './useChart.types'; import { useEchart } from '../../Echart'; import { getDefaultChartOptions } from '../config'; +import type { ChartHookProps, ChartInstance } from './useChart.types'; + export function useChart({ onChartReady = () => {}, zoomSelect, diff --git a/charts/core/src/Chart/hooks/useChart.types.ts b/charts/core/src/Chart/hooks/useChart.types.ts index d6ce942946..d3e3b245fb 100644 --- a/charts/core/src/Chart/hooks/useChart.types.ts +++ b/charts/core/src/Chart/hooks/useChart.types.ts @@ -1,6 +1,8 @@ +import { MutableRefObject } from 'react'; + import { Theme } from '@leafygreen-ui/lib'; + import type { EChartsInstance } from '../../Echart'; -import { MutableRefObject } from 'react'; export interface ZoomSelectionEvent { xAxis?: { startValue: number; endValue: number }; diff --git a/charts/core/src/Echart/Echart.types.ts b/charts/core/src/Echart/Echart.types.ts index ae69e6a57d..6d7e68cdfb 100644 --- a/charts/core/src/Echart/Echart.types.ts +++ b/charts/core/src/Echart/Echart.types.ts @@ -63,17 +63,17 @@ export interface ZoomSelectionEvent { yAxis?: { startValue: number; endValue: number }; } -export interface setupZoomSelectProps { +export interface SetupZoomSelectProps { xAxis?: boolean; yAxis?: boolean; } -type EChartsEventHandlerType = { +interface EChartsEventHandlerType { // Regular events use the original param type (event: EChartsEvents, callback: (params: any) => void): void; // Specific override for 'zoomselect' (event: 'zoomselect', callback: (params: ZoomSelectionEvent) => void): void; -}; +} export interface EChartsInstance { _echartsInstance: any; @@ -86,7 +86,7 @@ export interface EChartsInstance { removeSeries: (name: string) => void; addToGroup: (groupId: string) => void; removeFromGroup: () => void; - setupZoomSelect: (setupZoomSelectProps: setupZoomSelectProps) => void; + setupZoomSelect: (SetupZoomSelectProps: SetupZoomSelectProps) => void; error: Error | null; } diff --git a/charts/core/src/Echart/index.ts b/charts/core/src/Echart/index.ts index d995352a86..3359f8e419 100644 --- a/charts/core/src/Echart/index.ts +++ b/charts/core/src/Echart/index.ts @@ -1,6 +1,6 @@ -export { useEchart } from './useEchart'; export type { - ZoomSelectionEvent, - setupZoomSelectProps, EChartsInstance, + SetupZoomSelectProps, + ZoomSelectionEvent, } from './Echart.types'; +export { useEchart } from './useEchart'; diff --git a/charts/core/src/Echart/useEchart.spec.ts b/charts/core/src/Echart/useEchart.spec.ts index 5cb5a5c1ed..b6b15d88af 100644 --- a/charts/core/src/Echart/useEchart.spec.ts +++ b/charts/core/src/Echart/useEchart.spec.ts @@ -1,6 +1,9 @@ import { act } from 'react-dom/test-utils'; + import { renderHook } from '@leafygreen-ui/testing-lib'; + import { type SeriesOption } from '../Chart'; + import { useEchart } from './useEchart'; // Mock echarts instance creation with all required methods diff --git a/charts/core/src/Echart/useEchart.ts b/charts/core/src/Echart/useEchart.ts index 327a9d1c7b..e83ca158de 100644 --- a/charts/core/src/Echart/useEchart.ts +++ b/charts/core/src/Echart/useEchart.ts @@ -3,10 +3,11 @@ import debounce from 'lodash.debounce'; import { useDarkMode } from '@leafygreen-ui/leafygreen-provider'; -import { getDefaultChartOptions } from '../Chart/config'; import { ChartOptions } from '../Chart'; -import * as updateUtils from '../Chart/hooks/updateUtils'; import { chartSeriesColors } from '../Chart/chartSeriesColors'; +import { getDefaultChartOptions } from '../Chart/config'; +import * as updateUtils from '../Chart/hooks/updateUtils'; + import { EChartHookProps, EChartsInstance, @@ -92,7 +93,9 @@ export function useEchart({ // Keep track of active handlers const activeHandlers = useRef(new Map()); - const withInstanceCheck = void>(fn: T) => { + const withInstanceCheck = ) => void>( + fn: T, + ) => { return (...args: Parameters) => { if (!echartsInstance) { console.error('Echart instance not initialized'); @@ -236,6 +239,7 @@ export function useEchart({ activeHandlers.current.delete(`${action}-${callback.toString()}`); break; } + default: { echartsInstance.off(action, callback); activeHandlers.current.delete(`${action}-${callback.toString()}`); @@ -295,6 +299,7 @@ export function useEchart({ echartsInstance.on('datazoom', zoomHandler); break; } + default: { activeHandlers.current.set(handlerKey, callback); echartsInstance.on(action, callback); @@ -355,6 +360,7 @@ export function useEchart({ // Remove all registered handlers activeHandlers.current.forEach((handler, key) => { const [action] = key.split('-'); + if (action === 'zoomselect') { echartsInstance.off('datazoom', handler); } else { From 876dbe30fb7c097eefe044bab0c6606176820a0c Mon Sep 17 00:00:00 2001 From: Terrence Keane Date: Wed, 18 Dec 2024 16:01:14 -0500 Subject: [PATCH 17/21] changeset --- .changeset/stupid-ways-collect.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/stupid-ways-collect.md diff --git a/.changeset/stupid-ways-collect.md b/.changeset/stupid-ways-collect.md new file mode 100644 index 0000000000..3ecdfbc8ba --- /dev/null +++ b/.changeset/stupid-ways-collect.md @@ -0,0 +1,6 @@ +--- +'@lg-charts/core': patch +--- + +- Adds treeshaking of echarts to minimize bundle size. +- Improves echart initialization logic to better handle race conditions. From 481ea4aa2c2b3e3994ca0e285b7f00b7f5e548c0 Mon Sep 17 00:00:00 2001 From: Terrence Keane Date: Wed, 18 Dec 2024 16:11:37 -0500 Subject: [PATCH 18/21] Add theme prop --- charts/core/src/Chart/hooks/useChart.ts | 6 +++++- charts/core/src/Echart/Echart.types.ts | 2 ++ charts/core/src/Echart/useEchart.spec.ts | 2 +- charts/core/src/Echart/useEchart.ts | 2 +- 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/charts/core/src/Chart/hooks/useChart.ts b/charts/core/src/Chart/hooks/useChart.ts index 2220fc3763..584c7a33f5 100644 --- a/charts/core/src/Chart/hooks/useChart.ts +++ b/charts/core/src/Chart/hooks/useChart.ts @@ -14,7 +14,11 @@ export function useChart({ }: ChartHookProps): ChartInstance { const chartRef = useRef(null); const initialOptions = getDefaultChartOptions(theme); - const echart = useEchart({ container: chartRef.current, initialOptions }); + const echart = useEchart({ + container: chartRef.current, + initialOptions, + theme, + }); useEffect(() => { if (echart.ready) { diff --git a/charts/core/src/Echart/Echart.types.ts b/charts/core/src/Echart/Echart.types.ts index 6d7e68cdfb..94275d992a 100644 --- a/charts/core/src/Echart/Echart.types.ts +++ b/charts/core/src/Echart/Echart.types.ts @@ -1,3 +1,4 @@ +import { DarkModeProps, Theme } from '@leafygreen-ui/lib'; import type { ChartOptions, SeriesOption } from '../Chart'; type EChartsEvents = @@ -93,4 +94,5 @@ export interface EChartsInstance { export interface EChartHookProps { container: HTMLDivElement | null; initialOptions?: Partial; + theme: Theme; } diff --git a/charts/core/src/Echart/useEchart.spec.ts b/charts/core/src/Echart/useEchart.spec.ts index b6b15d88af..08f8254c7d 100644 --- a/charts/core/src/Echart/useEchart.spec.ts +++ b/charts/core/src/Echart/useEchart.spec.ts @@ -51,7 +51,7 @@ const setupHook = async () => { await act(async () => { const hookResult = renderHook(() => - useEchart({ container: mockContainer }), + useEchart({ theme: 'dark', container: mockContainer }), ); result = hookResult.result; // Wait for all state updates to complete diff --git a/charts/core/src/Echart/useEchart.ts b/charts/core/src/Echart/useEchart.ts index e83ca158de..65a54c2bde 100644 --- a/charts/core/src/Echart/useEchart.ts +++ b/charts/core/src/Echart/useEchart.ts @@ -81,11 +81,11 @@ async function initializeEcharts() { export function useEchart({ container, initialOptions, + theme, }: EChartHookProps): EChartsInstance { const [echartsInstance, setEchartsInstance] = useState(null); // has to be any since no types exist until import const [error, setError] = useState(null); const [ready, setReady] = useState(false); - const { theme } = useDarkMode(); const [options, setOptions] = useState( initialOptions || {}, ); From 945b3bc82b81f5a6f983aa2441e37d9f493796c5 Mon Sep 17 00:00:00 2001 From: Terrence Keane Date: Wed, 18 Dec 2024 16:28:43 -0500 Subject: [PATCH 19/21] Lint --- charts/core/src/Echart/Echart.types.ts | 3 ++- charts/core/src/Echart/useEchart.ts | 3 --- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/charts/core/src/Echart/Echart.types.ts b/charts/core/src/Echart/Echart.types.ts index 94275d992a..e57f8f7847 100644 --- a/charts/core/src/Echart/Echart.types.ts +++ b/charts/core/src/Echart/Echart.types.ts @@ -1,4 +1,5 @@ -import { DarkModeProps, Theme } from '@leafygreen-ui/lib'; +import { Theme } from '@leafygreen-ui/lib'; + import type { ChartOptions, SeriesOption } from '../Chart'; type EChartsEvents = diff --git a/charts/core/src/Echart/useEchart.ts b/charts/core/src/Echart/useEchart.ts index 65a54c2bde..2884e3ea09 100644 --- a/charts/core/src/Echart/useEchart.ts +++ b/charts/core/src/Echart/useEchart.ts @@ -1,11 +1,8 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import debounce from 'lodash.debounce'; -import { useDarkMode } from '@leafygreen-ui/leafygreen-provider'; - import { ChartOptions } from '../Chart'; import { chartSeriesColors } from '../Chart/chartSeriesColors'; -import { getDefaultChartOptions } from '../Chart/config'; import * as updateUtils from '../Chart/hooks/updateUtils'; import { From eea4d45fc4b815f0b028478b7f6a7ec890680d65 Mon Sep 17 00:00:00 2001 From: Terrence Keane Date: Wed, 18 Dec 2024 16:31:02 -0500 Subject: [PATCH 20/21] Turn zoom select on both charts in storybook --- charts/core/src/Chart.stories.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/charts/core/src/Chart.stories.tsx b/charts/core/src/Chart.stories.tsx index a23849f7e9..5ee9fb3528 100644 --- a/charts/core/src/Chart.stories.tsx +++ b/charts/core/src/Chart.stories.tsx @@ -498,6 +498,7 @@ export const WithSameGroupIds: StoryObj = { xAxis: zoomSelectXAxis, yAxis: zoomSelectYAxis, }} + onZoomSelect={zoomSelectCallback} > {renderHeader && (
From 68595ff91a38eaef596ee7277209df5178662124 Mon Sep 17 00:00:00 2001 From: Terrence Keane Date: Thu, 26 Dec 2024 13:24:23 -0500 Subject: [PATCH 21/21] CR Feedback --- charts/core/src/Chart/Chart.types.ts | 46 +--- charts/core/src/Chart/hooks/useChart.spec.ts | 12 +- charts/core/src/Chart/hooks/useChart.ts | 19 +- charts/core/src/Chart/hooks/useChart.types.ts | 13 +- charts/core/src/Echart/Echart.types.ts | 154 +++++++----- charts/core/src/Echart/index.ts | 8 +- charts/core/src/Echart/initializeEcharts.ts | 75 ++++++ charts/core/src/Echart/useEchart.spec.ts | 22 +- charts/core/src/Echart/useEchart.ts | 222 +++++++----------- 9 files changed, 306 insertions(+), 265 deletions(-) create mode 100644 charts/core/src/Echart/initializeEcharts.ts diff --git a/charts/core/src/Chart/Chart.types.ts b/charts/core/src/Chart/Chart.types.ts index 34c1feacd7..314e3fea36 100644 --- a/charts/core/src/Chart/Chart.types.ts +++ b/charts/core/src/Chart/Chart.types.ts @@ -1,40 +1,18 @@ import { PropsWithChildren } from 'react'; -import type { XAXisComponentOption, YAXisComponentOption } from 'echarts'; -import type { LineSeriesOption } from 'echarts/charts'; -import type { - DatasetComponentOption, - GridComponentOption, - LegendComponentOption, - TitleComponentOption, - ToolboxComponentOption, - TooltipComponentOption, -} from 'echarts/components'; -import type { ComposeOption } from 'echarts/core'; import { DarkModeProps, type HTMLElementProps } from '@leafygreen-ui/lib'; -import { ZoomSelect, ZoomSelectionEvent } from './hooks/useChart.types'; +import { + EChartOptions, + EChartSeriesOption, + EChartZoomSelectionEvent, +} from '../Echart'; -type RequiredSeriesProps = 'type' | 'name' | 'data'; -export type SeriesOption = Pick & - Partial>; +import { ZoomSelect } from './hooks/useChart.types'; -/** - * TODO: This might need to be improved. `ComposeOption` appears to make most base option - * keys "Arrayable". This is making it difficult to properly test partial options on - * methods like updateUtils > updateOptions(), since something like `options.grid` could be - * an array even if an object. - */ -export type ChartOptions = ComposeOption< - | TooltipComponentOption - | GridComponentOption - | DatasetComponentOption - | TitleComponentOption - | LegendComponentOption - | ToolboxComponentOption - | XAXisComponentOption - | YAXisComponentOption -> & { series?: Array }; +export type SeriesOption = EChartSeriesOption; +export type ChartOptions = EChartOptions; +type ZoomSelectionEvent = EChartZoomSelectionEvent; export type ChartProps = HTMLElementProps<'div'> & DarkModeProps & @@ -59,9 +37,3 @@ export type ChartProps = HTMLElementProps<'div'> & */ groupId?: string; }>; - -export const ChartActionType = { - addChartSeries: 'addChartSeries', - removeChartSeries: 'removeChartSeries', - updateOptions: 'updateOptions', -} as const; diff --git a/charts/core/src/Chart/hooks/useChart.spec.ts b/charts/core/src/Chart/hooks/useChart.spec.ts index 490f74c41b..cc94a19843 100644 --- a/charts/core/src/Chart/hooks/useChart.spec.ts +++ b/charts/core/src/Chart/hooks/useChart.spec.ts @@ -4,6 +4,10 @@ import { renderHook } from '@leafygreen-ui/testing-lib'; import { useChart } from './useChart'; +const EChartEventsMock = { + ZoomSelect: 'zoomselect', +}; + // Mock the useEchart hook jest.mock('../../Echart', () => ({ useEchart: jest.fn(() => ({ @@ -12,6 +16,7 @@ jest.mock('../../Echart', () => ({ setupZoomSelect: jest.fn(), on: jest.fn(), })), + EChartEvents: EChartEventsMock, })); describe('@lg-echarts/core/hooks/useChart', () => { @@ -88,7 +93,10 @@ describe('@lg-echarts/core/hooks/useChart', () => { renderHook(() => useChart({ theme: 'dark', onZoomSelect })); - expect(on).toHaveBeenCalledWith('zoomselect', expect.any(Function)); + expect(on).toHaveBeenCalledWith( + EChartEventsMock.ZoomSelect, + expect.any(Function), + ); // Simulate zoom select event const zoomEventResponse = { start: 0, end: 100 }; @@ -115,7 +123,7 @@ describe('@lg-echarts/core/hooks/useChart', () => { expect(result.current).toEqual({ ...mockEchartInstance, - ref: expect.any(Object), + ref: expect.any(Function), }); }); }); diff --git a/charts/core/src/Chart/hooks/useChart.ts b/charts/core/src/Chart/hooks/useChart.ts index 584c7a33f5..1b78dc3db6 100644 --- a/charts/core/src/Chart/hooks/useChart.ts +++ b/charts/core/src/Chart/hooks/useChart.ts @@ -1,6 +1,7 @@ -import { useEffect, useRef } from 'react'; +import { RefCallback, useCallback, useEffect, useState } from 'react'; import { useEchart } from '../../Echart'; +import { EChartEvents } from '../../Echart'; import { getDefaultChartOptions } from '../config'; import type { ChartHookProps, ChartInstance } from './useChart.types'; @@ -12,10 +13,20 @@ export function useChart({ groupId, theme, }: ChartHookProps): ChartInstance { - const chartRef = useRef(null); const initialOptions = getDefaultChartOptions(theme); + + /** + * It is necessary for `useEchart` to know when the container exists + * in order to instantiate the chart. Since this happens only on first render, + * we use a container element stored in state and a ref callback so that the + * element only gets populated after render. + */ + const [container, setContainer] = useState(null); + const chartRef: RefCallback = useCallback(node => { + setContainer(node); + }, []); const echart = useEchart({ - container: chartRef.current, + container, initialOptions, theme, }); @@ -45,7 +56,7 @@ export function useChart({ useEffect(() => { if (echart.ready && onZoomSelect) { - echart.on('zoomselect', zoomEventResponse => { + echart.on(EChartEvents.ZoomSelect, zoomEventResponse => { onZoomSelect(zoomEventResponse); }); } diff --git a/charts/core/src/Chart/hooks/useChart.types.ts b/charts/core/src/Chart/hooks/useChart.types.ts index d3e3b245fb..1898787ff2 100644 --- a/charts/core/src/Chart/hooks/useChart.types.ts +++ b/charts/core/src/Chart/hooks/useChart.types.ts @@ -1,13 +1,8 @@ -import { MutableRefObject } from 'react'; +import { RefCallback } from 'react'; import { Theme } from '@leafygreen-ui/lib'; -import type { EChartsInstance } from '../../Echart'; - -export interface ZoomSelectionEvent { - xAxis?: { startValue: number; endValue: number }; - yAxis?: { startValue: number; endValue: number }; -} +import type { EChartsInstance, EChartZoomSelectionEvent } from '../../Echart'; export type ZoomSelect = | { @@ -41,9 +36,9 @@ export interface ChartHookProps { /** * Callback to be called when a zoom selection is made. */ - onZoomSelect?: (e: ZoomSelectionEvent) => void; + onZoomSelect?: (e: EChartZoomSelectionEvent) => void; } export interface ChartInstance extends EChartsInstance { - ref: MutableRefObject; + ref: RefCallback; } diff --git a/charts/core/src/Echart/Echart.types.ts b/charts/core/src/Echart/Echart.types.ts index e57f8f7847..7ac47f6de7 100644 --- a/charts/core/src/Echart/Echart.types.ts +++ b/charts/core/src/Echart/Echart.types.ts @@ -1,99 +1,129 @@ +import type { XAXisComponentOption, YAXisComponentOption } from 'echarts'; +import type { LineSeriesOption } from 'echarts/charts'; +import type { + DatasetComponentOption, + GridComponentOption, + LegendComponentOption, + TitleComponentOption, + ToolboxComponentOption, + TooltipComponentOption, +} from 'echarts/components'; +import type { EChartsType } from 'echarts/core'; +import type { ComposeOption } from 'echarts/core'; + import { Theme } from '@leafygreen-ui/lib'; -import type { ChartOptions, SeriesOption } from '../Chart'; +type RequiredSeriesProps = 'type' | 'name' | 'data'; +export type EChartSeriesOption = Pick & + Partial>; -type EChartsEvents = - // Mouse Events - | 'click' // Triggered when clicking on a series, data item, or other graphical element - | 'dblclick' // Triggered when double clicking - | 'mousedown' // Triggered when pressing down the mouse button - | 'mouseup' // Triggered when releasing the mouse button - | 'mouseover' // Triggered when mouse hovers over a component - | 'mouseout' // Triggered when mouse leaves a component - | 'mousemove' // Triggered when mouse moves within a component - | 'globalout' // Triggered when mouse leaves the chart area - | 'contextmenu' // Triggered when right clicking +/** + * TODO: This might need to be improved. `ComposeOption` appears to make most base option + * keys "Arrayable". This is making it difficult to properly test partial options on + * methods like updateUtils > updateOptions(), since something like `options.grid` could be + * an array even if an object. + */ +export type EChartOptions = ComposeOption< + | TooltipComponentOption + | GridComponentOption + | DatasetComponentOption + | TitleComponentOption + | LegendComponentOption + | ToolboxComponentOption + | XAXisComponentOption + | YAXisComponentOption +> & { series?: Array }; +export const EChartEvents = { + // Mouse Events + Click: 'click', + DblClick: 'dblclick', + MouseDown: 'mousedown', + MouseUp: 'mouseup', + MouseOver: 'mouseover', + MouseOut: 'mouseout', + MouseMove: 'mousemove', + GlobalOut: 'globalout', + ContextMenu: 'contextmenu', // Drag Events - | 'dragstart' // Triggered when starting to drag a component - | 'drag' // Triggered while dragging - | 'dragend' // Triggered when ending a drag operation - + DragStart: 'dragstart', + Drag: 'drag', + DragEnd: 'dragend', // Brush Events - | 'brushselected' // Triggered when selecting an area using the brush tool - | 'brushEnd' // Triggered when brush selection ends - | 'brush' // Triggered during brush selection - + BrushSelected: 'brushselected', + BrushEnd: 'brushEnd', + Brush: 'brush', // Legend Events - | 'legendselectchanged' // Triggered when legend selection changes - | 'legendselected' // Triggered when clicking legend item - | 'legendunselected' // Triggered when unselecting legend item - | 'legendselectall' // Triggered when selecting all legend items - | 'legendinverseselect' // Triggered when inversely selecting legend items - | 'legendscroll' // Triggered when scrolling legend - - // Dataoom Events - | 'datazoom' // Triggered when data zoom area changes - | 'datarangeselected' // Triggered when selecting a range in continuous visual map - | 'timelinechanged' // Triggered when timeline changes - | 'timelineplaychanged' // Triggered when timeline play state changes - | 'restore' // Triggered when restoring chart to initial state - + LegendSelectChanged: 'legendselectchanged', + LegendSelected: 'legendselected', + LegendUnselected: 'legendunselected', + LegendSelectAll: 'legendselectall', + LegendInverseSelect: 'legendinverseselect', + LegendScroll: 'legendscroll', + // Datzoom Events + DataZoom: 'datazoom', + DataRangeSelected: 'datarangeselected', + TimelineChanged: 'timelinechanged', + TimelinePlayChanged: 'timelineplaychanged', + Restore: 'restore', // Map Events - | 'georoam' // Triggered when roaming (zooming/panning) in geo coordinate system - | 'geoselected' // Triggered when selecting areas in geo coordinate system - | 'geounselected' // Triggered when unselecting areas in geo coordinate system - + GeoRoam: 'georoam', + GeoSelected: 'geoselected', + GeoUnselected: 'geounselected', // Axis Events - | 'axisareaselected' // Triggered when selecting an axis area - | 'focusnodeadjacency' // Triggered when focusing on adjacent nodes (graph) - | 'unfocusnodeadjacency' // Triggered when unfocusing adjacent nodes (graph) - + AxisAreaSelected: 'axisareaselected', + FocusNodeAdjacency: 'focusnodeadjacency', + UnfocusNodeAdjacency: 'unfocusnodeadjacency', // Rendering Events - | 'rendered' // Triggered after chart rendering finished - | 'finished' // Triggered after chart animation finished - + Rendered: 'rendered', + Finished: 'finished', // Chart Action Events - | 'magictypechanged' // Triggered when changing chart type using magic type switcher - | 'tooltipshown' // Triggered when tooltip is shown - | 'tooltiphidden' // Triggered when tooltip is hidden - | 'selectchanged' // Triggered when selection state changes - | 'globalcursortaken'; // Triggered when global cursor is taken + MagicTypeChanged: 'magictypechanged', + TooltipShown: 'tooltipshown', + TooltipHidden: 'tooltiphidden', + SelectChanged: 'selectchanged', + GlobalCursorTaken: 'globalcursortaken', + // Custom + ZoomSelect: 'zoomselect', +} as const; + +export type EChartEventsType = (typeof EChartEvents)[keyof typeof EChartEvents]; -export interface ZoomSelectionEvent { +export interface EChartZoomSelectionEvent { xAxis?: { startValue: number; endValue: number }; yAxis?: { startValue: number; endValue: number }; } -export interface SetupZoomSelectProps { +export interface EChartSetupZoomSelectProps { xAxis?: boolean; yAxis?: boolean; } interface EChartsEventHandlerType { - // Regular events use the original param type - (event: EChartsEvents, callback: (params: any) => void): void; - // Specific override for 'zoomselect' - (event: 'zoomselect', callback: (params: ZoomSelectionEvent) => void): void; + (event: EChartEventsType, callback: (params: any) => void): void; + ( + event: 'zoomselect', + callback: (params: EChartZoomSelectionEvent) => void, + ): void; } export interface EChartsInstance { - _echartsInstance: any; + _echartsInstance: EChartsType | null; ready: boolean; - options: Partial; - updateOptions: (options: Omit, 'series'>) => void; + options: Partial; + updateOptions: (options: Omit, 'series'>) => void; on: EChartsEventHandlerType; off: EChartsEventHandlerType; - addSeries: (series: SeriesOption) => void; + addSeries: (series: EChartSeriesOption) => void; removeSeries: (name: string) => void; addToGroup: (groupId: string) => void; removeFromGroup: () => void; - setupZoomSelect: (SetupZoomSelectProps: SetupZoomSelectProps) => void; + setupZoomSelect: (props: EChartSetupZoomSelectProps) => void; error: Error | null; } export interface EChartHookProps { container: HTMLDivElement | null; - initialOptions?: Partial; + initialOptions?: Partial; theme: Theme; } diff --git a/charts/core/src/Echart/index.ts b/charts/core/src/Echart/index.ts index 3359f8e419..62df21afa5 100644 --- a/charts/core/src/Echart/index.ts +++ b/charts/core/src/Echart/index.ts @@ -1,6 +1,10 @@ export type { + EChartEventsType, + EChartOptions, + EChartSeriesOption, + EChartSetupZoomSelectProps, EChartsInstance, - SetupZoomSelectProps, - ZoomSelectionEvent, + EChartZoomSelectionEvent, } from './Echart.types'; +export { EChartEvents } from './Echart.types'; export { useEchart } from './useEchart'; diff --git a/charts/core/src/Echart/initializeEcharts.ts b/charts/core/src/Echart/initializeEcharts.ts new file mode 100644 index 0000000000..db19a21c2d --- /dev/null +++ b/charts/core/src/Echart/initializeEcharts.ts @@ -0,0 +1,75 @@ +// Singleton promise for initialization to prevent duplication +let initPromise: Promise | null = null; +let echartsCore: typeof import('echarts/core') | null = null; +let echartsCharts: typeof import('echarts/charts') | null = null; +let echartsRenders: typeof import('echarts/renderers') | null = null; +let echartsComponents: typeof import('echarts/components') | null = null; + +/** + * Initializes ECharts core and required components in a singleton pattern. + * Dynamically imports and configures ECharts modules including charts, + * renderers, and components. Subsequent calls return the cached module to + * prevent duplicate initialization. + * + * @returns Promise resolving to the initialized ECharts core module + * @throws If module imports or initialization fails + * + * @example + * const echarts = await initializeEcharts(); + * // Use initialized echarts module + */ +export async function initializeEcharts(): Promise< + typeof import('echarts/core') +> { + // If already initialized, return echartsCore immediately + if (echartsCore) { + return echartsCore; + } + + // If initialization is in progress, wait for it + if (initPromise) { + return initPromise; + } + + // Start initialization and store the promise + initPromise = (async () => { + try { + const [ + echartsCoreResolved, + echartsChartsResolved, + echartsRendersResolved, + echartsComponentsResolved, + ] = await Promise.all([ + import('echarts/core'), + import('echarts/charts'), + import('echarts/renderers'), + import('echarts/components'), + ]); + + echartsCore = echartsCoreResolved; + echartsCharts = echartsChartsResolved; + echartsRenders = echartsRendersResolved; + echartsComponents = echartsComponentsResolved; + + echartsCore.use([ + echartsCharts.LineChart, + echartsRenders.CanvasRenderer, + echartsComponents.TitleComponent, + echartsComponents.TooltipComponent, + echartsComponents.GridComponent, + echartsComponents.LegendComponent, + echartsComponents.ToolboxComponent, + echartsComponents.DataZoomComponent, + echartsComponents.DataZoomInsideComponent, + ]); + + return echartsCore; + } catch (error) { + // Ensure we clear the promise if initialization fails + initPromise = null; + throw error; + } + })(); + + return initPromise; +} diff --git a/charts/core/src/Echart/useEchart.spec.ts b/charts/core/src/Echart/useEchart.spec.ts index 08f8254c7d..446460629e 100644 --- a/charts/core/src/Echart/useEchart.spec.ts +++ b/charts/core/src/Echart/useEchart.spec.ts @@ -149,7 +149,7 @@ describe('@lg-echarts/core/hooks/useChart', () => { await Promise.resolve(); }); - expect(result.current._echartsInstance.on).toHaveBeenCalledWith( + expect(result?.current?._echartsInstance?.on).toHaveBeenCalledWith( 'click', mockCallback, ); @@ -165,7 +165,7 @@ describe('@lg-echarts/core/hooks/useChart', () => { await Promise.resolve(); }); - expect(result.current._echartsInstance.off).toHaveBeenCalledWith( + expect(result?.current?._echartsInstance?.off).toHaveBeenCalledWith( 'click', mockCallback, ); @@ -179,7 +179,7 @@ describe('@lg-echarts/core/hooks/useChart', () => { await Promise.resolve(); }); - expect(result.current._echartsInstance.group).toBe('test-group'); + expect(result?.current?._echartsInstance?.group).toBe('test-group'); }); test('should remove chart from group on call to `removeFromGroup`', async () => { @@ -191,7 +191,7 @@ describe('@lg-echarts/core/hooks/useChart', () => { await Promise.resolve(); }); - expect(result.current._echartsInstance.group).toBeNull(); + expect(result?.current?._echartsInstance?.group).toBe(''); }); test('should setup zoom select on call to `setupZoomSelect`', async () => { @@ -218,13 +218,13 @@ describe('@lg-echarts/core/hooks/useChart', () => { await Promise.resolve(); }); - expect(result.current._echartsInstance.dispatchAction).toHaveBeenCalledWith( - { - type: 'takeGlobalCursor', - key: 'dataZoomSelect', - dataZoomSelectActive: true, - }, - ); + expect( + result?.current?._echartsInstance?.dispatchAction, + ).toHaveBeenCalledWith({ + type: 'takeGlobalCursor', + key: 'dataZoomSelect', + dataZoomSelectActive: true, + }); }); test('should set `ready` to true when chart instance is available', async () => { diff --git a/charts/core/src/Echart/useEchart.ts b/charts/core/src/Echart/useEchart.ts index 2884e3ea09..631c0f1e55 100644 --- a/charts/core/src/Echart/useEchart.ts +++ b/charts/core/src/Echart/useEchart.ts @@ -1,74 +1,18 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import type { EChartsType } from 'echarts/core'; import debounce from 'lodash.debounce'; -import { ChartOptions } from '../Chart'; import { chartSeriesColors } from '../Chart/chartSeriesColors'; import * as updateUtils from '../Chart/hooks/updateUtils'; import { - EChartHookProps, - EChartsInstance, - ZoomSelectionEvent, + EChartEvents, + type EChartHookProps, + type EChartOptions, + type EChartsInstance, + type EChartZoomSelectionEvent, } from './Echart.types'; - -// Singleton promise for initialization to prevent duplication -let initPromise: Promise | null = null; -let echartsCore: any; -let echartsCharts: any; -let echartsRenders: any; -let echartsComponents: any; - -async function initializeEcharts() { - // If already initialized, return immediately - if (echartsCore) { - return; - } - - // If initialization is in progress, wait for it - if (initPromise) { - return initPromise; - } - - // Start initialization and store the promise - initPromise = (async () => { - try { - const [ - echartsCoreResolved, - echartsChartsResolved, - echartsRendersResolved, - echartsComponentsResolved, - ] = await Promise.all([ - import('echarts/core'), - import('echarts/charts'), - import('echarts/renderers'), - import('echarts/components'), - ]); - - echartsCore = echartsCoreResolved; - echartsCharts = echartsChartsResolved; - echartsRenders = echartsRendersResolved; - echartsComponents = echartsComponentsResolved; - - echartsCore.use([ - echartsCharts.LineChart, - echartsRenders.CanvasRenderer, - echartsComponents.TitleComponent, - echartsComponents.TooltipComponent, - echartsComponents.GridComponent, - echartsComponents.LegendComponent, - echartsComponents.ToolboxComponent, - echartsComponents.DataZoomComponent, - echartsComponents.DataZoomInsideComponent, - ]); - } catch (error) { - // Ensure we clear the promise if initialization fails - initPromise = null; - throw error; - } - })(); - - return initPromise; -} +import { initializeEcharts } from './initializeEcharts'; /** * Wrapper around the ECharts library. Instantiates an ECharts instance. @@ -80,7 +24,10 @@ export function useEchart({ initialOptions, theme, }: EChartHookProps): EChartsInstance { - const [echartsInstance, setEchartsInstance] = useState(null); // has to be any since no types exist until import + const echartsCoreRef = useRef(null); + const [echartsInstance, setEchartsInstance] = useState( + null, + ); const [error, setError] = useState(null); const [ready, setReady] = useState(false); const [options, setOptions] = useState( @@ -90,22 +37,21 @@ export function useEchart({ // Keep track of active handlers const activeHandlers = useRef(new Map()); - const withInstanceCheck = ) => void>( - fn: T, - ) => { - return (...args: Parameters) => { + const withInstanceCheck = ) => any>(fn: T) => { + return (...args: Parameters): ReturnType => { if (!echartsInstance) { console.error('Echart instance not initialized'); - return; + return undefined as ReturnType; } - fn(...args); + + return fn(...args); }; }; const setEchartOptions = useMemo( () => debounce( - withInstanceCheck((options: Partial) => { + withInstanceCheck((options: Partial) => { /** * The second argument is `true` to merge the new options with the existing ones. * This is needed to ensure that series get removed properly. @@ -154,22 +100,24 @@ export function useEchart({ [setEchartOptions], ); - // ECharts does not automatically resize when the window resizes. - const resizeHandler = withInstanceCheck(() => { - echartsInstance.resize(); + const resizeEchartInstance = withInstanceCheck(() => { + echartsInstance?.resize(); }); const addToGroup: EChartsInstance['addToGroup'] = useCallback( - withInstanceCheck(groupId => { - echartsInstance.group = groupId; - echartsCore.connect(groupId); + withInstanceCheck((groupId: string) => { + // echartsCoreRef.current should exist if instance does, but checking for extra safety + if (echartsCoreRef.current) { + (echartsInstance as EChartsType).group = groupId; + echartsCoreRef.current.connect(groupId); + } }), - [echartsCore, echartsInstance], + [echartsCoreRef.current, echartsInstance], ); const removeFromGroup: EChartsInstance['removeFromGroup'] = useCallback( withInstanceCheck(() => { - echartsInstance.group = null; + (echartsInstance as EChartsType).group = ''; }), [echartsInstance], ); @@ -177,13 +125,14 @@ export function useEchart({ const clearDataZoom = useCallback( withInstanceCheck((params: any) => { /** - * If start is not 0% or end is not 100%, the 'dataZoom' event was triggered by a zoom. - * We override the zoom to prevent it from actually zooming. + * If start is not 0% or end is not 100%, the 'dataZoom' event was + * triggered by a zoom. We override the zoom to prevent it from actually + * zooming. */ const isZoomed = params?.start !== 0 || params?.end !== 100; if (isZoomed) { - echartsInstance.dispatchAction({ + echartsInstance?.dispatchAction({ type: 'dataZoom', start: 0, // percentage of starting position end: 100, // percentage of ending position @@ -196,13 +145,13 @@ export function useEchart({ const setupZoomSelect: EChartsInstance['setupZoomSelect'] = useCallback( withInstanceCheck(({ xAxis, yAxis }) => { const enableZoom = withInstanceCheck(() => { - echartsInstance.dispatchAction({ + echartsInstance?.dispatchAction({ type: 'takeGlobalCursor', key: 'dataZoomSelect', dataZoomSelectActive: xAxis || yAxis, }); // This will trigger a render so we need to remove the handler to prevent a loop - echartsInstance.off('rendered', enableZoom); + echartsInstance?.off('rendered', enableZoom); }); // `0` index enables zoom on that index, `'none'` disables zoom on that index @@ -220,9 +169,9 @@ export function useEchart({ }, }); - echartsInstance.on('rendered', enableZoom); - echartsInstance.off('dataZoom', clearDataZoom); // prevent adding dupes - echartsInstance.on('dataZoom', clearDataZoom); + echartsInstance?.on('rendered', enableZoom); + echartsInstance?.off('dataZoom', clearDataZoom); // prevent adding dupes + echartsInstance?.on('dataZoom', clearDataZoom); }), [echartsInstance, updateOptions], ); @@ -230,15 +179,15 @@ export function useEchart({ const off: EChartsInstance['off'] = useCallback( withInstanceCheck((action, callback) => { switch (action) { - case 'zoomselect': { - echartsInstance.off('datazoom', callback); + case EChartEvents.ZoomSelect: { + echartsInstance?.off('datazoom', callback); // Remove from active handlers activeHandlers.current.delete(`${action}-${callback.toString()}`); break; } default: { - echartsInstance.off(action, callback); + echartsInstance?.off(action, callback); activeHandlers.current.delete(`${action}-${callback.toString()}`); } } @@ -257,15 +206,13 @@ export function useEchart({ } switch (action) { - case 'zoomselect': { + case EChartEvents.ZoomSelect: { const zoomHandler = (params: any) => { - const zoomSelectionEvent: ZoomSelectionEvent = {}; + const zoomSelectionEvent: EChartZoomSelectionEvent = {}; + const isSingleAxisZoom = + params.startValue !== undefined && params.endValue !== undefined; - if ( - params.startValue !== undefined && - params.endValue !== undefined - ) { - // Handle single axis zoom + if (isSingleAxisZoom) { const axis = params.dataZoomId?.includes('y') ? 'yAxis' : 'xAxis'; zoomSelectionEvent[axis] = { startValue: params.startValue, @@ -293,34 +240,43 @@ export function useEchart({ // Store the wrapper function so we can remove it later activeHandlers.current.set(handlerKey, zoomHandler); - echartsInstance.on('datazoom', zoomHandler); + echartsInstance?.on('datazoom', zoomHandler); break; } default: { activeHandlers.current.set(handlerKey, callback); - echartsInstance.on(action, callback); + echartsInstance?.on(action, callback); } } }), [echartsInstance], ); + /** + * Sets up the echart instance on initial render or if the container changes. + * Additionally, disposes of echart instance and cleans up handlers on unmount. + */ useEffect(() => { - (async function setupChart() { - try { - setError(null); + setError(null); - await initializeEcharts(); + initializeEcharts() + .then(echartsCore => { + echartsCoreRef.current = echartsCore; if (container) { - const newChart = echartsCore.init(container); + // Init an echart instance + const newChart = echartsCoreRef.current.init(container); + // Set the initial options on the instance newChart.setOption(options); - window.addEventListener('resize', resizeHandler); + // Resize chart when window resizes because echarts don't be default + window.addEventListener('resize', resizeEchartInstance); + setEchartsInstance(newChart); setReady(true); } - } catch (err) { + }) + .catch(err => { setReady(false); console.error(err); setError( @@ -328,46 +284,36 @@ export function useEchart({ ? err : new Error('Failed to initialize ECharts'), ); - } - })(); + }); return () => { + window.removeEventListener('resize', resizeEchartInstance); + activeHandlers.current.clear(); + if (echartsInstance) { - window.removeEventListener('resize', resizeHandler); - echartsInstance.dispose(); + echartsInstance?.dispose(); } }; }, [container]); + /** + * Sets the theme when the instance is created or the theme changes. + * This is not actually necessary on initial render because the theme + * is also set on the default options. This is primarily necessary + * for updating the theme if the theme changes. + */ useEffect(() => { - setOptions(currentOptions => { - const updatedOptions = { - ...currentOptions, - color: chartSeriesColors[theme], - }; - setEchartOptions(updatedOptions); - return updatedOptions; - }); - }, [theme]); - - // Clean up all handlers when the echartsInstance changes - useEffect(() => { - return () => { - if (echartsInstance) { - // Remove all registered handlers - activeHandlers.current.forEach((handler, key) => { - const [action] = key.split('-'); - - if (action === 'zoomselect') { - echartsInstance.off('datazoom', handler); - } else { - echartsInstance.off(action, handler); - } - }); - activeHandlers.current.clear(); - } - }; - }, [echartsInstance]); + if (echartsInstance) { + setOptions(currentOptions => { + const updatedOptions = { + ...currentOptions, + color: chartSeriesColors[theme], + }; + setEchartOptions(updatedOptions); + return updatedOptions; + }); + } + }, [echartsInstance, theme]); return { _echartsInstance: echartsInstance,