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

Cascading peer dependencies does not work and probably should? #4432

Closed
tomaskulich opened this issue Sep 13, 2017 · 5 comments
Closed

Cascading peer dependencies does not work and probably should? #4432

tomaskulich opened this issue Sep 13, 2017 · 5 comments

Comments

@tomaskulich
Copy link

tomaskulich commented Sep 13, 2017

I'm sorry if this is a dupe or known wontfix. Also it may be the case that I don't understand the semantics of peerDependencies. I failed to find a proper spec on this and the semantics seems to differ between npm and yarn, however the current behavior does not make much sense to me. To the point:

Imagine we have:

{ 
  "name": "A",
  someVersion,
   "dependencies": {
     "B": someVersion
   }
   "peerDependencies": {
      "C": someVersion,
   }
}

{
  "name": "B",
   someVersion,
  "peerDependencies": {
    "C": someVersion
  }
}

in such a setting yarn add A should install A and B without any warnings (assuming all versions match). Instead it installs the packages with a warning that package B has unmet peer dependency (C).

The warning seems strange: Even if C is not listed among A's dependencies, A listed C as the peer dependency. Therefore anyone installing A will have to install C as well which satisfies also peerDep constraint posed by B. The warning therefore does not make sense to me.

tested with yarn v. 1.0.1-20170913.1010 on linux.

@BYK
Copy link
Member

BYK commented Sep 15, 2017

This behavior is expected since peer dependencies are not automatically installed. That said I think you want #1503 which sounds reasonable.

I'm going to close this ticket in favor of others but feel free to reopen if you think this is not a good answer or you have things to add.

@BYK BYK closed this as completed Sep 15, 2017
@tomaskulich
Copy link
Author

No, this is not related to #1503. The problem is that some (quite natural) situations are impossible to model without getting warnings.

Let me rephrase the example to (hope) something little bit more imaginable. I'll use the standard 'plugin' example which motivated peerDeps in the first place.

Say application A depends on some technology W (say, webpack) and plugin P which enhanes webpack somehow. This is the perfect case for peerDependencies:

  • A lists W and P as dependencies
  • P lists W as peerDep

So far so good. But what if P also depends on other plugin, P2 which also does something with webpack and therefore it lists it as a peerDependency. Now you'd want to:

  • A will list W and P as dependencies (nothing changes here)
  • P will list P2 as (std) dependency and W as ... what exactly... ?

And this is the problem - there is no good solution to this. If P lists W as std dependency, you may end up with two incopatible webpacks being installed, which is no-go. If P lists W as peerDep, you end up with warning you cannot avoid - that's what this issue is about.

Maybe the whole concept of peerDependencies needs a little bit more thinking - at least to clear out what exactly should be the semantics of it.

@BYK
Copy link
Member

BYK commented Sep 16, 2017

Thanks a lot for the detailed explanation!

And this is the problem - there is no good solution to this. If P lists W as std dependency, you may end up with two incopatible webpacks being installed, which is no-go. If P lists W as peerDep, you end up with warning you cannot avoid - that's what this issue is about.

Not necessarily. If P -> P2 and P2 lists W as a peer dependency, since A lists W as a dependency, it will be resolved without warnings. Resolution was a bit too strict recently but #4478 addresses that problem and I think it implements what you have in mind.

@BYK
Copy link
Member

BYK commented Sep 16, 2017

Also if both P and P2 lists W as a peer dependency, as long as they list compatible ranges, it will still resolve without warnings.

@tomaskulich
Copy link
Author

Wow #4478 looks really usefull. Out of ~20 warnings I've got before, only two of them remains now. Both are real relevant and should be fixed. Beautiful :)

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

2 participants