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

[Bug?]: node_modules hoisting issue with inner workspaces #6494

Closed
1 task
iffa opened this issue Sep 12, 2024 · 0 comments · Fixed by #6495
Closed
1 task

[Bug?]: node_modules hoisting issue with inner workspaces #6494

iffa opened this issue Sep 12, 2024 · 0 comments · Fixed by #6495
Assignees
Labels
bug Something isn't working node-modules

Comments

@iffa
Copy link

iffa commented Sep 12, 2024

Self-service

  • I'd be willing to implement a fix

Describe the bug

Discord ref: https://discord.com/channels/226791405589233664/1283003259937755148
Not related to #6493 but found at the same time as part of the same discussion.

Example monorepo setup:

  • root
    • mobile
      • Is an inner workspace (has workspaces: [] in package.json)
      • contains n packages
    • web
      • is an inner workspace too
      • contains some web apps, packages etc. all with a matching react, react-query version

The shared dependencies in the web inner workspace should have been hoisted to web, but some dependencies are left as multiple instances in the packages instead (see attached screenshot & reproduction).

To reproduce

Reproduction: https://github.com/iffa/yarn-hoisting-repro/tree/b77e8e2446a62325c847a9595153a1402c9fed30
(Important to view this specific commit SHA)

yarn install and observe node_modules directories in the repo

Environment

System:
    OS: macOS 14.5
    CPU: (10) arm64 Apple M1 Pro
  Binaries:
    Node: 20.17.0 - /private/var/folders/00/m6b9k2y13rvgz5dn7y7_dtx00000gn/T/xfs-6c80b460/node
    Yarn: 4.4.1 - /private/var/folders/00/m6b9k2y13rvgz5dn7y7_dtx00000gn/T/xfs-6c80b460/yarn
    npm: 10.8.2 - ~/.nvm/versions/node/v20.17.0/bin/npm

Additional context

Screenshot 2024-09-12 at 12 17 18
@iffa iffa added the bug Something isn't working label Sep 12, 2024
@larixer larixer self-assigned this Sep 12, 2024
github-merge-queue bot pushed a commit that referenced this issue Sep 12, 2024
## What's the problem this PR addresses?

<!-- Describe the rationale of your PR. -->
<!-- Link all issues that it closes. (Closes/Resolves #xxxx.) -->

Fixes #6493
Fixes #6494

## How did you fix it?

<!-- A detailed description of your implementation. -->

Issue: #6493. Sometimes hoisting algorithm was stopped too early,
because it wasn't able to determine correctly stop condition. I made it
safer but possibly one round slower by stopping only when nothing was
hoisted during the last round.

Issue: #6494. Hoisting avoided hoisting to non-root workspace
previously, I have removed this limitation, because it made inner
workspaces meaningless.

## Checklist

<!--- Don't worry if you miss something, chores are automatically
tested. -->
<!--- This checklist exists to help you remember doing the chores when
you submit a PR. -->
<!--- Put an `x` in all the boxes that apply. -->
- [x] I have read the [Contributing
Guide](https://yarnpkg.com/advanced/contributing).

<!-- See
https://yarnpkg.com/advanced/contributing#preparing-your-pr-to-be-released
for more details. -->
<!-- Check with `yarn version check` and fix with `yarn version check
-i` -->
- [x] I have set the packages that need to be released for my changes to
be effective.

<!-- The "Testing chores" workflow validates that your PR follows our
guidelines. -->
<!-- If it doesn't pass, click on it to see details as to what your PR
might be missing. -->
- [x] I will check that all automated PR checks pass before the PR gets
reviewed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working node-modules
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants