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

Move to peer dependencies #247

Open
VanTanev opened this issue Jul 28, 2017 · 5 comments
Open

Move to peer dependencies #247

VanTanev opened this issue Jul 28, 2017 · 5 comments
Assignees
Milestone

Comments

@VanTanev
Copy link
Collaborator

VanTanev commented Jul 28, 2017

Right now, we have jquery, angular and chosen-js as normal dependencies.

Especially for angular and jquery, this is a big issue when used with packers like webpack, because it might force the packer to include 2 versions of those huge libraries - the user-defined one in their package.json and ours. This will happen when the 2 versions are not semver compatible, which is quite easy to fall into.

I would say that we definitely want to move those two to a peerDependency.

For more on peer deps: https://nodejs.org/en/blog/npm/peer-dependencies/

Edit: As an aside, I'm not sure that we're using jQuery v3+ methods? Any reason for pinning to that instead of ^2.0? chosen-js itself supports ^1.4

@leocaseiro
Copy link
Owner

leocaseiro commented Jul 29, 2017

Hi @VanTanev,

Thank you for your suggestion. I agree with you, it sounds very reasonable.

The reason I added [email protected] is because it was the default version from npm install jquery --save.
I'm happy to replace that with an older version if needed to.

Would you like to send me a PR or do you want me to work on that?

@VanTanev
Copy link
Collaborator Author

VanTanev commented Jul 30, 2017

@leocaseiro The change itself is trivial, but we need to make a decision about semver - does this change necessitate bump to 2.0?

Changing a dependency to a peerDependency means that it will no longer be installed with current versions of npm/yarn unless it's present in the user's own package.json. While that is the desired behavior for a plugin, we might have users that depend on it and who might end up with broken projects after the change.

It's kind of unfortunate that such a small fix might necessitate a major version bump, but then again, numbers are fairly cheap. Maybe we just take the jump and do version 2.0 ?

@leocaseiro
Copy link
Owner

If we have break changes, we should 2.x it is.

Sounds good to me.

@leocaseiro leocaseiro added this to the 2.0.0 milestone Jul 30, 2017
@VanTanev
Copy link
Collaborator Author

VanTanev commented Aug 5, 2017

@leocaseiro I am trying to figure out our min requrements. README says angular 1.3, but we are pinned to 1.5+ right now. Where do we want it?

@leocaseiro
Copy link
Owner

I'm almost sure that the compatibility has changed in the last commits, however, the last time I tested it was working on 1.3.x still.

I normally recommend the latest version of all libraries, so I prefer having 1.5+ or even 1.6+ on the package.json, IMO, it would push most users to update the libraries.

However, if it could be an issue for compatibility, perhaps 1.3 would be the most indicated.

I wonder how many developers are within 1.3.x still.

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

No branches or pull requests

2 participants