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

Yarn install peerDependencies of no-hoisted packages' dependencies wrongly #6206

Closed
wuct opened this issue Aug 2, 2018 · 6 comments
Closed
Assignees
Labels
fixed-in-modern This issue has been fixed / implemented in Yarn 2+. triaged

Comments

@wuct
Copy link

wuct commented Aug 2, 2018

Do you want to request a feature or report a bug?
A bug.

What is the current behavior?

Let's say we are working on a big workspace project and all sub-projects are using react@15. We have the following setting in the root package.json:

"workspaces": {
  "packages": ["a", "b"]
}

After react@16 released, we want to migrate our project incrementally and we want to do it for b first, so we update the workspaces to:

"workspaces": {
  "packages": ["a", "b"],
  "nohoist": ["b/**"]
}

This setup can help us avoid getting different versions of reacts in b. For example, if we are using react-redux@5 in both a and b, the current result is:

├── node_modules
│   ├── react@15
│   └── react-redux@5
│     
├── a
│   └── node_modules
├── b
│   └── node_modules
│       ├── react@16
│       └── react-redux@5
│           └── node_modules
│               └── react@15

where b/node_modules/react-redux/node_modules/react is wrong.

What is the expected behavior?

The expected result is:

├── node_modules
│   ├── react@15
│   └── react-redux@5
│     
├── a
│   └── node_modules
├── b
│   └── node_modules
│       ├── react@16
│       └── react-redux@5

Please mention your node.js, yarn and operating system version.

yarn: 1.9.2
node: 8.11.2
npm: 5.6.0
os: macOS 10.13.5

@ghost ghost assigned rally25rs Aug 2, 2018
@ghost ghost added the triaged label Aug 2, 2018
@wuct
Copy link
Author

wuct commented Aug 16, 2018

I think the bug is from this function, but there is no unit test for this file so it's hard to trace code. @BYK could you provide some insight into how this function works?

@SebastianBogado
Copy link

+1 @wuct . Same bug as here #5978

@wuct
Copy link
Author

wuct commented Aug 21, 2018

I've updated the comment to follow the template.

@wuct
Copy link
Author

wuct commented Aug 21, 2018

@SebastianBogado I'm not using lerna, so I guess the bug is from yarn.

@SebastianBogado
Copy link

@wuct 100% agreed

@paul-soporan
Copy link
Member

Closing as fixed in v2.

https://yarnpkg.com/getting-started/migration

@paul-soporan paul-soporan added the fixed-in-modern This issue has been fixed / implemented in Yarn 2+. label Jan 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed-in-modern This issue has been fixed / implemented in Yarn 2+. triaged
Projects
None yet
Development

No branches or pull requests

4 participants