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

Remove the sshpk from peerDependencies #51

Closed
IgorAufricht opened this issue Nov 18, 2015 · 10 comments
Closed

Remove the sshpk from peerDependencies #51

IgorAufricht opened this issue Nov 18, 2015 · 10 comments

Comments

@IgorAufricht
Copy link

http-signature doesn't play well with npm prune and npm shrinkwrap when using npm@2. The reason for this is the sshpk dependency in peerDependencies:

  "dependencies": {
    "assert-plus": "^0.1.5",
    "jsprim": "^1.2.2",
    "sshpk": "^1.4.6"
  },
  "peerDependencies": {
    "sshpk": "^1.4.6"   <- this
  },

How to reproduce the problem:

  1. create a package.json file like this:
{
  "dependencies": {
    "http-signature": "~1.0.2"
  }
}
  1. run npm install (use npm@2, I used 2.14.0)
  2. runing npm shrinkwrap fails:
npm ERR! extraneous: sshpk@1.6.2 c:\dev\labs\http-signature-test\node_modules\sshpk
  1. running npm prune and then npm shrinkwrap results in npm prune unbuilding the peer dependency and npm shrinkwrap failing:
npm ERR! missing: sshpk@^1.4.6, required by http-signature@1.0.2

I propose to remove sshpk from peerDependencies, since it is already specified in dependencies.

(sshpk was added to peerDependencies in this commit: dc1ac85)

Thanks.

@ghost
Copy link

ghost commented Nov 18, 2015

Hi,

Can you please remove sshpk as the peer dependencies. It does not go well with node 4.2.2 and npm 2.14.7, while doing a shrink-wrap

@scottgrasley
Copy link

+1

@suprememoocow
Copy link

I'm experiencing this problem (via a dependency on request). I'm sure a lot of other request users are probably going to experience it too.

As a workaround, I've added sshpk as a direct dependency of the module we're trying to shrinkwrap until we have a better solution.

@nunofgs
Copy link

nunofgs commented Nov 18, 2015

@suprememoocow yup, just experienced it myself. In my case I locked request to 2.65.0 for now.

@suprememoocow
Copy link

@nunofgs That works if you're using request directly, but it's not possible if you have a dependency that uses request@^2.65.0 (which semver will resolve with `request@2.66.0) -- unless you change it there too, which isn't always possible.

@tmpvar
Copy link

tmpvar commented Nov 18, 2015

took the same approach as @nunofgs, and got lucky enough to avoid the situation described by @suprememoocow

@arekinath
Copy link
Contributor

This is all rather unfortunate. It seems that peerDependencies with npm shrinkwrap doesn't do at all what it does with npm install (which is also not what it's documented as supposed to do, either).

The reason why there is both a peerDependencies and dependencies entry for sshpk is that some of the public functions for http-signature in the latest version can accept sshpk.Key objects as an argument. To make sure that everything works properly, this means that anything that is using both http-signature and sshpk like this should make sure to use the same version of sshpk that http-signature does (or else the mixed versions may not play nicely) -- so the peerDependency enforces this, making sure there can only be one copy of the sshpk library in your node_modules.

Since it doesn't look like npm shrinkwrap is going to be fixed any time soon wrt this (the bug has been open for a very long time), I guess I'll have to make sshpk detect any potential misbehaviour resulting from mixed versions at runtime (by tagging all of its own objects with what version they were created by, or something similar). Then the peerDependency can be dropped.

@arekinath
Copy link
Contributor

Ok, sshpk has been updated to deal with versioning its objects, and http-signature 1.1.0 is now published to npm which has the dep bumped up to this version and removes the peerDependency.

@suprememoocow
Copy link

Thank you so much @arekinath. That's great news

@IgorAufricht
Copy link
Author

Wow, that was fast. Thank you very much @arekinath.

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

No branches or pull requests

6 participants