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 jquery to peerDependencies from dependencies #2163

Merged
merged 1 commit into from
Apr 21, 2017
Merged

move jquery to peerDependencies from dependencies #2163

merged 1 commit into from
Apr 21, 2017

Conversation

Mithgol
Copy link
Contributor

@Mithgol Mithgol commented Apr 5, 2017

Q A
Bug fix? no
New feature? yes
BC breaks? should not
License MIT

After this patch jquery is removed from the dependencies section of package.json and granted peerDependencies status instead of it.

Rationale:

  • Both of these packages (bootstrap-datepicker and jquery) are quite likely to be defined as dependencies of their common parent application (which is, most likely, a web site; could also be an application built on Electron, NW.js, JXcore for Cordova, etc.) and thus to become peers in that sense. Currently, if some version mismatch happens on that level (under that common parent), npm would install another version of jquery under bootstrap-datepicker (and that would be the version required by bootstrap-datepicker in its dependencies). However, bootstrap-datepicker does not rely on Node.js require() unless a CommonJS environment is encountered by UMD, in any other environment that npm's behaviour does not really help anything, and thus npm should rather warn on version mismatch (like it does with peerDependencies) and refrain from trying to fix things.
  • Likewise, if a user has some reason to refrain from npm install jquery (for example, that user might prefer to use a <script src="…"> tag referencing jQuery CDN instead of a local npm-installed jQuery), then npm should not decide to install jQuery as a dependency (which won't be used anyway); a warning about a missing peerDependency would be enough.

@acrobat
Copy link
Member

acrobat commented Apr 6, 2017

Thanks for the explanation @Mithgol. It looks reasonable to move it to peerDependencies.

What do you think @vsn4ik @Azaret?

@acrobat acrobat requested review from Azaret and vsn4ik April 6, 2017 06:10
@acrobat
Copy link
Member

acrobat commented Apr 21, 2017

Thanks @Mithgol

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

Successfully merging this pull request may close these issues.

2 participants