Skip to content

Commit

Permalink
fix(@angular/cli): logic which determines which temp version of the C…
Browse files Browse the repository at this point in the history
…LI 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 1e9e890)
  • Loading branch information
alan-agius4 committed Nov 23, 2021
1 parent 6650765 commit 250a58b
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 64 deletions.
119 changes: 65 additions & 54 deletions packages/angular/cli/commands/update-impl.ts
Expand Up @@ -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',
Expand All @@ -57,6 +52,8 @@ const disableVersionCheck =
disableVersionCheckEnv !== '0' &&
disableVersionCheckEnv.toLowerCase() !== 'false';

const ANGULAR_PACKAGES_REGEXP = /^@(?:angular|nguniversal)\//;

export class UpdateCommand extends Command<UpdateCommandSchema> {
public override readonly allowMissingWorkspace = true;
private workflow!: NodeWorkflow;
Expand Down Expand Up @@ -272,19 +269,26 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
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) => {
Expand Down Expand Up @@ -452,8 +456,7 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {

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;
Expand All @@ -479,9 +482,8 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
}
}

let success = false;
if (typeof options.migrateOnly == 'string') {
success = await this.executeMigration(
await this.executeMigration(
packageName,
migrations,
options.migrateOnly,
Expand All @@ -495,7 +497,7 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
return 1;
}

success = await this.executeMigrations(
await this.executeMigrations(
packageName,
migrations,
from,
Expand All @@ -504,19 +506,6 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
);
}

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;
}

Expand Down Expand Up @@ -612,7 +601,7 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
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];
Expand Down Expand Up @@ -791,17 +780,6 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
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;
Expand Down Expand Up @@ -879,22 +857,55 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
}

/**
* 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<boolean> {
const installedCLIVersion = VERSION.full;

const LatestCLIManifest = await fetchPackageManifest(
`@angular/cli@${next ? 'next' : 'latest'}`,
private async checkCLIVersion(
packagesToUpdate: string[] | undefined,
verbose = false,
next = false,
): Promise<string | null> {
const { version } = await fetchPackageManifest(
`@angular/cli@${this.getCLIUpdateRunnerVersion(packagesToUpdate, next)}`,
this.logger,
{
verbose,
usingYarn: this.packageManager === PackageManager.Yarn,
},
);

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;
}
}

Expand Down
6 changes: 5 additions & 1 deletion packages/angular/cli/lib/init.ts
Expand Up @@ -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
Expand Down
16 changes: 11 additions & 5 deletions 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;
Expand All @@ -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.');
}
Expand All @@ -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.');
}
Expand Down Expand Up @@ -85,5 +92,4 @@ export default async function() {
// Reset version back to 6.x
await npm('install', '--global', 'npm@6');
}

}
14 changes: 10 additions & 4 deletions 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));
}

0 comments on commit 250a58b

Please sign in to comment.