Migrating code from react-chart-js to chartjs

[ramyaragupathy]

While working on developing partner pages, @VinayakRugvedi flagged the following:

• we are dependent on both chartjs and react-chartjs libraries for frontend components
• chartjs library community is more active
• react-chartjs library has not been in active development in the last two years

Going ahead we should stick to chartjs and move the existing react-chartjs components to chartjs as well.

cc @royallsilwallz @emi420 @manjitapandey

[htulipe]

Hi, I would like to contribute to the project and saw this issue that seemed like a good first task to tackle. I registered to attend the next team meetup to introduce myself and maybe talk a bit more about this issue.

In the meantime, I took the liberty to dig a little bit more into the matter here. I saw that react-chartjs-2 is a wrapper around chartjs so we can't replace the former by the latter unless we want to code our own chartjs react wrapper. That's doable but I don't see the benefit of this other than removing a dependency (which is always nice). If the wrapping library does a good job, even though it's old, I would stick to it.

Anyhow, I look forward to discuss the matter with you guys.

Cheers

[royallsilwallz]

Hey @htulipe, we're glad that you digged into this issue.

The point you put out is valid. Redoing the charts in chartjs isn't much of a benefit if everything is working fine.
The only time it would create an issue is when we (in future) upgrade our main packages such as react or node version to higher ones. Then, the unmaintained react-chartjs-2 library might not support them and hence we would have to migrate the code to chartjs anyway.

So, you can start working on this and see how it goes. I'll be able to help you if you need any.

Thanks for your interest!

[htulipe]

Hi @royallsilwallz thanks for your reply. I see your point, I dug a little bit deeper as a consequence, here's what I found.

react-chartjs-2 has peer dependencies only on react and chartjs, no worries for node version then.

For chartjs, the peer dependency semver will allow any 4.x versions of the lib. Looking at the library and its repo, it does not look like there's a plan for a v5, v4 is the only version they have so far. Should they respect semver properly, chartjs wont be an issue here for the forseable future.

For react though, the semver won't allow v19 of React which had an RC release in April. There is no due date for the final version but we can expect it anytime soon. At this point we will have an issue if we update this project to React 19.

Where to go from here? First of all, I looked at the BC changes of React 19 and the codebase of react-chartjs-2: luckily none of the BC changes affect the codebase. So if React 19 final does not had other BC changes, updating the semver in react-chartsjs-2 should suffice. Maybe the author, even though inactive right now, will make a new release: I'm pretty sure we won't be the only one interested in having the library accepts React 19.

If that does not happen, we can integrate the codebase of react-chartjs-2 in this repo: we won't have to think about it anymore then. That's doable but I think it a bit early to do that, especially since the library is written in Typscript while our codebase here is not: we would have to strip away the typing which seems to me like a bad idea. Moreover I saw a PR that add TS support, so maybe we should wait for this PR to be merged before incorporating react-chartjs-2 code.

As a recap, I would say there is no rush in ditching react-chartjs-2 right now. Let me known.

[royallsilwallz]

Hey @htulipe, that's a really deep investigation to the problem.
I also think it's quite early at this point. Thanks for the detailed review.

Do you have any thoughts on this @tsmock ?
Will this cause test cases failure as in Chartjs PR when we upgrade packages in the future?

[tsmock]

IIRC, most of the test issues in the chartjs PR were due to lazy imports. So it might be an issue if there are lazy imports in updated libraries. If there are, it is a "simple" matter of figuring out what the lazy imports are and preloading them in the test. I doubt chartjs will use lazy imports in their code (as a library).

With that said, my personal feeling is avoid deprecated/effectively deprecated libraries whenever possible. While we can probably continue using react-chartjs-2 for awhile, there will come a time where we have to move off of it.

[royallsilwallz]

Hey @htulipe, no rush, but if you feel like contributing, you can start anytime. We need not wait for the charts to crash.

Thanks for summarizing @tsmock, really appreciate your opinion here.

[htulipe]

Hi @royallsilwallz the only thing we can do right now is reimplementing react-chartjs-2. It implies ditching all the Typescript they have. Are you sure you want to do that? For me it looks like a bad idea since there is a pr to add TS support to this project.

[htulipe]

I mean, we would end up executing the exact same code but without type safety and tests (they have unit tests)