[ChartJS] Fix failing build with ChartJS >= 3.9.0

[t-richard]

Q	A
Bug fix?	yes
New feature?	no
Tickets	N/A
License	MIT

Fix the failing builds like https://github.com/symfony/ux/runs/7813370449?check_suite_focus

If running with ChartJS < 3.9.0, the build passes, not sure where it comes from but changing the way we import fixes it.

When I builded locally, it changed some other files which I included here. Those changes seem mostly like syntax changes.

[weaverryan]

Thank you! This has been bothering me, but I hadn’t had a chance to look yet!

But, I think this new import may have a different meaning? See: https://www.chartjs.org/docs/latest/getting-started/integration.html - the old way, it appears, registered everything, while I think the new way might not. We should check. But if I’m right, it looks like we could import that registerables thing and get the same behavior with this new import (I have no idea why the old auto import now fails).

[t-richard]

You are right, we should use the "registerables" thing to make sure we are on par with what was done previously and follow the documented way, though I couldn't find a case where doing it the way this PR does breaks (even when running encore production for example which should do tree-shaking).

I think the culprit PR on the ChartJS side is chartjs/Chart.js#10479 but not sure why it fails as they say to have kept backward compatibility.

I've pushed a new commit doing Chart.register(...registerables) but that now fails the tests 😅

So I'm wondering if that's not a test vs build toolchain difference, a bit clueless on this one to be honest.

[weaverryan]

I was just about to open an issue on chart.js about this, but i thought I should dig into the issue more and see if I could get a reproducer. Well, I failed! And I can't figure our why yet.

Here is my reproducer - https://github.com/weaverryan/chartjs-rollup-reproducer - which leverages rollup + typescript and imports chart.js. But, when you run yarn rollup -c, it works. We need to figure out what differs between that project and this project. If you have any time to check into it, I'd appreciate it - I'm about to start vacation. I tried commenting out some things in the symfony/ux rollup script to see if I could find the cause, but no luck.

[t-richard]

As pointed out in your reproducer repo, it's reproduceable.

The only thing I can say from my tests is that it seems linked to Typescript because if I remove the rollup typescript plugin and change index.ts to index.js, it builds correctly.

Do you want me to go ahead and create an issue in chart.js or do you have a clue to why it's failing ?

For reference, my reproducer is failing in the same way as the UX repo on main https://github.com/t-richard/chartjs-rollup-reproducer

[weaverryan]

For reference, my reproducer is failing in the same way as the UX repo on main https://github.com/t-richard/chartjs-rollup-reproducer

Ah, wonderful! I may have written poorly, I actually could NOT reproduce the issue on my repo. But I see that I had a silly typo that you fixed 👏 .

So now that you DO have a simple reproducer repository, yes, please open an issue on the chart.js repository. It will be especially helpful to be absolutely sure that the bad behavior was introduced in exactly 3.9.0 (so, try 3.8.2 on the reproducer temporarily so that you can say on the issue for sure that 3.9.0 is the cause).

Thank you :)

[t-richard]

I validated that 3.8.2 works as expected.

I've opened an issue on chart.js, lets see if someone can help there.

Have nice vacation!

[weaverryan]

Hey @t-richard!

Did you get any answers on the chartjs issue? Can you link to it?

Thanks!

[ahmedyakoubi]

hey @weaverryan
here

[weaverryan]

Closing for #468