search

chartjs / chartjs-plugin-zoom

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

refactor: VuePress to VitePress #809

Open bavoco opened 8 months ago

bavoco commented 8 months ago

This reduces the number of dev dependencies by about 700.

Towards #812

NotTsunami commented 7 months ago

By nature, the move from VuePress -> VitePress should also allow us to drop the babel dependencies too, right? A very brief peek at the source suggests it is not used outside of VuePress.

NotTsunami commented 7 months ago

I tested it on your branch and npm run docs passes with the babel dependencies removed. With @babel/core, @babel/preset-env, and babel-loader removed, another 155 packages are removed. npm run test passes with 76/76 tests while npm run lint and npm run build complete without error.

NotTsunami commented 7 months ago

You should be able to remove ng-hammerjs as well, since it was only used in VuePress: https://github.com/chartjs/chartjs-plugin-zoom/blob/bfd207d9108cbcd612b8f4c2b0221ef8e7ed373b/docs/.vuepress/config.js#L89-L90

bavoco commented 7 months ago

Great! If #808 gets merged there won't be two versions of Rollup and related plugins further reducing the count.

kurkle commented 2 weeks ago

You should be able to remove ng-hammerjs as well, since it was only used in VuePress:

https://github.com/chartjs/chartjs-plugin-zoom/blob/bfd207d9108cbcd612b8f4c2b0221ef8e7ed373b/docs/.vuepress/config.js#L89-L90

Hammer is needed by the zoom plugin, and ng-hammerjs can be used as replacemend (it does not require window). So I don't think that can be removed (until this plugin gets rid of the dependency)

kurkle commented 2 weeks ago

I did not have time to take a look at this yet, but I'm game getting rid of VuePress.

NotTsunami commented 2 weeks ago

You should be able to remove ng-hammerjs as well, since it was only used in VuePress: https://github.com/chartjs/chartjs-plugin-zoom/blob/bfd207d9108cbcd612b8f4c2b0221ef8e7ed373b/docs/.vuepress/config.js#L89-L90

Hammer is needed by the zoom plugin, and ng-hammerjs can be used as replacemend (it does not require window). So I don't think that can be removed (until this plugin gets rid of the dependency)

Yes, hammer.js is still needed, but ng-hammerjs was only used for VuePress, as it appeared, so I was suggesting the move to VitePress could remove ng-hammerjs. I'm not suggesting to remove hammer entirely.

bavoco commented 2 weeks ago

Good to know there is interest in this! I will have to take another look at it, because some things have changed in the meantime (VitePress 1.0, ...).

stockiNail commented 2 weeks ago

I did not have time to take a look at this yet, but I'm game getting rid of VuePress.

@kurkle does it mean we will migrate the other plugins (i.e. annotation) too? I personally agree with that because many of security alerts are coming from a Vuepress version.

kurkle commented 2 weeks ago

@kurkle does it mean we will migrate the other plugins (i.e. annotation) too? I personally agree with that because many of security alerts are coming from a Vuepress version.

Hopefully 😅

stockiNail commented 2 weeks ago

@kurkle fine! I'll open a task in the annotation as soon as I can ;)

NotTsunami commented 1 day ago

What’s the consensus on moving the doc portion to a separate package.json like the main chart.js repo does?