Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

q: chart.js 2.8.0 #489

Closed
andreineculau opened this issue Mar 15, 2019 · 5 comments · May be fixed by cbrightly/pump-and-dump#4
Closed

q: chart.js 2.8.0 #489

andreineculau opened this issue Mar 15, 2019 · 5 comments · May be fixed by cbrightly/pump-and-dump#4

Comments

@andreineculau
Copy link

Expected Behavior

all peer deps are met

Actual Behavior

chart.js 2.8 has been released yesterday, and because we were installing "chart.js@^2.7.2" as a peer dep we now get an error about unmet peer dep

historically, I see vue-chart.js went from

all of that happening in sep-nov 2017, but without clear indications (the peer dep changes are tangent to other fixes)

can you confirm that you have a strict requirement on 2.7, rather than ^2.7.0 ?
thanks!

Environment

  • vue.js version: 2.8.0
  • vue-chart.js version: ^3.4.0
  • npm version: 6.7.0
@apertureless
Copy link
Owner

Hey @andreineculau

thanks for the issue.
The release cycle of chart.js is rather slow. However, they kinda like to introduce sometimes breaking changes in minor versions.

Thus for compatibility and stability reasons we have a rather strict version requirement.

I will always need some time to review the changes in chart.js and see if they affect vue-chartjs, before bumping the peerDependency version.

However, the cool thing about peerDependencies is, that you only get a warning that the version does not match. But it will continue to work. As long as they did not introduce some heavy breaking changes.

If I find a free minute, I will go over the changes and bump the version.

✌️

@andreineculau
Copy link
Author

@apertureless Thanks for the quick reply and confirming the status-quo is intentional
I would have closed this "question" issue, but I see you marked it as feature, so I leave it open.

PS: FWIW for us it fails, because we want to have visual feedback on what top-level deps we install, so after we do npm install, we also call npm ls --depth=0 which then fails with "unmet peer dep" (not saying anything about discrepancy in the behavior 🤐 ) but it's not a biggie for now to be as strict with chart.js as you are in vue-chartjs, since they are unfortunately doing breaking changes in minor versions :(

@shubsengupta
Copy link

FWIW, I manually upgraded to chart.js 2.8.0 and so far so good!

@andreineculau
Copy link
Author

Forgot to ask how do you address vue-chartjs semver? Because the commit is breaking. Non-breaking would be ">=2.7 <2.9"

@apertureless
Copy link
Owner

Oh, good catch! Fixed the peerDependency version to include older versions.
I generally do not consider it as a breaking change, because either in a npm env or browser env it would work / properly build, even if the peerDependency version is mismatched.

A breaking change for me, would be an external API change, that would break peoples projects if they upgrade. Or an internal change that requires strict versions of for example vue.js to work. If vue would add some new function that I utilize and vue-chartjs thus requiring at least that version.

Because then, if people only upgrade vue-chartjs it would not work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants