Skip to content

Commit

Permalink
fix(core): don't set legacy-peer-deps by default
Browse files Browse the repository at this point in the history
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 #22066, fixes #29537
  • Loading branch information
simon-abbott committed Jan 23, 2025
1 parent 123602c commit 3dea7e2
Show file tree
Hide file tree
Showing 18 changed files with 76 additions and 53 deletions.
2 changes: 1 addition & 1 deletion docs/shared/migration/adding-to-existing-project.md
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ jobs:
with:
node-version: 20
cache: 'npm'
- run: npm ci --legacy-peer-deps
- run: npm ci
- uses: nrwl/nx-set-shas@v4
# Nx Affected runs only tasks affected by the changes in this PR/commit. Learn more: https://nx.dev/ci/features/affected
- run: npx nx affected -t lint test build
Expand Down
2 changes: 1 addition & 1 deletion docs/shared/migration/adding-to-monorepo.md
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ jobs:
with:
node-version: 20
cache: 'npm'
- run: npm ci --legacy-peer-deps
- run: npm ci
- uses: nrwl/nx-set-shas@v4
# Nx Affected runs only tasks affected by the changes in this PR/commit. Learn more: https://nx.dev/ci/features/affected
- run: npx nx affected -t lint test build
Expand Down
2 changes: 1 addition & 1 deletion docs/shared/migration/migration-angular.md
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ jobs:
with:
node-version: 20
cache: 'npm'
- run: npm ci --legacy-peer-deps
- run: npm ci
- uses: nrwl/nx-set-shas@v4
# Nx Affected runs only tasks affected by the changes in this PR/commit. Learn more: https://nx.dev/ci/features/affected
- run: npx nx affected -t lint test build
Expand Down
2 changes: 1 addition & 1 deletion docs/shared/tutorials/angular-monorepo.md
Original file line number Diff line number Diff line change
Expand Up @@ -1303,7 +1303,7 @@ jobs:
with:
node-version: 20
cache: 'npm'
- run: npm ci --legacy-peer-deps
- run: npm ci
- uses: nrwl/nx-set-shas@v4
# Nx Affected runs only tasks affected by the changes in this PR/commit. Learn more: https://nx.dev/ci/features/affected
# When you enable task distribution, run the e2e-ci task instead of e2e
Expand Down
2 changes: 1 addition & 1 deletion docs/shared/tutorials/angular-standalone.md
Original file line number Diff line number Diff line change
Expand Up @@ -1104,7 +1104,7 @@ jobs:
with:
node-version: 20
cache: 'npm'
- run: npm ci --legacy-peer-deps
- run: npm ci
- uses: nrwl/nx-set-shas@v4
# Nx Affected runs only tasks affected by the changes in this PR/commit. Learn more: https://nx.dev/ci/features/affected
# When you enable task distribution, run the e2e-ci task instead of e2e
Expand Down
2 changes: 1 addition & 1 deletion docs/shared/tutorials/react-monorepo.md
Original file line number Diff line number Diff line change
Expand Up @@ -1106,7 +1106,7 @@ jobs:
with:
node-version: 20
cache: 'npm'
- run: npm ci --legacy-peer-deps
- run: npm ci
- uses: nrwl/nx-set-shas@v4
# Nx Affected runs only tasks affected by the changes in this PR/commit. Learn more: https://nx.dev/ci/features/affected
# When you enable task distribution, run the e2e-ci task instead of e2e
Expand Down
2 changes: 1 addition & 1 deletion docs/shared/tutorials/react-standalone.md
Original file line number Diff line number Diff line change
Expand Up @@ -1055,7 +1055,7 @@ jobs:
with:
node-version: 20
cache: 'npm'
- run: npm ci --legacy-peer-deps
- run: npm ci
- uses: nrwl/nx-set-shas@v4
# Nx Affected runs only tasks affected by the changes in this PR/commit. Learn more: https://nx.dev/ci/features/affected
# When you enable task distribution, run the e2e-ci task instead of e2e
Expand Down
2 changes: 1 addition & 1 deletion docs/shared/tutorials/typescript-packages.md
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ jobs:
with:
node-version: 20
cache: 'npm'
- run: npm ci --legacy-peer-deps
- run: npm ci
- uses: nrwl/nx-set-shas@v4
# Nx Affected runs only tasks affected by the changes in this PR/commit. Learn more: https://nx.dev/ci/features/affected
# When you enable task distribution, run the e2e-ci task instead of e2e
Expand Down
2 changes: 1 addition & 1 deletion docs/shared/tutorials/vue-standalone.md
Original file line number Diff line number Diff line change
Expand Up @@ -1104,7 +1104,7 @@ jobs:
with:
node-version: 20
cache: 'npm'
- run: npm ci --legacy-peer-deps
- run: npm ci
- uses: nrwl/nx-set-shas@v4
# Nx Affected runs only tasks affected by the changes in this PR/commit. Learn more: https://nx.dev/ci/features/affected
# When you enable task distribution, run the e2e-ci task instead of e2e
Expand Down
38 changes: 38 additions & 0 deletions e2e/release/src/lock-file-updates.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import {
checkFilesExist,
cleanupProject,
newProject,
readJson,
runCLI,
runCommand,
uniq,
Expand Down Expand Up @@ -154,6 +156,42 @@ describe('nx release lock file updates', () => {
`);
});

it('should not mess with peer dependencies when package manager is npm', async () => {
initializeProject('npm');

updateJson('package.json', (json) => {
json.workspaces = [pkg1, pkg2, pkg3];
return json;
});

updateJson(`${pkg1}/package.json`, (json) => {
json.peerDependencies = {
semver: '^7.3.2',
};
return json;
});

runCommand(`npm install`);

// Make sure that the peer dependency was installed, and that the lock file
// is up-to-date, otherwise we are testing nothing.
checkFilesExist('node_modules/semver/package.json');
runCommand('npm ci', { failOnError: true });

// workaround for NXC-143
runCLI('reset');

runCommand(`git add .`);
runCommand(`git commit -m "chore: initial commit"`);

runCLI(`release version 999.9.9`);

// Make sure that the lock file is still valid, and that the peer dependency
// was not removed.
runCommand('npm ci', { failOnError: true });
checkFilesExist('node_modules/semver/package.json');
});

it('should not update lock file when package manager is yarn classic', async () => {
initializeProject('yarn');

Expand Down
4 changes: 2 additions & 2 deletions e2e/utils/command-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,8 @@ export function getPackageManagerCommand({
runUninstalledPackage: `npx --yes`,
install: 'npm install',
ciInstall: 'npm ci',
addProd: `npm install --legacy-peer-deps`,
addDev: `npm install --legacy-peer-deps -D`,
addProd: `npm install`,
addDev: `npm install -D`,
list: 'npm ls --depth 10',
runLerna: `npx lerna`,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ export function addNgRxToPackageJson(
: '~0.8.3';
const ngrxVersion = versions(tree).ngrxVersion;

process.env.npm_config_legacy_peer_deps ??= 'true';

return addDependenciesToPackageJson(
tree,
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ export function addNgRxToPackageJson(
: '~0.8.3';
const ngrxVersion = versions(tree).ngrxVersion;

process.env.npm_config_legacy_peer_deps ??= 'true';

return addDependenciesToPackageJson(
tree,
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ export function addNgRxToPackageJson(
: '~0.8.3';
const ngrxVersion = versions(tree).ngrxVersion;

process.env.npm_config_legacy_peer_deps ??= 'true';

return addDependenciesToPackageJson(
tree,
{
Expand Down
2 changes: 0 additions & 2 deletions packages/angular/src/generators/utils/add-jest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ export async function addJest(
options: AddJestOptions
): Promise<void> {
if (!options.skipPackageJson) {
process.env.npm_config_legacy_peer_deps ??= 'true';

addDependenciesToPackageJson(
tree,
{},
Expand Down
4 changes: 0 additions & 4 deletions packages/nx/src/command-line/migrate/migrate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1382,10 +1382,6 @@ function showConnectToCloudMessage() {
function runInstall() {
const pmCommands = getPackageManagerCommand();

// TODO: remove this
if (detectPackageManager() === 'npm') {
process.env.npm_config_legacy_peer_deps ??= 'true';
}
output.log({
title: `Running '${pmCommands.install}' to make sure necessary packages are installed`,
});
Expand Down
5 changes: 1 addition & 4 deletions packages/nx/src/utils/package-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,12 +166,9 @@ export function getPackageManagerCommand(
};
},
npm: () => {
// TODO: Remove this
process.env.npm_config_legacy_peer_deps ??= 'true';

return {
install: 'npm install',
ciInstall: 'npm ci --legacy-peer-deps',
ciInstall: 'npm ci',
updateLockFile: 'npm install --package-lock-only',
add: 'npm install',
addDev: 'npm install -D',
Expand Down
Loading

0 comments on commit 3dea7e2

Please sign in to comment.