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 workspace not hoisting dependencies correctly #7572

Closed
jackyef opened this issue Sep 25, 2019 · 27 comments
Closed

Yarn workspace not hoisting dependencies correctly #7572

jackyef opened this issue Sep 25, 2019 · 27 comments
Labels
fixed-in-modern This issue has been fixed / implemented in Yarn 2+.

Comments

@jackyef
Copy link

jackyef commented Sep 25, 2019

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

What is the current behavior?
Yarn workspace seems to be counting the references from external modules to decide whether to hoist a module or not. This causes an unexpected behavior in my opinion. A minimal reproduction can be looked at this repository.

In the repo, there are 3 packages, called serviceA, serviceB, and serviceC.

  1. serviceA has dependency to react@16.8.6, react-dom@16.8.6 and react-image-fallback@8.0.0
  2. serviceB and serviceC have dependency to react@16.9.0, react-dom@16.9.0

After running yarn install in the project root, we end up with react@16.8.6 and react-dom@16.9.0 in the root node_modules. This mismatching version can cause problems in react apps.

What is the expected behavior?
I expected that react@16.8.6 and react-dom@16.8.6 to be installed in serviceA's node_modules, and react@16.9.0, react-dom@16.9.0 to be hoisted to root node_modules because there are more local packages that depends on them.

Even when I run yarn why react, yarn itself seems to be expecting react@16.9.0 to be hoisted, not react@16.8.6.

yarn why v1.17.3
[1/4] 🤔  Why do we have the module "react"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "react@16.9.0"
info Has been hoisted to "react"
info Reasons this module exists
   - "workspace-aggregator-0de2b996-5873-43fd-bf58-f1a0a092304c" depends on it
   - Hoisted from "_project_#serviceb#react"
   - Hoisted from "_project_#servicec#react"
info Disk size without dependencies: "248KB"
info Disk size with unique dependencies: "436KB"
info Disk size with transitive dependencies: "524KB"
info Number of shared dependencies: 3
=> Found "servicea#react@16.8.6"
info This module exists because "_project_#servicea" depends on it.
✨  Done in 0.13s.

Please mention your node.js, yarn and operating system version.
Node version: 10.16.2
Yarn version: 1.17.3
OS version: macOS 10.14.6

@jackyef
Copy link
Author

jackyef commented Sep 30, 2019

Sorry, bumping this issue to keep it alive.

@exogen
Copy link

exogen commented Oct 4, 2019

I'm seeing both incorrect resolution and hoisting as well with at least v1.17.3, v1.18.0, and v1.19.0.

packages/demo-app/package.json declares a dep on next: 9.0.7 (exact).
packages/next-server/package.json declares a peerDep and devDep on next: ^9.0.0.

First off they both should resolve to 9.0.7, because that satisfies ^9.0.0 just fine. Instead, version 9.0.0 is also installed to satisfy next-server for some reason.

Additionally, Yarn's own output of yarn list does not match how it was actually hoisted.

├─ @techstyle/next-server@1.3.0
│  └─ next@9.0.0
└─ next@9.0.7

In reality, 9.0.0 was placed in the root and 9.0.7 was placed in demo-app. Yarn is confused about both what should be installed and the tree it actually installed.

@attilavago
Copy link

Having the same issue with @material-ui
I seem to be ending up with it in the root of my monorepo for no good reason. 😮

@faudisio
Copy link

Same issue here. Can this issue be assigned for resolution?

@christ776
Copy link

Same issue here as well.

@Siegrift
Copy link

Siegrift commented Apr 2, 2020

I have also problem with dependency hoisting. From https://classic.yarnpkg.com/en/docs/workspaces/ I assumed that the hoisting is done only when all of the modules specify the same dependency...

In my case I have two workspaces A and B. I want to add @types/lodash to A, but it is automatically hoisted and used in B which is just wrong (My intention is to have this dependency only for A).

For now, my solution is to forbid hoisting at all. E.g. change "workspaces": ["A", "B"] to:

  "workspaces": {
    "packages": [
      "A",
      "B"
    ],
    "nohoist": [
      "**"
    ]
  },

