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

fix(core): don't set legacy-peer-deps by default #29726

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

simon-abbott
Copy link

Current Behavior

Currently certain actions, notably nx release, run npm with legacy-peer-deps enabled. This is an issue because running with legacy-peer-deps turned when it is normally off (which is the default) causes the installed dependency graph to be different. This then can break other scripts that expect the structure to be the same, including npm ci.

Changing the value of legacy-peer-deps causes npm to install a different graph, which can break builds and the npm ci command. Any users who rely on legacy-peer-deps should already have it set in an .npmrc file, so this should not affect them. It does, however, fix bugs with using nx in repos that don't have legacy-peer-deps enabled, which is the vast majority of them.

Expected Behavior

Running nx commands, especially nx release, shouldn't change the structure of the installed packages at all.

Related Issue(s)

Fixes #22066
Fixes #29537


Note: I was unable to fully test this locally because the e2e target the contribution guide says to run no longer exists, and trying to run e2e-local fails with Error: Jest: Got error running globalSetup - /workspaces/nx/e2e/utils/global-setup.ts, reason: 1 (example run). I tried asking about this error in the Discord, but didn't get a reply, so I'm just posting this PR anyway in the hopes that CI passes.

Copy link

vercel bot commented Jan 23, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nx-dev ✅ Ready (Inspect) Visit Preview Jan 23, 2025 1:05am

Copy link

nx-cloud bot commented Jan 23, 2025

View your CI Pipeline Execution ↗ for commit 3dea7e2.

Command Status Duration Result
nx affected --targets=lint,test,build,e2e,e2e-c... ✅ Succeeded 41m 21s View ↗
nx run-many -t check-imports check-commit check... ✅ Succeeded 1m 1s View ↗
nx-cloud record -- nx format:check --base=12360... ✅ Succeeded 26s View ↗
nx-cloud record -- nx-cloud conformance:check ✅ Succeeded 2s View ↗
nx-cloud record -- nx sync:check ✅ Succeeded 22s View ↗
nx documentation --no-dte ✅ Succeeded 1m 13s View ↗

☁️ Nx Cloud last updated this comment at 2025-01-23 01:44:42 UTC

Changing the value of `legacy-peer-deps` causes npm to install a
different graph, which can break builds and the `npm ci` command. Any
users who rely on `legacy-peer-deps` should already have it set in an
`.npmrc` file, so this should not affect them. It does, however, fix
bugs with using `nx` in repos that _don't_ have `legacy-peer-deps`
enabled, which is the vast majority of them.

Fixes nrwl#22066, fixes nrwl#29537
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant