From 038065e10ce906bb3d2c15793b64f0d2dd4e6dc0 Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Fri, 1 Sep 2023 10:37:45 +0000 Subject: [PATCH] refactor(@angular/cli): remove legacy NPM compatibility check This check should no longer be needed as on supported versions of Node.js NPM 7.5 is no longer installed by default. --- packages/angular/cli/src/commands/add/cli.ts | 1 - packages/angular/cli/src/commands/new/cli.ts | 9 -- .../angular/cli/src/commands/update/cli.ts | 2 - .../cli/src/utilities/package-manager.ts | 27 ----- .../tests/misc/dedupe-duplicate-modules.ts | 79 +++++++------ tests/legacy-cli/e2e/tests/misc/npm-7.ts | 110 ------------------ 6 files changed, 43 insertions(+), 185 deletions(-) delete mode 100644 tests/legacy-cli/e2e/tests/misc/npm-7.ts diff --git a/packages/angular/cli/src/commands/add/cli.ts b/packages/angular/cli/src/commands/add/cli.ts index f90b25b508c9..05827c861403 100644 --- a/packages/angular/cli/src/commands/add/cli.ts +++ b/packages/angular/cli/src/commands/add/cli.ts @@ -112,7 +112,6 @@ export default class AddCommadModule async run(options: Options & OtherOptions): Promise { const { logger, packageManager } = this.context; const { verbose, registry, collection, skipConfirmation } = options; - packageManager.ensureCompatibility(); let packageIdentifier; try { diff --git a/packages/angular/cli/src/commands/new/cli.ts b/packages/angular/cli/src/commands/new/cli.ts index 202dd491bb3c..caa8801fe980 100644 --- a/packages/angular/cli/src/commands/new/cli.ts +++ b/packages/angular/cli/src/commands/new/cli.ts @@ -74,15 +74,6 @@ export default class NewCommandModule }); workflow.registry.addSmartDefaultProvider('ng-cli-version', () => VERSION.full); - // Compatibility check for NPM 7 - if ( - collectionName === '@schematics/angular' && - !schematicOptions.skipInstall && - (schematicOptions.packageManager === undefined || schematicOptions.packageManager === 'npm') - ) { - this.context.packageManager.ensureCompatibility(); - } - return this.runSchematic({ collectionName, schematicName: this.schematicName, diff --git a/packages/angular/cli/src/commands/update/cli.ts b/packages/angular/cli/src/commands/update/cli.ts index 5b9f45e85c32..906537aa08c5 100644 --- a/packages/angular/cli/src/commands/update/cli.ts +++ b/packages/angular/cli/src/commands/update/cli.ts @@ -178,8 +178,6 @@ export default class UpdateCommandModule extends CommandModule): Promise { const { logger, packageManager } = this.context; - packageManager.ensureCompatibility(); - // Check if the current installed CLI version is older than the latest compatible version. // Skip when running `ng update` without a package name as this will not trigger an actual update. if (!disableVersionCheck && options.packages?.length) { diff --git a/packages/angular/cli/src/utilities/package-manager.ts b/packages/angular/cli/src/utilities/package-manager.ts index 95799efd8747..2290960d0fe5 100644 --- a/packages/angular/cli/src/utilities/package-manager.ts +++ b/packages/angular/cli/src/utilities/package-manager.ts @@ -11,7 +11,6 @@ import { execSync, spawn } from 'child_process'; import { existsSync, promises as fs, realpathSync, rmSync } from 'fs'; import { tmpdir } from 'os'; import { join } from 'path'; -import { satisfies, valid } from 'semver'; import { PackageManager } from '../../lib/config/workspace-schema'; import { AngularWorkspace, getProjectByCwd } from './config'; import { memoize } from './memoize'; @@ -44,32 +43,6 @@ export class PackageManagerUtils { return this.getVersion(this.name); } - /** - * Checks if the package manager is supported. If not, display a warning. - */ - ensureCompatibility(): void { - if (this.name !== PackageManager.Npm) { - return; - } - - try { - const version = valid(this.version); - if (!version) { - return; - } - - if (satisfies(version, '>=7 <7.5.6')) { - // eslint-disable-next-line no-console - console.warn( - `npm version ${version} detected.` + - ' When using npm 7 with the Angular CLI, npm version 7.5.6 or higher is recommended.', - ); - } - } catch { - // npm is not installed. - } - } - /** Install a single package. */ async install( packageName: string, diff --git a/tests/legacy-cli/e2e/tests/misc/dedupe-duplicate-modules.ts b/tests/legacy-cli/e2e/tests/misc/dedupe-duplicate-modules.ts index f8a7927662e6..5258f8d8eeb7 100644 --- a/tests/legacy-cli/e2e/tests/misc/dedupe-duplicate-modules.ts +++ b/tests/legacy-cli/e2e/tests/misc/dedupe-duplicate-modules.ts @@ -1,25 +1,27 @@ -import { expectFileToMatch, writeFile } from '../../utils/fs'; +import { expectFileToMatch, rimraf, writeFile } from '../../utils/fs'; +import { gitClean } from '../../utils/git'; import { installWorkspacePackages } from '../../utils/packages'; import { ng } from '../../utils/process'; import { updateJsonFile } from '../../utils/project'; import { expectToFail } from '../../utils/utils'; export default async function () { - // Force duplicate modules - await updateJsonFile('package.json', (json) => { - json.dependencies = { - ...json.dependencies, - 'tslib': '2.0.0', - 'tslib-1': 'npm:tslib@1.13.0', - 'tslib-1-copy': 'npm:tslib@1.13.0', - }; - }); + try { + // Force duplicate modules + await updateJsonFile('package.json', (json) => { + json.dependencies = { + ...json.dependencies, + 'tslib': '2.0.0', + 'tslib-1': 'npm:tslib@1.13.0', + 'tslib-1-copy': 'npm:tslib@1.13.0', + }; + }); - await installWorkspacePackages(); + await installWorkspacePackages(); - await writeFile( - './src/main.ts', - ` + await writeFile( + './src/main.ts', + ` import { __assign as __assign_0 } from 'tslib'; import { __assign as __assign_1 } from 'tslib-1'; import { __assign as __assign_2 } from 'tslib-1-copy'; @@ -30,29 +32,34 @@ export default async function () { __assign_2, }) `, - ); - - const { stderr } = await ng( - 'build', - '--verbose', - '--no-vendor-chunk', - '--no-progress', - '--configuration=development', - ); - const outFile = 'dist/test-project/main.js'; + ); - if (/\[DedupeModuleResolvePlugin\]:.+tslib-1-copy.+ -> .+tslib-1.+/.test(stderr)) { - await expectFileToMatch(outFile, './node_modules/tslib-1/tslib.es6.js'); - await expectToFail(() => - expectFileToMatch(outFile, './node_modules/tslib-1-copy/tslib.es6.js'), + const { stderr } = await ng( + 'build', + '--verbose', + '--no-vendor-chunk', + '--no-progress', + '--configuration=development', ); - } else if (/\[DedupeModuleResolvePlugin\]:.+tslib-1.+ -> .+tslib-1-copy.+/.test(stderr)) { - await expectFileToMatch(outFile, './node_modules/tslib-1-copy/tslib.es6.js'); - await expectToFail(() => expectFileToMatch(outFile, './node_modules/tslib-1/tslib.es6.js')); - } else { - console.error(`\n\n\n${stderr}\n\n\n`); - throw new Error('Expected stderr to contain [DedupeModuleResolvePlugin] log for tslib.'); - } + const outFile = 'dist/test-project/main.js'; - await expectFileToMatch(outFile, './node_modules/tslib/tslib.es6.js'); + if (/\[DedupeModuleResolvePlugin\]:.+tslib-1-copy.+ -> .+tslib-1.+/.test(stderr)) { + await expectFileToMatch(outFile, './node_modules/tslib-1/tslib.es6.js'); + await expectToFail(() => + expectFileToMatch(outFile, './node_modules/tslib-1-copy/tslib.es6.js'), + ); + } else if (/\[DedupeModuleResolvePlugin\]:.+tslib-1.+ -> .+tslib-1-copy.+/.test(stderr)) { + await expectFileToMatch(outFile, './node_modules/tslib-1-copy/tslib.es6.js'); + await expectToFail(() => expectFileToMatch(outFile, './node_modules/tslib-1/tslib.es6.js')); + } else { + console.error(`\n\n\n${stderr}\n\n\n`); + throw new Error('Expected stderr to contain [DedupeModuleResolvePlugin] log for tslib.'); + } + + await expectFileToMatch(outFile, './node_modules/tslib/tslib.es6.js'); + } finally { + await rimraf('node_modules/tslib'); + await gitClean(); + await installWorkspacePackages(); + } } diff --git a/tests/legacy-cli/e2e/tests/misc/npm-7.ts b/tests/legacy-cli/e2e/tests/misc/npm-7.ts deleted file mode 100644 index deabdc21270a..000000000000 --- a/tests/legacy-cli/e2e/tests/misc/npm-7.ts +++ /dev/null @@ -1,110 +0,0 @@ -import * as assert from 'assert'; -import { valid as validSemVer } from 'semver'; -import { rimraf } from '../../utils/fs'; -import { getActivePackageManager } from '../../utils/packages'; -import { execWithEnv, ng, npm } from '../../utils/process'; -import { isPrereleaseCli } from '../../utils/project'; -import { expectToFail } from '../../utils/utils'; - -const warningText = 'npm version 7.5.6 or higher is recommended'; - -export default async function () { - // Only relevant with npm as a package manager - if (getActivePackageManager() !== 'npm') { - return; - } - - // Windows CI fails with permission errors when trying to replace npm - if (process.platform.startsWith('win')) { - return; - } - - // Get current package manager version to restore after tests - const initialVersionText = (await npm('--version')).stdout.trim(); - const initialVersion = validSemVer(initialVersionText); - assert.ok( - initialVersion, - `Invalid npm version string returned from "npm --version" [${initialVersionText}]`, - ); - - const currentDirectory = process.cwd(); - - const extraArgs: string[] = []; - if (isPrereleaseCli()) { - extraArgs.push('--next'); - } - - try { - // Install version >=7.5.6 - await npm('install', '--global', 'npm@>=7.5.6'); - - // Ensure `ng update` does not show npm warning - const { stderr: stderrUpdate1 } = await ng('update', ...extraArgs); - if (stderrUpdate1.includes(warningText)) { - throw new Error('ng update expected to not show npm version warning.'); - } - - // Install version <7.5.6 - await npm('install', '--global', 'npm@7.4.0'); - - // Ensure `ng add` shows npm warning - const { stderr: stderrAdd } = await execWithEnv( - 'ng', - ['add', '@angular/localize', '--skip-confirmation'], - { ...process.env, 'NPM_CONFIG_legacy_peer_deps': 'true' }, - ); - - if (!stderrAdd.includes(warningText)) { - throw new Error('ng add expected to show npm version warning.'); - } - - // Ensure `ng update` shows npm warning - const { stderr: stderrUpdate2 } = await ng('update', ...extraArgs); - if (!stderrUpdate2.includes(warningText)) { - throw new Error('ng update expected to show npm version warning.'); - } - - // Ensure `ng build` executes successfully - const { stderr: stderrBuild } = await ng('build', '--configuration=development'); - if (stderrBuild.includes(warningText)) { - throw new Error('ng build expected to not show npm version warning.'); - } - - // Ensure `ng new` shows npm warning - // Must be outside the project for `ng new` - process.chdir('..'); - const { message: stderrNew } = await expectToFail(() => ng('new')); - if (!stderrNew.includes(warningText)) { - throw new Error('ng new expected to show npm version warning.'); - } - - // Ensure `ng new --package-manager=npm` shows npm warning - const { message: stderrNewNpm } = await expectToFail(() => ng('new', '--package-manager=npm')); - if (!stderrNewNpm.includes(warningText)) { - throw new Error('ng new expected to show npm version warning.'); - } - - // Ensure `ng new --skip-install` executes successfully - const { stderr: stderrNewSkipInstall } = await ng('new', 'npm-seven-skip', '--skip-install'); - if (stderrNewSkipInstall.includes(warningText)) { - throw new Error('ng new --skip-install expected to not show npm version warning.'); - } - - // Ensure `ng new --package-manager=yarn` executes successfully - // Need an additional npmrc file since yarn does not use the NPM registry environment variable - const { stderr: stderrNewYarn } = await ng('new', 'npm-seven-yarn', '--package-manager=yarn'); - if (stderrNewYarn.includes(warningText)) { - throw new Error('ng new --package-manager=yarn expected to not show npm version warning.'); - } - } finally { - // Cleanup extra test projects - await rimraf('npm-seven-skip'); - await rimraf('npm-seven-yarn'); - - // Change directory back - process.chdir(currentDirectory); - - // Reset version back to initial version - await npm('install', '--global', `npm@${initialVersion}`); - } -}