chartjs / chartjs-adapter-luxon

Luxon adapter for Chart.js
MIT License
33 stars 24 forks source link

chartjs-adapter-luxon 1.1.0 is incompatible with luxon 3.0.1 #61

Closed StephanTLavavej closed 1 year ago

StephanTLavavej commented 1 year ago

I'm seeing the following npm warning when I attempt to update to today's release of luxon 3.0.1:

D:\GitHub\chart>npm install luxon@latest
npm WARN ERESOLVE overriding peer dependency
npm WARN While resolving: stl-status-chart@1.0.0
npm WARN Found: luxon@2.5.0
npm WARN node_modules/luxon
npm WARN   peer luxon@"^1.0.0 || ^2.0.0" from chartjs-adapter-luxon@1.1.0
npm WARN   node_modules/chartjs-adapter-luxon
npm WARN     chartjs-adapter-luxon@"^1.1.0" from the root project
npm WARN   1 more (the root project)
npm WARN 
npm WARN Could not resolve dependency:
npm WARN peer luxon@"^1.0.0 || ^2.0.0" from chartjs-adapter-luxon@1.1.0
npm WARN node_modules/chartjs-adapter-luxon
npm WARN   chartjs-adapter-luxon@"^1.1.0" from the root project

changed 1 package, and audited 81 packages in 457ms

10 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

This may be related to:

https://github.com/chartjs/chartjs-adapter-luxon/blob/1b1c23073eb364bd5bd25de6161a19be44d5b5fc/package.json#L56

It appears that chartjs-adapter-luxon needs to be updated to accept the luxon 3.x series.

(Thanks again for your great work! I use it in the microsoft/STL Status Chart. :smile_cat:)

stockiNail commented 1 year ago

@StephanTLavavej I don't have a huge experience on npm, therefore I'm not sure, but I think the peerDependencies could be:

 "luxon": "^1.0.0"

because the adapter is declared to work with Luxon greater than 1.0.0 version.

stockiNail commented 1 year ago

I have done some tests and the current adapter sounds working properly with Luxon 3.0.1.

Cluster2a commented 1 year ago

Same problem here....

image

benmccann commented 1 year ago

I think the peerDependencies could be: "luxon": "^1.0.0" because the adapter is declared to work with Luxon greater than 1.0.0 version.

That means it must be 1.x version. You'd want it to be >= 1.0.0 for that. * or >= 0.0.0 would probably work too. Docs for it are here: https://docs.npmjs.com/cli/v6/using-npm/semver#ranges

stockiNail commented 1 year ago

@benmccann I had a look to the doc (thanks!). In my opinion, it sounds better to have "luxon": "*", but I'm not aware if the adapter can work with Luxon 0.*.

kurkle commented 1 year ago

I'd use >=1.0.0

claireramming commented 1 year ago

I see the pull request to fix this issue is approved... are there plans to merge it soon?

claireramming commented 1 year ago

does this change also need to be released? I'm still getting the same error when I try to install.

Cluster2a commented 1 year ago

Same here - still getting the same error.

benmccann commented 1 year ago

The change has not been released. You can see that here: https://www.npmjs.com/package/chartjs-adapter-luxon

Perhaps @stockiNail or @kurkle would be able to help with cutting a release

stockiNail commented 1 year ago

@benmccann I cannot do anything because I am not a collaborator

kurkle commented 1 year ago

Released: https://www.npmjs.com/package/chartjs-adapter-luxon/v/1.2.0