[Bug]: nodeLinker: node-modules with nmHoistingLimits: workspaces does not create symlink when inner workspace depends on outer workspace

[trusktr]

Self-service

• I'd be willing to implement a fix

Describe the bug

I have this yarnrc:

nodeLinker: node-modules
nmHoistingLimits: workspaces

I have these workspaces where I'm trying to have the root of the super module also be a package (all other packages are git submodules):

	"workspaces": [
		".", // <----- the top level of the repo is a package too
		"apps/*",
		"packages/!(glas|docsifyjs+docsify)"
	],

The structure is like this:

main-package/
  apps/
    docs/

The docs package depends on the main-package which is the root of the repo, and docs has main-package listed in its package.json, but docs will not get main-package linked into its node_modules by Yarn.

Is this expected that the root being also a package does not get linked to other packages? I'm wondering if this has to do with trying to use the top level of the repo as a package.

In Yarn Classic, this was not allowed, and it would require private: true or throw an error, but Yarn Berry is not giving any errors regarding this, so I assume it should work.

Workaround

As a workaround, I have this script in the root package.json:

		"link": "yarn install && symlink-dir . apps/docs/node_modules/main-package",

using symlink-dir, that I call with npm run link (I'm still using npm for most things, but delegating to yarn only for the purpose of installing/boostrapping everything as an alternative to using Lerna for that specific purpose).

To reproduce

TODO: I'll make one if confirmed this should be working.

If it is expected, I'd like to make a feature request.

Environment

System:
    OS: macOS 13.4
    CPU: (8) arm64 Apple M2
  Binaries:
    Node: 20.6.1 - /private/var/folders/7y/wy4mhdj114g5xj1ktvh6hdz80000gn/T/xfs-9bb50836/node
    Yarn: 3.6.4 - /private/var/folders/7y/wy4mhdj114g5xj1ktvh6hdz80000gn/T/xfs-9bb50836/yarn
    npm: 10.2.0 - ~/.npm-packages/bin/npm
    pnpm: 7.24.3 - ~/.npm-packages/bin/pnpm

Additional context

No response

[larixer]

This is a known issue with node_modules installation layout. If we will make a symlink to the top-level workspace, this will create a cyclic symlink, which will make many tools refusing to work properly and will lead to many users posting issues here that Yarn generated dangerous install layout. Please see my explanations and links in the second paragraph of this PR:
#4571

Perhaps I'm overly protecting users here, maybe Yarn should generate a warning and still proceed if explicitly requested to create cyclic symlinks...

[trusktr]

Agreed, cyclic links can cause problems (some people might still want them, and an option could be nice), but in my example main-package does not depend on docs so there would not be circular links in this case.

[larixer]

but in my example main-package does not depend on docs so there would not be circular links in this case.

The main-package/apps/docs/node_modules/main-package will point to main-package. This is a circular link, since the link points to the directory which contains the link itself in one of its subdirectories, which will make the tool traversing the directories that has no logic to detect cyclic links to loop forever.

[trusktr]

I see what you mean. I haven't had an issue with that one-way dependency link, and have been working this way for years already. I think it works fine because the access of the links is one-way with all tools I've used (in this repo).

One time I had a problem in the past with a circular dependency between both root and sub, with links like so:

main-package/pkgs/foo/node_modules/main-package --> main-package/
main-package/node_modules/foo --> main-package/pkgs/foo/

I think its the linked circular dependencies that really do throw in a wrench.

Anywho, I've finished migrating from Lerna to Yarn just now for bootstrapping, with the applied workaround, and loving the new speed compared to Lerna! Also really love the install output which is much more helpful than NPM's, and love that it doesn't fail to install upon failed build of some package so at least everything else is in place and actions that don't require the failed package can still be taken subsequently.

EDIT: Oh, I love how fast the node-modules install is compared to PnP install too (without initial lock files)! Lucky I'm in that format (useful for import maps for web app). EDIT: Could be I didn't have some home folder cache at first, not sure. Anyway I'm really liking the migration. Thanks for this!

[trusktr]

Should I close this and open a feature request?

[larixer]

@trusktr

Should I close this and open a feature request?

I don't think it matters, this issue is known.

[trusktr]

What's the chance of it being accepted as a feature?

The reason I landed here is I migrated from Lerna to Yarn (Lerna v7 removed bootstrap+linking, they now tell users to use yarn/npm/pnpm for that). This was built into Lerna before v7, and looking achieve the same with Yarn.

[larixer]

@trusktr What exactly should be accepted as a feature? There is more than one way to solve this issue.

[trusktr]

Just linking the parent workspace to child workspaces, and vice versa, nothing more than that.

[larixer]

@trusktr

It sounds like "Just make it fast" (c), it's not going to work if you want some change in the project at very least you should come up with well thought analysis of your proposition and its alternatives, their outcomes and weaknesses, so that we can start discussion around something.

[trusktr]

Hmm, yeah, I'm completely new to Yarn, as an end user. I don't know what implementation to propose.

The downsides are clear: some users' tooling may fail when they encounter symbolic links. However, I believe this should be something that the end user must discover, and they must decide if they keep using that tool or not. I do not think Yarn can make this decision.

---

All I can say from my limited experience as end user is that in my package.json I have something like this:

	"workspaces": [
		"apps/*",
		// ...
	],
	"scripts": {
		"link": "yarn install && symlink-dir . apps/docs/node_modules/main-package && symlink-dir . apps/first-person-shooter/node_modules/main-package && ...",
	}

and contributors to the repo will need to run the link script to bootstrap. If a newcomer assumes they can simply run yarn install and things will work, they'll be wrong.

It's not the end of the world, I can make sure my README clearly states to run npm run link instead of yarn install.

---

Is there a life cycle script similar to postinstall so I can simply make yarn install work well without it also running when the package is installed as a dependency outside of the repo?

[trusktr]

Just for reference, I had this in lerna.json before migrating:

{
	"packages": [".", "apps/*", "packages/*"],
}

Basically I'm re-creating that same setup in Yarn.

Lerna simply treats any packages, regardless of nesting, as a package, and links them (if semver compatible).

With regards to circular dependency problems, the same issues (with 3rd party tools) would likely exist if all packages were in sibling folders but depended on each other circularly. I think that the parent-child nesting does not have much impact on that, circular links are circular links regardless.

With Lerna (after linking with Yarn with above workaround) I can currently do this:

yarn lerna run test --scope lume --include-dependencies

where lume is the top-level workspace (and a package itself). This runs tests in topological order by default, with concurrency when possible.

[trusktr]

UPDATE:

I discovered that, even without specifying the top-level workspace/package in workspaces, yarn still recognizes it by name!

I can successfully run this, to the same effect as the previous lerna run test command:

yarn workspaces foreach -Rtp --from lume run test

[arcanis]

Yes, that was pointed out in #5799 (comment). We perhaps should add an error if people try to do this.

[trusktr]

Yes, that was pointed out in #5799 (comment).

That comment didn't mention that I can run scripts in the top level workspace by its name though (without listing it in workspaces). That's the nifty thing I'm pointing out.

We perhaps should add an error if people try to do this.

Yeah, perhaps throwing when the top level is listed in workspaces would be good, until symlinking is added for the top level package.

But please do keep the ability to run the top level scripts given its name in workspaces foreach. If that's removed, I'll need to make another workaround.

Developing at at the top level, and splitting its details into re-usable packages as sub-workspaces in the same repo with the main library staying at the top, is totally a good use case.

[larixer]

@trusktr I'v submitted a fix for this issue in #5913, it waits for review from the Yarn team. You can however try it and provide feedback already by getting Yarn version from this PR via:
yarn set version from sources --branch 5913

[trusktr]

Confirmed, it works! 🙌