Skip to content

fix(pnpm): pnpm linker no longer removes node_modules when nm linker is active #4206

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

Merged
merged 2 commits into from
Mar 15, 2022

Conversation

larixer
Copy link
Member

@larixer larixer commented Mar 10, 2022

What's the problem this PR addresses?

The pnpm linker attempts to delete node_modules directory all the time, this causes install failure when Yarn has no permissions. This problem happens even when node-modules linker is active.

Fixes #4172
Fixes #4160

How did you fix it?

Based on the evaluation of pnpm linker finalizeInstall code, seems this behavior - to manipulate node_modules directory- was not intended. I have updated it so that this code was not run.

Checklist

  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

@larixer larixer changed the title Fix: pnpm linker no longer removes node_modules when nm linker is active fix: pnpm linker no longer removes node_modules when nm linker is active Mar 10, 2022
@larixer larixer changed the title fix: pnpm linker no longer removes node_modules when nm linker is active fix(pnpm): pnpm linker no longer removes node_modules when nm linker is active Mar 10, 2022
await this.asyncActions.wait(),
await this.asyncActions.wait();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this the reason for the flaky test? 🤯

Copy link
Member

@arcanis arcanis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like it'd be better if the pnpm linker could have no knowledge of other linkers, but that would require a better cleaning mechanism 🤔 fine to merge as is

@arcanis arcanis merged commit 27e3886 into master Mar 15, 2022
@arcanis arcanis deleted the larixer/pnpm-linker-nm-removal-fix branch March 15, 2022 13:53
dehy added a commit to dehy/crowd-news that referenced this pull request Apr 15, 2022
merceyz pushed a commit that referenced this pull request May 12, 2022
…is active (#4206)

* Fix: pnpm linker no longer removes node_modules when nm linker is active

* Updates artifact cleanup code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants