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

[Undefined behavior]: peerDependencies prevents correct node_modules linking in workspaces #6558

Open
1 task done
GauBen opened this issue Oct 18, 2024 · 10 comments
Open
1 task done
Labels
bug Something isn't working

Comments

@GauBen
Copy link
Contributor

GauBen commented Oct 18, 2024

Self-service

  • I'd be willing to implement a fix

Describe the bug

With this monorepo structure

/
+-- package.json        # "name": "hello"
+-- index.js
\-- demo/
    +-- package.json    # "name:" "demo"
    \-- index.json

If demo/package.json declares a dependency on hello and nodeLinker: node-modules is set, the directory should look like this:

/
+-- package.json
+-- index.js
\-- demo/
    +-- node_modules/
    |   \-- hello        --> /
    +-- package.json
    \-- index.json

This is true unless hello declares a peer dependency!

If it declares a peer dependency, e.g.

{
  "peerDependencies": {
    "whatever": "*"
  },
  "peerDependenciesMeta": {
    "whatever": {
      "optional": true
    }
  }
}

Then /demo/node_modules/hello is no longer created

To reproduce

git clone https://github.com/GauBen/yarn-issue
cd yarn-issue
yarn
cd demo && yarn start
# Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'hello'

Removing peerDependencies and peerDependenciesMeta from the root package.json will fix the issue

Environment

System:
OS: Linux 5.15 Ubuntu 22.04.5 LTS 22.04.5 LTS (Jammy Jellyfish)
CPU: (12) x64 Intel(R) Core(TM) i7-10750H CPU @ 2.60GHz
Binaries:
Node: 22.9.0 - /tmp/xfs-73d42128/node
Yarn: 4.5.0 - /tmp/xfs-73d42128/yarn
npm: 10.8.3 - ~/.pkgx/npmjs.com/v10.8.3/bin/npm

Additional context

Reproduction logs: https://github.com/GauBen/yarn-issue/actions/runs/11400676401/job/31722024945#step:5:22

@GauBen GauBen added the bug Something isn't working label Oct 18, 2024
@GauBen
Copy link
Contributor Author

GauBen commented Oct 18, 2024

I don't know how to fix this, but with a few hints I should be able to write a patch and a test

@larixer
Copy link
Member

larixer commented Oct 18, 2024

If demo/package.json declares a dependency on hello and nodeLinker: node-modules is set, the directory should look like this:

It would create circular symlink, hence the behavior you expect is problematic.

@GauBen
Copy link
Contributor Author

GauBen commented Oct 18, 2024

It works fine if peerDependencies gets removed: https://github.com/GauBen/yarn-issue/actions/runs/11401008393/job/31723033705?pr=1#step:5:18

There is indeed a circular symlink, but it does not seem to be an issue

@larixer
Copy link
Member

larixer commented Oct 18, 2024

There is indeed a circular symlink, but it does not seem to be an issue

The fact it is not an issue for you, doesn't mean it is not an issue for someone else. You are in a grey zone when you depend on the root workspace with node-moidules linker, the behavior is not guaranteed and there is no guarantee it will not break at some moment. This is a circular dependency between workspaces, we do support circular dependencies for normal dependencies, but for workspaces the behavior is not guaranteed.

Example of issues with circular symlinks (that are unavoidable in case you depend on root workspace) are in commens to the issue below:
#3256

@GauBen
Copy link
Contributor Author

GauBen commented Oct 18, 2024

Ok I get it, but is it a bug that declaring or removing peerDependencies changes the resulting node_modules?

the behavior is not guaranteed and there is no guarantee it will not break at some moment

Yarn should emit a warning in this case instead of failing silently as it is the case in the reproduction, if that's the road you're willing to take

@larixer
Copy link
Member

larixer commented Oct 18, 2024

Ok I get it, but is it a bug that declaring or removing peerDependencies changes the resulting node_modules?

Well, it can be viewed as a bug, and we can try to solve it. Still the layout of project you use is problematic and you might run in issues with this layout and then change it to non-circular one. And our efforts to fix a "bug" will be useless, that's my point. We can still fix this "bug" for the sake of some correctness in undefined situations, but it is a rabbit hole

@GauBen
Copy link
Contributor Author

GauBen commented Oct 18, 2024

Ok I'd love to make the fact that this behavior is undefined clearer for future users, is a CLI warning the right way to do it?

I'd start with a simple foreach packages, if is_workspace_dependency && is_workspace_root then warn, would that make the cut?

Edit: just tested, there is no such issue with nodeLinker: pnp, the heuristic should only trigger in the node-modules linker, not during resolution

@GauBen
Copy link
Contributor Author

GauBen commented Oct 18, 2024

I'm starting to investigate the rabbit hole: it also works with nodeLinker: pnpm. I guess this will be my work around for the time being. Thanks for your quick responses!

@GauBen
Copy link
Contributor Author

GauBen commented Oct 18, 2024

Unrelated issue: declaring a nodeLinker that does not exist does not trigger an error, nodeLinker: ghjklm will fail silently

@larixer
Copy link
Member

larixer commented Oct 18, 2024

Yeah, non-existing linker worth fixing, might be not a trivial fix though, due to how plugins work

@GauBen GauBen changed the title [Bug?]: peerDependencies prevents correct node_modules linking in workspaces [Undefined behavior]: peerDependencies prevents correct node_modules linking in workspaces Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants