From 1a7bfc77f7d56ac08d8da97d52a9be6e9af0f479 Mon Sep 17 00:00:00 2001 From: justinpark Date: Tue, 15 Apr 2025 18:40:22 -0700 Subject: [PATCH 1/4] fix(echart): Thrown errors shown after resized --- .../src/components/Echart.tsx | 37 ++++++++++++------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx index 5f5f71c14bd6..116e89e7b22b 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx @@ -25,6 +25,7 @@ import { useLayoutEffect, useCallback, Ref, + useState, } from 'react'; import { useSelector } from 'react-redux'; @@ -106,6 +107,15 @@ use([ LabelLayout, ]); +const loadLocale = async (locale: string) => { + try { + const lang = await import(`echarts/lib/i18n/lang${locale}`); + return lang?.default; + } catch (e) { + console.error(`Locale ${locale} not supported in ECharts`, e); + } +}; + function Echart( { width, @@ -123,6 +133,7 @@ function Echart( // eslint-disable-next-line no-param-reassign refs.divRef = divRef; } + const [didMount, setDidMount] = useState(false); const chartRef = useRef(); const currentSelection = useMemo( () => Object.keys(selectedValues) || [], @@ -148,20 +159,20 @@ function Echart( ); useEffect(() => { - const loadLocaleAndInitChart = async () => { - if (!divRef.current) return; - - const lang = await import(`echarts/lib/i18n/lang${locale}`).catch(e => { - console.error(`Locale ${locale} not supported in ECharts`, e); - }); - if (lang?.default) { - registerLocale(locale, lang.default); + loadLocale(locale).then(localeObj => { + if (localeObj) { + registerLocale(locale, localeObj); } - + if (!divRef.current) return; if (!chartRef.current) { chartRef.current = init(divRef.current, null, { locale }); } + setDidMount(true); + }); + }, [locale]); + useEffect(() => { + if (didMount) { Object.entries(eventHandlers || {}).forEach(([name, handler]) => { chartRef.current?.off(name); chartRef.current?.on(name, handler); @@ -172,14 +183,12 @@ function Echart( chartRef.current?.getZr().on(name, handler); }); - chartRef.current.setOption(echartOptions, true); + chartRef.current?.setOption(echartOptions, true); // did mount handleSizeChange({ width, height }); - }; - - loadLocaleAndInitChart(); - }, [echartOptions, eventHandlers, zrEventHandlers, locale]); + } + }, [didMount, echartOptions, eventHandlers, zrEventHandlers]); // highlighting useEffect(() => { From a399abd2037b4fb8488e32bb1c324f8181839a55 Mon Sep 17 00:00:00 2001 From: justinpark Date: Tue, 15 Apr 2025 19:03:44 -0700 Subject: [PATCH 2/4] add missing dispose --- .../plugins/plugin-chart-echarts/src/components/Echart.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx index 116e89e7b22b..d5f037932bd1 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx @@ -190,6 +190,10 @@ function Echart( } }, [didMount, echartOptions, eventHandlers, zrEventHandlers]); + useEffect(() => { + return () => chartRef.current?.dispose(); + }, []); + // highlighting useEffect(() => { if (!chartRef.current) return; From 70a81dc14ce4eed6ea4b8f301458811c46517b10 Mon Sep 17 00:00:00 2001 From: justinpark Date: Wed, 16 Apr 2025 08:50:20 -0700 Subject: [PATCH 3/4] eslint --- .../plugin-chart-echarts/src/components/Echart.tsx | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx index d5f037932bd1..6844cc815c87 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx @@ -108,12 +108,13 @@ use([ ]); const loadLocale = async (locale: string) => { + let lang; try { - const lang = await import(`echarts/lib/i18n/lang${locale}`); - return lang?.default; + lang = await import(`echarts/lib/i18n/lang${locale}`); } catch (e) { console.error(`Locale ${locale} not supported in ECharts`, e); } + return lang?.default; }; function Echart( @@ -184,15 +185,16 @@ function Echart( }); chartRef.current?.setOption(echartOptions, true); - - // did mount - handleSizeChange({ width, height }); } }, [didMount, echartOptions, eventHandlers, zrEventHandlers]); + // did mount useEffect(() => { + if (didMount) { + handleSizeChange({ width, height }); + } return () => chartRef.current?.dispose(); - }, []); + }, [didMount]); // highlighting useEffect(() => { From 62355cb1bf6b0b19b878cbfe5319563c8332de0f Mon Sep 17 00:00:00 2001 From: justinpark Date: Wed, 16 Apr 2025 09:58:42 -0700 Subject: [PATCH 4/4] recover dispose logic --- .../plugin-chart-echarts/src/components/Echart.tsx | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx index 6844cc815c87..e93327a696f6 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx @@ -185,16 +185,13 @@ function Echart( }); chartRef.current?.setOption(echartOptions, true); - } - }, [didMount, echartOptions, eventHandlers, zrEventHandlers]); - // did mount - useEffect(() => { - if (didMount) { + // did mount handleSizeChange({ width, height }); } - return () => chartRef.current?.dispose(); - }, [didMount]); + }, [didMount, echartOptions, eventHandlers, zrEventHandlers]); + + useEffect(() => () => chartRef.current?.dispose(), []); // highlighting useEffect(() => {