Berry: [Bug] nohoist does not work for transitive dependancies

[1st1]

• [ ] I'd be willing to implement a fix

Describe the bug

We have a monorepo with two applications: one is a CRA app, another is a nextjs app.

The CRA app is not ejected and transitively depends on [email protected]. The nextjs app transitively depends on [email protected].

Here's the top-level package.json:

{
  "private": true,
  "workspaces": {
    "packages": [
      "apps/*",
      "common"
    ],
    "nohoist": [
      "**/electron",
      "**/electron/**",
      "**/electron-builder",
      "**/electron-builder/**",
      "**/webpack",
      "**/webpack/**",
      "**/react-scripts",
      "**/react-scripts/**"
    ]
  }
}

And here's the output of yarn why webpack:

{edgedb} ~/d/e/t/a/studio (monorepo *%) » yarn why webpack
├─ next@npm:9.4.0
│  └─ webpack@npm:4.43.0 (via npm:4.43.0)
│
├─ next@npm:9.4.0 [88fd1]
│  └─ webpack@npm:4.43.0 (via npm:4.43.0)
│
├─ react-scripts@npm:3.4.1
│  └─ webpack@npm:4.42.0 (via npm:4.42.0)
│
└─ react-scripts@npm:3.4.1 [f2665]
   └─ webpack@npm:4.42.0 (via npm:4.42.0)

The actual installed webpack is 4.43.0, so CRA naturally complains with

{edgedb} ~/d/e/t/a/studio (monorepo *%) » yarn electron-dev
[0]
[0] There might be a problem with the project dependency tree.
[0] It is likely not a bug in Create React App, but something you need to fix locally.
[0]
[0] The react-scripts package provided by Create React App requires a dependency:
[0]
[0]   "webpack": "4.42.0"
[0]
[0] Don't try to install it manually: your package manager does it automatically.
[0] However, a different version of webpack was detected higher up in the tree:
[0]
[0]   /Users/yury/dev/edge/tutorial/node_modules/webpack (version: 4.43.0)

To Reproduce

If this looks like a bug to yarn maintainers I can create a repo that exhibits the bug to help debugging.

Additional context

• yarn version: 2.0.0-rc.33

• Everything works as expected in yarn 1

[larixer]

@1st1 Yarn 2 does not implement nohoist, we do not know of use cases where it might be really needed, nohoist feature was added into Yarn 1 to support old broken React Native versions and is no longer needed for newer React Native versions.

Maybe the problem here is not related to nohoist though? We need a reproduction to investigate.

[1st1]

I found the problem: it's the incorrectly implemented check in
https://github.com/facebook/create-react-app/blob/a0b3753476053ed60159fb209f974b83e2aad7f9/packages/react-scripts/scripts/utils/verifyPackageTree.js#L54-L65 that tries to replicate node import mechanism to verify that package versions are what the tool expects.

Thanks for replying promptly!

@1st1 Yarn 2 does not implement nohoist, we do not know of use cases where it might be really needed, nohoist feature was added into Yarn 1 to support old broken React Native versions and is no longer needed for newer React Native versions.

It would be great if this was better and explicitly documented.

[darthtrevino]

So just to follow up on this, nohoist is useful when you're defining serverless functions and want to create zip archives that include their dependencies.

It's useful in a workspace that's using create-react-app because CRA enforces that you can't resolve modules outside of root.

Honestly nohoisting everything seems like the sane, safe default when using the node-modules linker and Yarn v2. Individual modules are only require-able from where they are needed.

[darthtrevino]

IIRC, the Yarn v1 introduced hoisting to improve installation speed, but at the cost of a little safety. Yarn v2's goals seem more aligned with uncompromised safety first.

[juuyan]

So just to follow up on this, nohoist is useful when you're defining serverless functions and want to create zip archives that include their dependencies.

It's useful in a workspace that's using create-react-app because CRA enforces that you can't resolve modules outside of root.

Honestly nohoisting everything seems like the sane, safe default when using the node-modules linker and Yarn v2. Individual modules are only require-able from where they are needed.

@darthtrevino The serverless function use case is exactly what I am facing. How could I solve it properly?

[merceyz]

@juuyan Use nmHoistingLimits
https://yarnpkg.com/configuration/yarnrc#nmHoistingLimits
https://yarnpkg.com/getting-started/migration/#replace-nohoist-by-nmhoistinglimits

[juuyan]

@merceyz
Hi thx I have tried it:

// root/serverless-workspace/package.json
...
"installConfig": {
    "hoistingLimits": "workspaces"
  }

But yarn command did not bring me the local node-module.
yarn ver.: 2.3.3

---

It seems

• I should config nodeLinker: node-modules
• Local node-modules would come yet the whole project would stop using pnp...
• If I want to use pnp in most workspaces while keep nodemodules in sls workspace, I have to set it as a separated project

Am I right?

[merceyz]

hoistingLimit is only for the node-modules linker, if you want something to isolate a workspace for PnP use https://gitlab.com/Larry1123/yarn-contrib/-/tree/master/packages/plugin-production-install by @Larry1123

[darthtrevino]

We ended up running webpack over our Azure Function code. This ended up kind of negating the problem of needing node_modules and improved our uploads & storage use.

[merceyz]

That would be the preferred way to go about it for those exact reasons