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

Correctly resolve peerDependencies for concurrent installs #2801

Merged
merged 2 commits into from
Feb 28, 2017
Merged

Correctly resolve peerDependencies for concurrent installs #2801

merged 2 commits into from
Feb 28, 2017

Conversation

kamilogorek
Copy link
Contributor

Summary

When installing multiple packages at the same time eg. yarn add react react-dom, peerDependencies were incorrectly resolved and triggered warnings.

Related: #2132

Test plan

Before:

λ: yarn add request request-promise-native
yarn add v0.21.3
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
warning "[email protected]" has unmet peer dependency "request@^2.34".
warning "[email protected]" has unmet peer dependency "request@^2.34".
[4/4] 📃  Building fresh packages...
success Saved 2 new dependencies.
├─ [email protected]
└─ [email protected]
✨  Done in 6.05s.

λ: yarn add react react-dom
yarn add v0.21.3
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
warning "[email protected]" has unmet peer dependency "react@^15.4.2".
[4/4] 📃  Building fresh packages...
success Saved 2 new dependencies.
├─ [email protected]
└─ [email protected]
✨  Done in 5.65s.

After:

λ: yarn add request request-promise-native
yarn add v0.21.3
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
[4/4] 📃  Building fresh packages...
success Saved 2 new dependencies.
├─ [email protected]
└─ [email protected]
✨  Done in 5.67s.

λ: yarn add react react-dom
yarn add v0.21.3
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
[4/4] 📃  Building fresh packages...
success Saved 2 new dependencies.
├─ [email protected]
└─ [email protected]
✨  Done in 5.65s.

@bestander
Copy link
Member

bestander commented Feb 28, 2017

Cool, thanks a lot!
Can you add a case when warning is expected to the Test Plan?
Could you add unit tests for the test plan?

@evenstensberg
Copy link

Sweet, I'll close my PR :)

@kamilogorek
Copy link
Contributor Author

@bestander updated PR with tests for all 3 cases

@bestander bestander merged commit 0ad2bf9 into yarnpkg:master Feb 28, 2017
@bestander
Copy link
Member

🥇

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.

3 participants