react-echarts icon indicating copy to clipboard operation
react-echarts copied to clipboard

Browser Console: [ECharts] There is a chart instance already initialized on the dom.

Open robertgz opened this issue 1 year ago • 20 comments

Description

I am seeing a browser console messages when a component containing the EChart is rendered multiple times while the chart is initializing. I am not sure if this is an issue or if I need to change how I use the EChart component.

The console message I see when using Chrome is: [ECharts] There is a chart instance already initialized on the dom.

Link to Reproduction

https://stackblitz.com/edit/stackblitz-starters-6kgmdx?file=app%2Fpage.tsx

Steps to reproduce

  1. Open the Stackblitz
  2. Click Open Preview in a new tab at the top right of stackblitz and click on Connect to Project in the new tab/window.
  3. Open the browser Console for the new tab/window.
  4. The tab/window with the Preview may need to be refreshed a few times to see the message in the browsers Console.

JS Framework

Next.js 13 (TS)

Version

1.2.0

Browser

Google Chrome 126

Operating System

  • [ ] macOS
  • [X] Windows
  • [ ] Linux

Additional Information

If the number used in setInterval is increased from 200 to 2000 the console message does not show.

robertgz avatar Jul 02 '24 02:07 robertgz

@robertgz fixed in 1.2.1

hugocxl avatar Aug 12 '24 13:08 hugocxl

@robertgz fixed in 1.2.1

Still appears in 1.4.0

toshiya14 avatar Sep 19 '24 13:09 toshiya14

This issue appears not to be solved. Like @toshiya14 it also still happens to me for most Charts without apparent reason. I found out this only accures on the first render when the chart is rerendered the chart appears. So my current solution: I use the useEffect hook without parameters and make a timeout with 1ms delay that just somehow triggers a rerender. In my case by just useing the next best useState I found. Like this:

const [value, setValue] = useState(0);
useEffect(() => {
    setTimeout(() => {
      setValue(value);
    }, 1);
  });

But this is propably the dirtiest quick fix I came up with and I hate it.

Addition: the setValue forcing a rerender must also be a dependency in the Chart instance to force the rerender there.

hoeflerjo avatar Oct 09 '24 08:10 hoeflerjo

I can confirm. I'm experiencing same issue here. Any chance it will be fixed ?

piotrzarzycki21 avatar Oct 21 '24 17:10 piotrzarzycki21

Hi guys, I am currently really busy and I cannot dedicate much time to solve it. It'd great if you could take a look (I can provide details for those who are interested). I am planning on releasing a v2.0 with a fresher API in the upcoming future, but currently I am not getting much time to dedicate to the lib. Sorry for the inconveniences 🙏

hugocxl avatar Oct 21 '24 19:10 hugocxl

Workaround from @hoeflerjo worked, but definitely I would love to look and see if I can provide PR. :)

piotrzarzycki21 avatar Oct 21 '24 20:10 piotrzarzycki21

If you could point me to the right direction @hugocxl or if you won't have time I will try to figure out myself what is going on, but it will be probably super slowly ;)

piotrzarzycki21 avatar Oct 22 '24 07:10 piotrzarzycki21

@piotrzarzycki21 sure. Whenever the init function is called the ECharts instance gets initialized. https://github.com/hugocxl/react-echarts/blob/68f40a939e4be015f8647e412ae2f98c04af30bd/src/use-echarts.ts#L164

I tried disposing when the component unmounts (well I tried several options to do this, nothing worked), but somehow the instance does not get discarded. Therefore, next time init is called that warning is thrown.

Not sure what I'm missing here 😕

hugocxl avatar Nov 02 '24 08:11 hugocxl

@piotrzarzycki21 sure. Whenever the init function is called the ECharts instance gets initialized.

https://github.com/hugocxl/react-echarts/blob/68f40a939e4be015f8647e412ae2f98c04af30bd/src/use-echarts.ts#L164

I tried disposing when the component unmounts (well I tried several options to do this, nothing worked), but somehow the instance does not get discarded. Therefore, next time init is called that warning is thrown.

Not sure what I'm missing here 😕

Thank you! :) I will dive into that, won't be quick since I'm busy as well, but this warning drive me crazy in our app. ;)

piotrzarzycki21 avatar Nov 02 '24 08:11 piotrzarzycki21

@hugocxl I have implemented a solution to this in the PR I created.

const setContainerRef = async (node: T) => {
    if (!node || node === containerRef.current) return;
    if (echartsRef.current) echartsRef.current.dispose();

    containerRef.current = node;
    echartsRef.current = await setupECharts(node, options);
    resizeObserverRef.current = startResizeObserver();

    setStarted(true);
};

brandanking-decently avatar Nov 27 '24 15:11 brandanking-decently

Awesome @brandanking-decently !! Thank you! I run out of time to look into that. I hope it will be merged.

piotrzarzycki21 avatar Nov 27 '24 15:11 piotrzarzycki21

Still waiting for an update on this getting reviewed. Hopefully soon as this is starting to be a blocker for us

brandanking-decently avatar Dec 04 '24 13:12 brandanking-decently

@brandanking-decently Maybe you can publish your fork on npm if the maintainer is not responsive ?

amarant avatar Dec 18 '24 14:12 amarant

Merged and published @brandanking-decently's PR under v1.4.1. Thanks for having the time to look into this 🙏🏻

hugocxl avatar Dec 19 '24 07:12 hugocxl

There seems to be a problem with the release :

[plugin:vite:import-analysis] Failed to resolve entry for package "@kbox-labs/react-echarts". The package may have incorrect main/module/exports specified in its package.json.

There is no index.mjs in the dist directory as specified in the module section of the package.json.

amarant avatar Dec 19 '24 12:12 amarant

@amarant published a patch. Should be solved now

hugocxl avatar Dec 20 '24 00:12 hugocxl

@hugocxl yes it works fine now, thanks a lot !

amarant avatar Dec 20 '24 13:12 amarant

@hugocxl I have implemented a solution to this in the PR I created.

const setContainerRef = async (node: T) => { if (!node || node === containerRef.current) return; if (echartsRef.current) echartsRef.current.dispose();

containerRef.current = node;
echartsRef.current = await setupECharts(node, options);
resizeObserverRef.current = startResizeObserver();

setStarted(true);

};

@brandanking-decently @hugocxl These changes causes some regression in our app. What is happening is that charts which are super busy started to be drawing really slow. Our application is shadcn Tab oriented. When I click on each tab I have charts. There are mostly line charts, when I switch between tabs I see on the screen that lines are being drawn. I will try capture some video of this, but I have a question - do you have to dispose that echartsRef instance ? I have a feeling that this is the reason why it is all happen.

piotrzarzycki21 avatar Feb 03 '25 16:02 piotrzarzycki21

@piotrzarzycki21 any chance you got a video? Would like to take a look

MiracleBlue avatar Feb 22 '25 03:02 MiracleBlue

@piotrzarzycki21 any chance you got a video? Would like to take a look

@MiracleBlue I'm sorry for such a looong delay in replay. Here is recording of charts. I can't show to much more but you should see what is happening.

Version 1.4.0 - working great, chart is drawing instantly. I'm changing data during recording it is also happening instantly.

https://github.com/user-attachments/assets/80633257-bdf1-4cf0-8632-40573161de96

Version 1.4.2 - Slowness.

https://github.com/user-attachments/assets/2f45a90d-9bda-4e13-8dd5-419cb3a21464

piotrzarzycki21 avatar Mar 14 '25 14:03 piotrzarzycki21