From 250a58b4820a738aba7609627fa7fce0a24f10db Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Mon, 22 Nov 2021 07:51:45 +0100 Subject: [PATCH] fix(@angular/cli): logic which determines which temp version of the CLI is to be download during `ng update` Previously, when using an older version of the Angular CLI, during `ng update`, we download the temporary `latest` version to run the update. The ensured that when running that the runner used to run the update contains the latest bug fixes and improvements. This however, can be problematic in some cases. Such as when there are API breaking changes, when running a relatively old schematic with the latest CLI can cause runtime issues, especially since those schematics were never meant to be executed on a CLI X major versions in the future. With this change, we improve the logic to determine which version of the Angular CLI should be used to run the update. Below is a summarization of this. - When using the `--next` command line argument, the `@next` version of the CLI will be used to run the update. - When updating an `@angular/` or `@nguniversal/` package, the target version will be used to run the update. Example: `ng update @angular/core@12`, the update will run on most recent patch version of `@angular/cli` of that major version `@12.2.6`. - When updating an `@angular/` or `@nguniversal/` and no target version is specified. Example: `ng update @angular/core` the update will run on most latest version of the `@angular/cli`. - When updating a third-party package, the most recent patch version of the installed `@angular/cli` will be used to run the update. Example if `13.0.0` is installed and `13.1.1` is available on NPM, the latter will be used. (cherry picked from commit 1e9e890bb08ef2eea1ae9578c711922d4c3ac190) --- packages/angular/cli/commands/update-impl.ts | 119 ++++++++++-------- packages/angular/cli/lib/init.ts | 6 +- tests/legacy-cli/e2e/tests/misc/npm-7.ts | 16 ++- .../tests/update/update-secure-registry.ts | 14 ++- 4 files changed, 91 insertions(+), 64 deletions(-) diff --git a/packages/angular/cli/commands/update-impl.ts b/packages/angular/cli/commands/update-impl.ts index c0c461452bd0..6c7a52ea59d1 100644 --- a/packages/angular/cli/commands/update-impl.ts +++ b/packages/angular/cli/commands/update-impl.ts @@ -37,11 +37,6 @@ import { } from '../utilities/package-tree'; import { Schema as UpdateCommandSchema } from './update'; -const NG_VERSION_9_POST_MSG = colors.cyan( - '\nYour project has been updated to Angular version 9!\n' + - 'For more info, please see: https://v9.angular.io/guide/updating-to-version-9', -); - const UPDATE_SCHEMATIC_COLLECTION = path.join( __dirname, '../src/commands/update/schematic/collection.json', @@ -57,6 +52,8 @@ const disableVersionCheck = disableVersionCheckEnv !== '0' && disableVersionCheckEnv.toLowerCase() !== 'false'; +const ANGULAR_PACKAGES_REGEXP = /^@(?:angular|nguniversal)\//; + export class UpdateCommand extends Command { public override readonly allowMissingWorkspace = true; private workflow!: NodeWorkflow; @@ -272,19 +269,26 @@ export class UpdateCommand extends Command { async run(options: UpdateCommandSchema & Arguments) { await ensureCompatibleNpm(this.context.root); - // Check if the current installed CLI version is older than the latest version. - if (!disableVersionCheck && (await this.checkCLILatestVersion(options.verbose, options.next))) { - this.logger.warn( - `The installed local Angular CLI version is older than the latest ${ - options.next ? 'pre-release' : 'stable' - } version.\n` + 'Installing a temporary version to perform the update.', + // Check if the current installed CLI version is older than the latest compatible version. + if (!disableVersionCheck) { + const cliVersionToInstall = await this.checkCLIVersion( + options['--'], + options.verbose, + options.next, ); - return runTempPackageBin( - `@angular/cli@${options.next ? 'next' : 'latest'}`, - this.packageManager, - process.argv.slice(2), - ); + if (cliVersionToInstall) { + this.logger.warn( + 'The installed Angular CLI version is outdated.\n' + + `Installing a temporary Angular CLI versioned ${cliVersionToInstall} to perform the update.`, + ); + + return runTempPackageBin( + `@angular/cli@${cliVersionToInstall}`, + this.packageManager, + process.argv.slice(2), + ); + } } const logVerbose = (message: string) => { @@ -452,8 +456,7 @@ export class UpdateCommand extends Command { if (migrations.startsWith('../')) { this.logger.error( - 'Package contains an invalid migrations field. ' + - 'Paths outside the package root are not permitted.', + 'Package contains an invalid migrations field. Paths outside the package root are not permitted.', ); return 1; @@ -479,9 +482,8 @@ export class UpdateCommand extends Command { } } - let success = false; if (typeof options.migrateOnly == 'string') { - success = await this.executeMigration( + await this.executeMigration( packageName, migrations, options.migrateOnly, @@ -495,7 +497,7 @@ export class UpdateCommand extends Command { return 1; } - success = await this.executeMigrations( + await this.executeMigrations( packageName, migrations, from, @@ -504,19 +506,6 @@ export class UpdateCommand extends Command { ); } - if (success) { - if ( - packageName === '@angular/core' && - options.from && - +options.from.split('.')[0] < 9 && - (options.to || packageNode.version).split('.')[0] === '9' - ) { - this.logger.info(NG_VERSION_9_POST_MSG); - } - - return 0; - } - return 1; } @@ -612,7 +601,7 @@ export class UpdateCommand extends Command { continue; } - if (node.package && /^@(?:angular|nguniversal)\//.test(node.package.name)) { + if (node.package && ANGULAR_PACKAGES_REGEXP.test(node.package.name)) { const { name, version } = node.package; const toBeInstalledMajorVersion = +manifest.version.split('.')[0]; const currentMajorVersion = +version.split('.')[0]; @@ -791,17 +780,6 @@ export class UpdateCommand extends Command { return 0; } } - - if ( - migrations.some( - (m) => - m.package === '@angular/core' && - m.to.split('.')[0] === '9' && - +m.from.split('.')[0] < 9, - ) - ) { - this.logger.info(NG_VERSION_9_POST_MSG); - } } return success ? 0 : 1; @@ -879,14 +857,16 @@ export class UpdateCommand extends Command { } /** - * Checks if the current installed CLI version is older than the latest version. - * @returns `true` when the installed version is older. + * Checks if the current installed CLI version is older or newer than a compatible version. + * @returns the version to install or null when there is no update to install. */ - private async checkCLILatestVersion(verbose = false, next = false): Promise { - const installedCLIVersion = VERSION.full; - - const LatestCLIManifest = await fetchPackageManifest( - `@angular/cli@${next ? 'next' : 'latest'}`, + private async checkCLIVersion( + packagesToUpdate: string[] | undefined, + verbose = false, + next = false, + ): Promise { + const { version } = await fetchPackageManifest( + `@angular/cli@${this.getCLIUpdateRunnerVersion(packagesToUpdate, next)}`, this.logger, { verbose, @@ -894,7 +874,38 @@ export class UpdateCommand extends Command { }, ); - return semver.lt(installedCLIVersion, LatestCLIManifest.version); + return VERSION.full === version ? null : version; + } + + private getCLIUpdateRunnerVersion( + packagesToUpdate: string[] | undefined, + next: boolean, + ): string | number { + if (next) { + return 'next'; + } + + const updatingAngularPackage = packagesToUpdate?.find((r) => ANGULAR_PACKAGES_REGEXP.test(r)); + if (updatingAngularPackage) { + // If we are updating any Angular package we can update the CLI to the target version because + // migrations for @angular/core@13 can be executed using Angular/cli@13. + // This is same behaviour as `npx @angular/cli@13 update @angular/core@13`. + + // `@angular/cli@13` -> ['', 'angular/cli', '13'] + // `@angular/cli` -> ['', 'angular/cli'] + const tempVersion = coerceVersionNumber(updatingAngularPackage.split('@')[2]); + + return semver.parse(tempVersion)?.major ?? 'latest'; + } + + // When not updating an Angular package we cannot determine which schematic runtime the migration should to be executed in. + // Typically, we can assume that the `@angular/cli` was updated previously. + // Example: Angular official packages are typically updated prior to NGRX etc... + // Therefore, we only update to the latest patch version of the installed major version of the Angular CLI. + + // This is important because we might end up in a scenario where locally Angular v12 is installed, updating NGRX from 11 to 12. + // We end up using Angular ClI v13 to run the migrations if we run the migrations using the CLI installed major version + 1 logic. + return VERSION.major; } } diff --git a/packages/angular/cli/lib/init.ts b/packages/angular/cli/lib/init.ts index a48f388ba3fe..cf18b8bcd77b 100644 --- a/packages/angular/cli/lib/init.ts +++ b/packages/angular/cli/lib/init.ts @@ -73,7 +73,11 @@ import { isWarningEnabled } from '../utilities/config'; if (isGlobalGreater) { // If using the update command and the global version is greater, use the newer update command // This allows improvements in update to be used in older versions that do not have bootstrapping - if (process.argv[2] === 'update') { + if ( + process.argv[2] === 'update' && + cli.VERSION && + cli.VERSION.major - globalVersion.major <= 1 + ) { cli = await import('./cli'); } else if (await isWarningEnabled('versionMismatch')) { // Otherwise, use local version and warn if global is newer than local diff --git a/tests/legacy-cli/e2e/tests/misc/npm-7.ts b/tests/legacy-cli/e2e/tests/misc/npm-7.ts index c2bff8ad9d34..1789210a534a 100644 --- a/tests/legacy-cli/e2e/tests/misc/npm-7.ts +++ b/tests/legacy-cli/e2e/tests/misc/npm-7.ts @@ -1,11 +1,12 @@ -import { rimraf, writeFile } from '../../utils/fs'; +import { rimraf } from '../../utils/fs'; import { getActivePackageManager } from '../../utils/packages'; import { 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() { +export default async function () { // Only relevant with npm as a package manager if (getActivePackageManager() !== 'npm') { return; @@ -17,12 +18,18 @@ export default async function() { } const currentDirectory = process.cwd(); + + const extraArgs = []; + 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'); + const { stderr: stderrUpdate1 } = await ng('update', ...extraArgs); if (stderrUpdate1.includes(warningText)) { throw new Error('ng update expected to not show npm version warning.'); } @@ -37,7 +44,7 @@ export default async function() { } // Ensure `ng update` shows npm warning - const { stderr: stderrUpdate2 } = await ng('update'); + const { stderr: stderrUpdate2 } = await ng('update', ...extraArgs); if (!stderrUpdate2.includes(warningText)) { throw new Error('ng update expected to show npm version warning.'); } @@ -85,5 +92,4 @@ export default async function() { // Reset version back to 6.x await npm('install', '--global', 'npm@6'); } - } diff --git a/tests/legacy-cli/e2e/tests/update/update-secure-registry.ts b/tests/legacy-cli/e2e/tests/update/update-secure-registry.ts index 263892d1d01b..f6f7621ffc9a 100644 --- a/tests/legacy-cli/e2e/tests/update/update-secure-registry.ts +++ b/tests/legacy-cli/e2e/tests/update/update-secure-registry.ts @@ -1,29 +1,35 @@ import { ng } from '../../utils/process'; import { createNpmConfigForAuthentication } from '../../utils/registry'; import { expectToFail } from '../../utils/utils'; +import { isPrereleaseCli } from '../../utils/project'; export default async function () { // The environment variable has priority over the .npmrc delete process.env['NPM_CONFIG_REGISTRY']; const worksMessage = 'We analyzed your package.json'; + const extraArgs = []; + if (isPrereleaseCli()) { + extraArgs.push('--next'); + } + // Valid authentication token await createNpmConfigForAuthentication(false); - const { stdout: stdout1 } = await ng('update'); + const { stdout: stdout1 } = await ng('update', ...extraArgs); if (!stdout1.includes(worksMessage)) { throw new Error(`Expected stdout to contain "${worksMessage}"`); } await createNpmConfigForAuthentication(true); - const { stdout: stdout2 } = await ng('update'); + const { stdout: stdout2 } = await ng('update', ...extraArgs); if (!stdout2.includes(worksMessage)) { throw new Error(`Expected stdout to contain "${worksMessage}"`); } // Invalid authentication token await createNpmConfigForAuthentication(false, true); - await expectToFail(() => ng('update')); + await expectToFail(() => ng('update', ...extraArgs)); await createNpmConfigForAuthentication(true, true); - await expectToFail(() => ng('update')); + await expectToFail(() => ng('update', ...extraArgs)); }