search

leocaseiro / angular-chosen

AngularJS Chosen directive is an AngularJS Directive that brings the Chosen jQuery in a AngularJS way
http://leocaseiro.github.io/angular-chosen/
MIT License
682 stars 248 forks source link

Move to peer dependencies #247

Open VanTanev opened 7 years ago

VanTanev commented 7 years ago

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 commented 7 years ago

Hi @VanTanev,

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

The reason I added jQuery@3.x 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 commented 7 years ago

@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 commented 7 years ago

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

Sounds good to me.

VanTanev commented 7 years ago

@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 commented 7 years ago

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.