From 460ea21b5d4b8759a3f7457b885110022dd21dfc Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Wed, 24 Nov 2021 15:11:27 +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 4632f1ffdd86cc8ff9679b09100f4bf03cb6b0e6) --- packages/angular/cli/commands/update-impl.ts | 166 +++++++++++-------- packages/angular/cli/lib/cli/index.ts | 2 + packages/angular/cli/lib/init.ts | 134 +++++++-------- packages/angular/cli/models/version.ts | 30 ++++ 4 files changed, 188 insertions(+), 144 deletions(-) create mode 100644 packages/angular/cli/models/version.ts diff --git a/packages/angular/cli/commands/update-impl.ts b/packages/angular/cli/commands/update-impl.ts index 7002d4f8c8a4..6b70c22575ba 100644 --- a/packages/angular/cli/commands/update-impl.ts +++ b/packages/angular/cli/commands/update-impl.ts @@ -13,6 +13,7 @@ import { execSync } from 'child_process'; import * as fs from 'fs'; import * as path from 'path'; import * as semver from 'semver'; +import { VERSION } from '../lib/cli'; import { PackageManager } from '../lib/config/schema'; import { Command } from '../models/command'; import { Arguments } from '../models/interface'; @@ -38,11 +39,6 @@ const pickManifest = require('npm-pick-manifest') as ( const oldConfigFileNames = ['.angular-cli.json', 'angular-cli.json']; -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', -); - /** * Disable CLI version mismatch checks and forces usage of the invoked CLI * instead of invoking the local installed version. @@ -53,6 +49,8 @@ const disableVersionCheck = disableVersionCheckEnv !== '0' && disableVersionCheckEnv.toLowerCase() !== 'false'; +const ANGULAR_PACKAGES_REGEXP = /^@(?:angular|nguniversal)\//; + export class UpdateCommand extends Command { public readonly allowMissingWorkspace = true; private workflow!: NodeWorkflow; @@ -84,7 +82,7 @@ export class UpdateCommand extends Command { let logs: string[] = []; const files = new Set(); - const reporterSubscription = this.workflow.reporter.subscribe(event => { + const reporterSubscription = this.workflow.reporter.subscribe((event) => { // Strip leading slash to prevent confusion. const eventPath = event.path.startsWith('/') ? event.path.substr(1) : event.path; @@ -114,11 +112,11 @@ export class UpdateCommand extends Command { } }); - const lifecycleSubscription = this.workflow.lifeCycle.subscribe(event => { + const lifecycleSubscription = this.workflow.lifeCycle.subscribe((event) => { if (event.kind == 'end' || event.kind == 'post-tasks-start') { if (!error) { // Output the logging queue, no error happened. - logs.forEach(log => this.logger.info(log)); + logs.forEach((log) => this.logger.info(log)); logs = []; } } @@ -141,12 +139,14 @@ export class UpdateCommand extends Command { return { success: !error, files }; } catch (e) { if (e instanceof UnsuccessfulWorkflowExecution) { - this.logger.error(`${colors.symbols.cross} Migration failed. See above for further details.\n`); + this.logger.error( + `${colors.symbols.cross} Migration failed. See above for further details.\n`, + ); } else { const logPath = writeErrorToLogFile(e); this.logger.fatal( `${colors.symbols.cross} Migration failed: ${e.message}\n` + - ` See "${logPath}" for further details.\n`, + ` See "${logPath}" for further details.\n`, ); } @@ -164,7 +164,7 @@ export class UpdateCommand extends Command { commit?: boolean, ): Promise { const collection = this.workflow.engine.createCollection(collectionPath); - const name = collection.listSchematicNames().find(name => name === migrationName); + const name = collection.listSchematicNames().find((name) => name === migrationName); if (!name) { this.logger.error(`Cannot find migration '${migrationName}' in '${packageName}'.`); @@ -213,20 +213,20 @@ export class UpdateCommand extends Command { return true; } - this.logger.info( - colors.cyan(`** Executing migrations of package '${packageName}' **\n`), - ); + this.logger.info(colors.cyan(`** Executing migrations of package '${packageName}' **\n`)); return this.executePackageMigrations(migrations, packageName, commit); } private async executePackageMigrations( - migrations: Iterable<{ name: string; description: string; collection: { name: string }}>, + migrations: Iterable<{ name: string; description: string; collection: { name: string } }>, packageName: string, commit = false, ): Promise { for (const migration of migrations) { - this.logger.info(`${colors.symbols.pointer} ${migration.description.replace(/\. /g, '.\n ')}`); + this.logger.info( + `${colors.symbols.pointer} ${migration.description.replace(/\. /g, '.\n ')}`, + ); const result = await this.executeSchematic(migration.collection.name, migration.name); if (!result.success) { @@ -280,19 +280,27 @@ export class UpdateCommand extends Command { throw e; } - // 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.logger, - 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.logger, + this.packageManager, + process.argv.slice(2), + ); + } } const packages: PackageIdentifier[] = []; @@ -307,7 +315,7 @@ export class UpdateCommand extends Command { return 1; } - if (packages.some(v => v.name === packageIdentifier.name)) { + if (packages.some((v) => v.name === packageIdentifier.name)) { this.logger.error(`Duplicate package '${packageIdentifier.name}' specified.`); return 1; @@ -410,7 +418,9 @@ export class UpdateCommand extends Command { if (options.migrateOnly) { if (!options.from && typeof options.migrateOnly !== 'string') { - this.logger.error('"from" option is required when using the "migrate-only" option without a migration name.'); + this.logger.error( + '"from" option is required when using the "migrate-only" option without a migration name.', + ); return 1; } else if (packages.length !== 1) { @@ -436,7 +446,7 @@ export class UpdateCommand extends Command { // Allow running migrations on transitively installed dependencies // There can technically be nested multiple versions // TODO: If multiple, this should find all versions and ask which one to use - const child = packageTree.children.find(c => c.name === packageName); + const child = packageTree.children.find((c) => c.name === packageName); if (child) { packageNode = child; } @@ -471,8 +481,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; @@ -498,14 +507,15 @@ export class UpdateCommand extends Command { } } - let success = false; if (typeof options.migrateOnly == 'string') { - success = await this.executeMigration( + await this.executeMigration( packageName, migrations, options.migrateOnly, options.createCommits, ); + + return 0; } else { const from = coerceVersionNumber(options.from); if (!from) { @@ -518,28 +528,15 @@ export class UpdateCommand extends Command { '>' + from + ' <=' + (options.to || packageNode.package.version), ); - success = await this.executeMigrations( + await this.executeMigrations( packageName, migrations, migrationRange, options.createCommits, ); - } - - if (success) { - if ( - packageName === '@angular/core' - && options.from - && +options.from.split('.')[0] < 9 - && (options.to || packageNode.package.version).split('.')[0] === '9' - ) { - this.logger.info(NG_VERSION_9_POST_MSG); - } return 0; } - - return 1; } const requests: { @@ -634,7 +631,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]; @@ -681,7 +678,8 @@ export class UpdateCommand extends Command { if (success && options.createCommits) { const committed = this.commit( - `Angular CLI update for packages - ${packagesToUpdate.join(', ')}`); + `Angular CLI update for packages - ${packagesToUpdate.join(', ')}`, + ); if (!committed) { return 1; } @@ -711,10 +709,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; @@ -744,8 +738,7 @@ export class UpdateCommand extends Command { try { createCommit(message); } catch (err) { - this.logger.error( - `Failed to commit update (${message}):\n${err.stderr}`); + this.logger.error(`Failed to commit update (${message}):\n${err.stderr}`); return false; } @@ -754,8 +747,7 @@ export class UpdateCommand extends Command { const hash = findCurrentGitSha(); const shortMessage = message.split('\n')[0]; if (hash) { - this.logger.info(` Committed migration step (${getShortHash(hash)}): ${ - shortMessage}.`); + this.logger.info(` Committed migration step (${getShortHash(hash)}): ${shortMessage}.`); } else { // Commit was successful, but reading the hash was not. Something weird happened, // but nothing that would stop the update. Just log the weirdness and continue. @@ -768,7 +760,10 @@ export class UpdateCommand extends Command { private checkCleanGit(): boolean { try { - const topLevel = execSync('git rev-parse --show-toplevel', { encoding: 'utf8', stdio: 'pipe' }); + const topLevel = execSync('git rev-parse --show-toplevel', { + encoding: 'utf8', + stdio: 'pipe', + }); const result = execSync('git status --porcelain', { encoding: 'utf8', stdio: 'pipe' }); if (result.trim().length === 0) { return true; @@ -791,14 +786,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. - */ - private async checkCLILatestVersion(verbose = false, next = false): Promise { - const { version: installedCLIVersion } = require('../package.json'); - - const LatestCLIManifest = await fetchPackageManifest( - `@angular/cli@${next ? 'next' : 'latest'}`, + * 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 checkCLIVersion( + packagesToUpdate: string[] | undefined, + verbose = false, + next = false, + ): Promise { + const { version } = await fetchPackageManifest( + `@angular/cli@${this.getCLIUpdateRunnerVersion(packagesToUpdate, next)}`, this.logger, { verbose, @@ -806,7 +803,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; } } @@ -839,7 +867,7 @@ function createCommit(message: string) { */ function findCurrentGitSha(): string | null { try { - const hash = execSync('git rev-parse HEAD', {encoding: 'utf8', stdio: 'pipe'}); + const hash = execSync('git rev-parse HEAD', { encoding: 'utf8', stdio: 'pipe' }); return hash.trim(); } catch { diff --git a/packages/angular/cli/lib/cli/index.ts b/packages/angular/cli/lib/cli/index.ts index e25bab8e567a..3fb467066e32 100644 --- a/packages/angular/cli/lib/cli/index.ts +++ b/packages/angular/cli/lib/cli/index.ts @@ -13,6 +13,8 @@ import { getWorkspaceRaw } from '../../utilities/config'; import { writeErrorToLogFile } from '../../utilities/log-file'; import { getWorkspaceDetails } from '../../utilities/project'; +export { VERSION, Version } from '../../models/version'; + const debugEnv = process.env['NG_DEBUG']; const isDebug = debugEnv !== undefined && diff --git a/packages/angular/cli/lib/init.ts b/packages/angular/cli/lib/init.ts index 776af2d8b455..49ffb7a4557f 100644 --- a/packages/angular/cli/lib/init.ts +++ b/packages/angular/cli/lib/init.ts @@ -5,38 +5,16 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ + import 'symbol-observable'; // symbol polyfill must go first // tslint:disable-next-line:ordered-imports import-groups -import { tags } from '@angular-devkit/core'; import * as fs from 'fs'; import * as path from 'path'; import { SemVer } from 'semver'; -import { Duplex } from 'stream'; import { colors } from '../utilities/color'; import { isWarningEnabled } from '../utilities/config'; - -const packageJson = require('../package.json'); - -function _fromPackageJson(cwd = process.cwd()): SemVer | null { - do { - const packageJsonPath = path.join(cwd, 'node_modules/@angular/cli/package.json'); - if (fs.existsSync(packageJsonPath)) { - const content = fs.readFileSync(packageJsonPath, 'utf-8'); - if (content) { - const { version } = JSON.parse(content); - if (version) { - return new SemVer(version); - } - } - } - - // Check the parent. - cwd = path.dirname(cwd); - } while (cwd != path.dirname(cwd)); - - return null; -} +import { VERSION } from './cli'; // Check if we need to profile this CLI run. if (process.env['NG_CLI_PROFILING']) { @@ -99,43 +77,56 @@ if (process.env['NG_CLI_PROFILING']) { let cli; try { + // No error implies a projectLocalCli, which will load whatever + // version of ng-cli you have installed in a local package.json const projectLocalCli = require.resolve('@angular/cli', { paths: [process.cwd()] }); + cli = await import(projectLocalCli); - // This was run from a global, check local version. - const globalVersion = new SemVer(packageJson['version']); - let localVersion; - let shouldWarn = false; + const globalVersion = new SemVer(VERSION.full); + + // Older versions might not have the VERSION export + let localVersion = cli.VERSION?.full; + if (!localVersion) { + try { + const localPackageJson = fs.readFileSync( + path.join(path.dirname(projectLocalCli), '../../package.json'), + 'utf-8', + ); + localVersion = (JSON.parse(localPackageJson) as { version: string }).version; + } catch (error) { + // tslint:disable-next-line:no-console + console.error('Version mismatch check skipped. Unable to retrieve local version: ' + error); + } + } + let isGlobalGreater = false; try { - localVersion = _fromPackageJson(); - shouldWarn = localVersion != null && globalVersion.compare(localVersion) > 0; - } catch (e) { - // tslint:disable-next-line no-console - console.error(e); - shouldWarn = true; + isGlobalGreater = !!localVersion && globalVersion.compare(localVersion) > 0; + } catch (error) { + // tslint:disable-next-line:no-console + console.error('Version mismatch check skipped. Unable to compare local version: ' + error); } - if (shouldWarn && await isWarningEnabled('versionMismatch')) { - const warning = colors.yellow(tags.stripIndents` - Your global Angular CLI version (${globalVersion}) is greater than your local - version (${localVersion}). The local Angular CLI version is used. - - To disable this warning use "ng config -g cli.warnings.versionMismatch false". - `); - // Don't show warning colorised on `ng completion` - if (process.argv[2] !== 'completion') { - // tslint:disable-next-line no-console - console.error(warning); - } else { - // tslint:disable-next-line no-console - console.error(warning); - process.exit(1); + 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' && + 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 + const warning = + `Your global Angular CLI version (${globalVersion}) is greater than your local ` + + `version (${localVersion}). The local Angular CLI version is used.\n\n` + + 'To disable this warning use "ng config -g cli.warnings.versionMismatch false".'; + + // tslint:disable-next-line:no-console + console.error(colors.yellow(warning)); } } - - // No error implies a projectLocalCli, which will load whatever - // version of ng-cli you have installed in a local package.json - cli = await import(projectLocalCli); } catch { // If there is an error, resolve could not find the ng-cli // library from a package.json. Instead, include it from a relative @@ -149,26 +140,19 @@ if (process.env['NG_CLI_PROFILING']) { } return cli; -})().then(cli => { - // This is required to support 1.x local versions with a 6+ global - let standardInput; - try { - standardInput = process.stdin; - } catch (e) { - process.stdin = new Duplex(); - standardInput = process.stdin; - } - - return cli({ - cliArgs: process.argv.slice(2), - inputStream: standardInput, - outputStream: process.stdout, +})() + .then((cli) => { + return cli({ + cliArgs: process.argv.slice(2), + inputStream: process.stdin, + outputStream: process.stdout, + }); + }) + .then((exitCode: number) => { + process.exit(exitCode); + }) + .catch((err: Error) => { + // tslint:disable-next-line:no-console + console.error('Unknown error: ' + err.toString()); + process.exit(127); }); -}).then((exitCode: number) => { - process.exit(exitCode); -}) -.catch((err: Error) => { - // tslint:disable-next-line no-console - console.error('Unknown error: ' + err.toString()); - process.exit(127); -}); diff --git a/packages/angular/cli/models/version.ts b/packages/angular/cli/models/version.ts new file mode 100644 index 000000000000..ccbc523213fb --- /dev/null +++ b/packages/angular/cli/models/version.ts @@ -0,0 +1,30 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import { readFileSync } from 'fs'; +import { resolve } from 'path'; + +// Same structure as used in framework packages +export class Version { + public readonly major: string; + public readonly minor: string; + public readonly patch: string; + + constructor(public readonly full: string) { + this.major = full.split('.')[0]; + this.minor = full.split('.')[1]; + this.patch = full.split('.').slice(2).join('.'); + } +} + +// TODO: Convert this to use build-time version stamping once implemented in the build system +export const VERSION = new Version( + ( + JSON.parse(readFileSync(resolve(__dirname, '../package.json'), 'utf-8')) as { version: string } + ).version, +);