I would say this workaround is a fix for the problem above...

Am I misunderstanding something? Or this is intended behaviour? If so, can you explain the reasoning behind this?

@Fallup
Copy link

Fallup commented Apr 2, 2020

I have also problem with dependency hoisting. From https://classic.yarnpkg.com/en/docs/workspaces/ I assumed that the hoisting is done only when all of the modules specify the same dependency...

In my case I have two workspaces A and B. I want to add @types/lodash to A, but it is automatically hoisted and used in B which is just wrong (My intention is to have this dependency only for A).

For now, my solution is to forbid hoisting at all. E.g. change "workspaces": ["A", "B"] to:

  "workspaces": {
    "packages": [
      "A",
      "B"
    ],
    "nohoist": [
      "**"
    ]
  },

I would say this workaround is a fix for the problem above...

Am I misunderstanding something? Or this is intended behavior? If so, can you explain the reasoning behind this?

@Siegrift According to the issue it obviously does not matter whether all packages have the dependency or just one or few if external references are taken into account.
Currently the best approach is to imo assume that Yarn workspaces can hoist anything (also because the hoisting algorithm can change in the future).

Using noHoist is completely valid way of preventing the issue you mentioned. noHoist of everything is a brute force (but valid solution), you just loose the disc space saving.
I mostly prefer to be selective of packages which should not get hoisted:

  "workspaces": {
    "packages": [
      "packages/*"
    ],
    "nohoist": [
      "**/@types/lodash",
      "**/@types/lodash/**"
    ]

The bigger problem is then preventing Yarn from installing deps for other packages when you want to build e.g. one package on your CI (this is related #4099)

But it would be great if someone one could clarify the hoisting strategy.

@Siegrift
Copy link

Siegrift commented Apr 2, 2020

@Fallup Yeah, there are a few specific points that would be nice to address:

  1. Introduction to workspaces should mention this explicitly
  2. I don't understand why yarn can't hoist only if the dependency is exactly the same (or when the major version is the same similarly how node_modules are resolved). But this is basically clarifying the resolution strategy you mention.

With selective approach you never know if you workspace doesn't accidentally pick up unwanted dependency. I will gladly trade off space for this guarantee. And I guess it also resolves #4099, because the workspace has all of it's modules installed inside it's node_modules.

@Fallup
Copy link

Fallup commented Apr 2, 2020

@Siegrift Possible issues should be definitely communicated in a clearer, more explicit way. Although they were partially mentioned in Introduction to workspaces - limitations and caveats first point:

The package layout will be different between your workspace and what your users will get (the workspace dependencies will be hoisted higher into the filesystem hierarchy). Making assumptions about this layout was already hazardous since the hoisting process is not standardized, so theoretically nothing new here. If you encounter issues, try using the nohoist option

I'd also suggest to read Dependencies done right which explains few issues related to hoisting which might partially answer your 2. question.
Still I wish the hoisting behavior would be described a bit clearer as it is in case of Lerna.

Regarding the selective approach, this is the quote directly from yarn nohoist blog:

While nohoist is useful, it does come with drawbacks. The most obvious one is the nohoist modules could be duplicated in multiple locations, denying the benefit of hoisting mentioned above. Therefore, we recommend to keep nohoist scope as small and explicit as possible in your project.

Nohoisting everything does not resolve #4099 as "nohoist everything" does not equal to installing "one package in isolation" - it rather install all packages, but with deps. not getting hoisted to the root.

Hoisting has always been an issue and if your monorepo is small enough and you can afford it, then nohoisting everything might save you a few headaches.

@eliw00d
Copy link

eliw00d commented May 1, 2020

I just ran into some strange issues hoisting with the following:

structure

apps/mobile
apps/web
libs/shared

package.json

  "workspaces": {
    "nohoist": [
      "**/react-native",
      "**/react-native/**",
      "**/react-native-*",
      "**/*-react-native"
    ],
    "packages": [
      "apps/*",
      "libs/*"
    ]
  },

All three have the same version of react as a dependency. libs/shared is meant to be transpiled by babel and used as a package in apps/*.

react is correctly hoisted for apps/web and libs/shared, but not hoisted for apps/mobile.

yarn why v1.22.4
[1/4] 🤔  Why do we have the module "react"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "react@16.13.1"
info Reasons this module exists
   - "_project_#mobile" depends on it
   - Hoisted from "_project_#mobile#react"
   - Hoisted from "_project_#web#react"
   - Hoisted from "_project_#shared#react"
info Disk size without dependencies: "244KB"
info Disk size with unique dependencies: "432KB"
info Disk size with transitive dependencies: "520KB"
info Number of shared dependencies: 5
✨  Done in 0.93s.

This causes invalid hook errors due to having multiple react instances.

I was able to get it to work by moving react to peerDependencies, but that seems like something I should not have to do.

Any thoughts?

@Fallup
Copy link

Fallup commented May 1, 2020

I just ran into some strange issues hoisting with the following:

structure

apps/mobile
apps/web
libs/shared

package.json

  "workspaces": {
    "nohoist": [
      "**/react-native",
      "**/react-native/**",
      "**/react-native-*",
      "**/*-react-native"
    ],
    "packages": [
      "apps/*",
      "libs/*"
    ]
  },

