search

chartjs / chartjs-plugin-annotation

Annotation plugin for Chart.js
MIT License
609 stars 329 forks source link

Update packaging to match main chart.js library #815

Closed stockiNail closed 1 year ago

stockiNail commented 1 year ago

Fix #814

stockiNail commented 1 year ago

@kurkle I wen back to the original version, without using UMD in the dist files, as shared. Let me know if it's ok and if yes, let me know if you think we should publish new version 2.1.1 (bumping).

etimberg commented 1 year ago

Looks good to me, except it won't work in node/commonjs.

@etimberg is it ok for you to drop cjs support for plugins too? (main repo already did)

Dropping is fine by me, but should definitely be a major version release

stockiNail commented 1 year ago

Looks good to me, except it won't work in node/commonjs. @etimberg is it ok for you to drop cjs support for plugins too? (main repo already did)

Dropping is fine by me, but should definitely be a major version release

@etimberg ok that means all plugins/controllers in chartjs org must publish a major version for chartjs 4 compatibility, doesn’t it?

etimberg commented 1 year ago

Looks good to me, except it won't work in node/commonjs. @etimberg is it ok for you to drop cjs support for plugins too? (main repo already did)

Dropping is fine by me, but should definitely be a major version release

@etimberg ok that means all plugins/controllers in chartjs org must publish a major version for chartjs 4 compatibility, doesn’t it?

I don't think it's mandatory if the plugin packaging doesn't change

stockiNail commented 1 year ago

@etimberg @kurkle with CHART.JS 4.1.1 this PR sounds to me useless because the issue should be fixed.

kurkle commented 1 year ago

There could be some benefits to from type: 'module', but maybe nothing worth the time before someone requests something explicit.