search

hugocxl / react-echarts

🐳 ECharts for React
https://hugocxl.github.io/react-echarts/
MIT License
69 stars 4 forks source link

Data transition animation issue #20

Closed xsjcTony closed 4 months ago

xsjcTony commented 5 months ago

Description

Looking at the official data transition example: https://echarts.apache.org/handbook/en/how-to/animation/transition It looks really smooth and fancy, however, within this library, this is broken: https://stackblitz.com/edit/vitejs-vite-bcghqm?file=src%2FApp.tsx

Why invoke .clear() on option change?

https://github.com/hugocxl/react-echarts/blob/ae6bd50f253345edb7efef5936c0c04fd1b20057/src/use-echarts.ts#L179 This line is definitely which breaks this in the first place. If the instance is cleared, there's no way ECharts can diff the options, hence animation won't be applied. And what's even worse, the entry animation will be played everytime option changes. If it's intend to avoid memory leak, we should just clear it when component unmount by return value of empty-dependency-array useEffect

Maybe, passing down property: undefined is different from not passing that property in ECharts options

Since you are restructuring everything then pass down to setOptions(), this may have some weird behaviour on how ECharts handles data and diff them. At least this is true in some of ESLint rules, that passing undefined is different from not passing.

I've tried to remove .clear() call and remove everything except series when passing to setOptions(), https://github.com/hugocxl/react-echarts/blob/ae6bd50f253345edb7efef5936c0c04fd1b20057/src/use-echarts.ts#L215 then the example above works, but unfortunately I can't edit node_modules folder in stackblitz.

Link to Reproduction

https://stackblitz.com/edit/vitejs-vite-bcghqm?file=src%2FApp.tsx

Steps to reproduce

No response

JS Framework

React TS

Version

latest

Browser

Chrome 121

Operating System

Additional Information

No response

hugocxl commented 5 months ago

@xsjcTony I understand your solution is working? Could you open a PR? It'd be a great contribution. Thanks!

hugocxl commented 4 months ago

@xsjcTony I was reviewing the reason to include the "clear" call. I see that somehow, when it is not being called, ECharts does not render properly during the first render.

I need to spend some time on this to figure out where the problem is coming from, but I'm currently pretty busy. Sorry for the inconvenience.

hugocxl commented 4 months ago

Fixed in v1.0.4, thanks to @brandanking-decently contribution.

YuriGor commented 3 months ago

Hi @hugocxl I checked link to reproduction from OP https://stackblitz.com/edit/vitejs-vite-bcghqm?file=src%2FApp.tsx updated package.json there to have "@kbox-labs/react-echarts": "^1.0.4" still issue is reproducing. I already opened new issue #25 but it seems we need to reopen this one instead?

hugocxl commented 3 months ago

Hi @YuriGor,sorry for the delay. Thanks for the reproduction link. I'm having a pretty busy time lately, I'll try to get into the issue as soon as possible. Thanks again