All three have the same version of react as a dependency. libs/shared is meant to be transpiled by babel and used as a package in apps/*.

react is correctly hoisted for apps/web and libs/shared, but not hoisted for apps/mobile.

yarn why v1.22.4
[1/4] 🤔  Why do we have the module "react"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "react@16.13.1"
info Reasons this module exists
   - "_project_#mobile" depends on it
   - Hoisted from "_project_#mobile#react"
   - Hoisted from "_project_#web#react"
   - Hoisted from "_project_#shared#react"
info Disk size without dependencies: "244KB"
info Disk size with unique dependencies: "432KB"
info Disk size with transitive dependencies: "520KB"
info Number of shared dependencies: 5
✨  Done in 0.93s.

This causes invalid hook errors due to having multiple react instances.

I was able to get it to work by moving react to peerDependencies, but that seems like something I should not have to do.

Any thoughts?

I was in the same situation and went for noHoist of react as well.

@eliw00d
Copy link

eliw00d commented May 19, 2020

Is it better to only have react, or any other shared dependency, in the package.json of libs/shared knowing it will be hoisted?

@JaosnHsieh
Copy link

fixed electron-builder install-app-deps error by nohoist in a mono repository built with lerna.

commit

@selrond
Copy link

selrond commented Dec 15, 2020

Have you managed to find some solution? Dealing with exactly this problem

@ianmartorell
Copy link

I ended up upgrading to yarn berry and the experience with workspaces is much better and more consistent.

@selrond
Copy link

selrond commented Dec 15, 2020

@ianmartorell but does it somehow solve the unpredictable hoisting?

@ianmartorell
Copy link

@ianmartorell but does it somehow solve the unpredictable hoisting?

Yeah, I haven't had more hoisting issues since upgrading. I was previously using nohoist but getting different results on yarn install with unpredictable errors. Now with yarn berry I use this .yarnrc.yml:

nodeLinker: node-modules

yarnPath: .yarn/releases/yarn-berry.cjs

nmHoistingLimits: workspaces

Dependencies are properly hoisted to each workspace and the result of yarn install is always the same. No more random errors either.

@merceyz
Copy link
Member

merceyz commented Jan 2, 2021

Closing as fixed in v2 by using either the nmHoistingLimits option or PnP

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

@merceyz merceyz closed this as completed Jan 2, 2021
@merceyz merceyz added the fixed-in-modern This issue has been fixed / implemented in Yarn 2+. label Jan 2, 2021
@selrond
Copy link

selrond commented Jan 4, 2021

@merceyz should this be closed thought? From the link you've provided:

Yarn v2 is a very different software from the v1

this repo tracks v1 AFAIK - does it mean there's no chance in fixing this behavior in v1?

@arcanis
Copy link
Member

arcanis commented Jan 4, 2021

From the release blogpost, from almost a year ago:

Yarn 1.22 [...] won't receive further releases from me except when absolutely required to patch vulnerabilities. New features [and general fixes] will be developed exclusively against Yarn 2.

Which can be easily validated:

image

Just like what happens when any open-source project releases a new major, all of our resources have been, and will be, spent on the Modern codebase. As we stated before, Classic will remain exactly as it is, and we strongly recommend upgrading when you have the chance (Why?).

@ianmartorell
Copy link

@merceyz should this be closed thought? From the link you've provided:

Yarn v2 is a very different software from the v1

this repo tracks v1 AFAIK - does it mean there's no chance in fixing this behavior in v1?

From my experience, Berry with node-modules configured as the nodeLinker behaves very much like an improved version of Classic. You don't have to adopt the Plug'n'Play or zero-installs approach if you don't want to / can't. I feel this is not clear enough, and it took me a while to give Berry a go because I thought it wouldn't be directly compatible and I'd have to change my workflow significantly. But ultimately I'm very glad I upgraded.

@morewry
Copy link

morewry commented Aug 5, 2021

I keep running into a version of this with modular devDependencies. It's not that those tools are working poorly with hoisting (though, yeah, they can and have), it's that the yarn install hoisting is inconsistent. I have one package that lists eslint and several eslint plugins and configs. Because it's the only one, there are no version conflicts. Out of ~7 different eslint dependencies, 6 are hoisted. 1 is not. For no reason that I can discern. This causes an error when trying to run eslint.

yarn why v1.22.4
[1/4] 🤔  Why do we have the module "eslint-plugin-prettier"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "eslint-plugin-prettier@3.0.1"
info Reasons this module exists
   - "_project_#@collectivehealth#frontend-member-v2" depends on it
   - Hoisted from "_project_#@collectivehealth#frontend-member-v2#eslint-plugin-prettier"

Hilariously, yarn says it is hoisted when it isn't. I get that I can probably solve this with nohoist in the short term, but I want it to hoist and that would also fix the problem.

I appreciate that fixed in v2 is significant to the yarn team and to others who can take this plunge, but it's really not feasible for our team to adopt bleeding edge pre-release software for so many tools. Because yarn does not exist in a vacuum. The current JS ecosystem is incredibly demanding and in my context it wastes a lot of frontend time wrestling with these tools instead of focusing on user experience as we should. I wish that this weren't the case. But if wishes were horses, we'd all ride, so I am left with my regret that this "me three" comment isn't likely to help anything.

@arcanis
Copy link
Member

arcanis commented Aug 6, 2021

it's really not feasible for our team to adopt bleeding edge pre-release software

Yarn 2.0 got released a year and a half ago. We've had the time to release another major since then. I don't think the "bleeding edge pre-release" moniker is justified 🙂

In any case, hoisting in v1 will certainly not change, especially as it's one of the most complex and prone to break area. There's a reason why we decided it was worth reimplementing it.

@morewry
Copy link

morewry commented Aug 6, 2021

Is it under a different name? Because I checked 3 times and did not see a canonical 2.x release. Only rc pre-releases. If you're not ready to change that, how could I possibly view it as anything other than an unstable pre-release used lightly by early adopters?

Certainly I appreciate that input, but there is a reason I perceive it as I originally said.

@eliw00d
Copy link

eliw00d commented Aug 6, 2021

https://github.com/yarnpkg/berry

@zhang-quan-yi
Copy link

Having the same issue with yarn v1.22.5
My workaround is:

  1. use the same version for one dependency.
  2. add the dependency to the package.json which is in the workspace root directory.

@dimaslanjaka
Copy link

anyone can explain how yarn v3 hoisting works? https://yarnpkg.com/configuration/yarnrc#nmHoistingLimits

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+.
Projects
None yet
Development

No branches or pull requests