search

chartjs / chartjs-plugin-zoom

Zoom and pan plugin for Chart.js
MIT License
598 stars 327 forks source link

Modify UpdateModeEnum to not be const #736

Closed ghost closed 1 year ago

ghost commented 1 year ago

Closes https://github.com/chartjs/chartjs-plugin-zoom/issues/735 Closes https://github.com/chartjs/chartjs-plugin-zoom/issues/721 Closes https://github.com/chartjs/chartjs-plugin-zoom/issues/731

This enum was changed to a normal enum instead of using const in https://github.com/chartjs/Chart.js/pull/11095, so the type is no longer matching with Chart.js.

ghost commented 1 year ago

The change in Chart.js was technically breaking, since the Typescript types in any plugin could not have matched anymore. So bundled with the latest Chart.js version I wouldn't consider this a breaking change. With this change the code does however no longer work with any previous version of Chart.js except for those before the addition of the enum.

elinake commented 1 year ago

What are the exact versions of Chart.js that should work before and after the change? I mean when importing both Chart.js and zoom plugin in a project

flogoe commented 1 year ago

I got it running with the following versions: 

...
"chart.js": "4.1.1",
"chartjs-plugin-zoom": "^2.0.0",
...
ghost commented 1 year ago

I would say that based on https://github.com/chartjs/chartjs-plugin-zoom/pull/695/files#diff-093ad82a25aee498b11febf1cdcb6546e4d223ffcb49ed69cc275ac27ce0ccce

2.0.0 of this library introduced constfor chart.js 4.0.0. Then with 4.2.1const was removed in chart.js, so the changes in this PR should make the library compatible with >=4.2.1 and possibly with 3.x depending on the other changes.

kurkle commented 1 year ago

CI fails because of FF, fix is same as in Chart.js: https://github.com/chartjs/Chart.js/pull/11165/files Feel free to create another PR for that fix

luisrodriguesds commented 1 year ago

Hey! Do you guys have any prediction of when this will be merged?

Thank you!