From c0471515d2a1eb53f05ede6406ee465f598d634a Mon Sep 17 00:00:00 2001 From: Jason Jean Date: Tue, 12 Apr 2022 09:09:42 -0400 Subject: [PATCH] fix(core): fix migrate race conditions (#9794) --- packages/nx/package.json | 25 ++++ packages/nx/src/command-line/migrate.ts | 181 +++++++++++++++--------- packages/nx/src/utils/package-json.ts | 6 +- packages/workspace/package.json | 74 +++------- 4 files changed, 169 insertions(+), 117 deletions(-) diff --git a/packages/nx/package.json b/packages/nx/package.json index eb75e33f9cd6a..6f3b19c8559d2 100644 --- a/packages/nx/package.json +++ b/packages/nx/package.json @@ -60,5 +60,30 @@ "minimatch": "3.0.4", "enquirer": "~2.3.6", "tslib": "^2.3.0" + }, + "ng-update": { + "requirements": {}, + "packageGroup": { + "@nrwl/workspace": "*", + "@nrwl/angular": "*", + "@nrwl/cypress": "*", + "@nrwl/devkit": "*", + "@nrwl/eslint-plugin-nx": "*", + "@nrwl/express": "*", + "@nrwl/jest": "*", + "@nrwl/linter": "*", + "@nrwl/nest": "*", + "@nrwl/next": "*", + "@nrwl/node": "*", + "@nrwl/nx-plugin": "*", + "@nrwl/react": "*", + "@nrwl/storybook": "*", + "@nrwl/web": "*", + "@nrwl/js": "*", + "@nrwl/cli": "*", + "@nrwl/nx-cloud": "latest", + "@nrwl/react-native": "*", + "@nrwl/detox": "*" + } } } diff --git a/packages/nx/src/command-line/migrate.ts b/packages/nx/src/command-line/migrate.ts index 2764ba9843e9a..5f768a829886f 100644 --- a/packages/nx/src/command-line/migrate.ts +++ b/packages/nx/src/command-line/migrate.ts @@ -1,7 +1,7 @@ import { exec, execSync } from 'child_process'; import { remove } from 'fs-extra'; import { dirname, join } from 'path'; -import { gt, lte } from 'semver'; +import { gt, lt, lte } from 'semver'; import { promisify } from 'util'; import { MigrationsJson, @@ -16,7 +16,11 @@ import { writeJsonFile, } from '../utils/fileutils'; import { logger } from '../utils/logger'; -import { NxMigrationsConfiguration, PackageJson } from '../utils/package-json'; +import { + NxMigrationsConfiguration, + PackageGroup, + PackageJson, +} from '../utils/package-json'; import { createTempNpmDirectory, getPackageManagerCommand, @@ -91,11 +95,10 @@ export class Migrator { } async updatePackageJson(targetPackage: string, targetVersion: string) { - const packageJson = await this._updatePackageJson( - targetPackage, - { version: targetVersion, addToPackageJson: false }, - {} - ); + const packageJson = await this._updatePackageJson(targetPackage, { + version: targetVersion, + addToPackageJson: false, + }); const migrations = await this._createMigrateJson(packageJson); return { packageJson, migrations }; @@ -132,10 +135,11 @@ export class Migrator { return migrations.flat(); } + private collectedVersions: Record = {}; + private async _updatePackageJson( targetPackage: string, - target: PackageJsonUpdateForPackage, - collectedVersions: Record + target: PackageJsonUpdateForPackage ): Promise> { let targetVersion = target.version; if (this.to[targetPackage]) { @@ -155,6 +159,7 @@ export class Migrator { try { migrationsJson = await this.fetch(targetPackage, targetVersion); targetVersion = migrationsJson.version; + this.collectedVersions[targetPackage] = targetVersion; } catch (e) { if (e?.message?.includes('No matching version')) { throw new Error( @@ -173,20 +178,16 @@ export class Migrator { const childPackageMigrations = await Promise.all( Object.keys(packages) - .filter((packageName) => { - return ( - !collectedVersions[packageName] || + .filter( + (packageName) => + !this.collectedVersions[packageName] || this.gt( packages[packageName].version, - collectedVersions[packageName].version + this.collectedVersions[packageName] ) - ); - }) + ) .map((packageName) => - this._updatePackageJson(packageName, packages[packageName], { - ...collectedVersions, - [targetPackage]: target, - }) + this._updatePackageJson(packageName, packages[packageName]) ) ); @@ -222,11 +223,24 @@ export class Migrator { // this should be used to know what version to include // we should use from everywhere we use versions + // Support Migrating to older versions of Nx + // Use the packageGroup of the latest version of Nx instead of the one from the target version which could be older. + if ( + packageName === '@nrwl/workspace' && + lt(targetVersion, '14.0.0-beta.0') + ) { + migration.packageGroup = + require('../../package.json')['ng-update'].packageGroup; + } + if (migration.packageGroup) { migration.packageJsonUpdates ??= {}; - migration.packageJsonUpdates[`${targetVersion}-defaultPackages`] = { + + const packageGroup = normalizePackageGroup(migration.packageGroup); + + migration.packageJsonUpdates[targetVersion + '--PackageGroup'] = { version: targetVersion, - packages: migration.packageGroup.reduce((acc, packageConfig) => { + packages: packageGroup.reduce((acc, packageConfig) => { const { package: pkg, version } = typeof packageConfig === 'string' ? { package: packageConfig, version: targetVersion } @@ -292,6 +306,18 @@ export class Migrator { } } +function normalizePackageGroup( + packageGroup: PackageGroup +): (string | { package: string; version: string })[] { + if (!Array.isArray(packageGroup)) { + return Object.entries(packageGroup).map(([pkg, version]) => ({ + package: pkg, + version, + })); + } + return packageGroup; +} + function normalizeVersionWithTagCheck(version: string) { if (version === 'latest' || version === 'next') return version; return normalizeVersion(version); @@ -427,58 +453,74 @@ function versions(root: string, from: Record) { // testing-fetch-start function createFetcher() { - const cache: Record = {}; + const migrationsCache: Record< + string, + Promise + > = {}; + const resolvedVersionCache: Record> = {}; + + function fetchMigrations( + packageName, + packageVersion, + setCache: (packageName: string, packageVersion: string) => void + ): Promise { + const cacheKey = packageName + '-' + packageVersion; + return Promise.resolve(resolvedVersionCache[cacheKey]) + .then((cachedResolvedVersion) => { + if (cachedResolvedVersion) { + return cachedResolvedVersion; + } + + resolvedVersionCache[cacheKey] = resolvePackageVersionUsingRegistry( + packageName, + packageVersion + ); + return resolvedVersionCache[cacheKey]; + }) + .then((resolvedVersion) => { + if ( + resolvedVersion !== packageVersion && + migrationsCache[`${packageName}-${resolvedVersion}`] + ) { + return migrationsCache[`${packageName}-${resolvedVersion}`]; + } + setCache(packageName, resolvedVersion); + return getPackageMigrationsUsingRegistry(packageName, resolvedVersion); + }) + .catch(() => { + logger.info(`Fetching ${packageName}@${packageVersion}`); - return async function nxMigrateFetcher( + return getPackageMigrationsUsingInstall(packageName, packageVersion); + }); + } + + return function nxMigrateFetcher( packageName: string, packageVersion: string ): Promise { - if (cache[`${packageName}-${packageVersion}`]) { - return cache[`${packageName}-${packageVersion}`]; + if (migrationsCache[`${packageName}-${packageVersion}`]) { + return migrationsCache[`${packageName}-${packageVersion}`]; } let resolvedVersion: string = packageVersion; - let resolvedMigrationConfiguration: ResolvedMigrationConfiguration; - - try { - resolvedVersion = await resolvePackageVersionUsingRegistry( - packageName, - packageVersion - ); - - if (cache[`${packageName}-${resolvedVersion}`]) { - return cache[`${packageName}-${resolvedVersion}`]; - } - - logger.info(`Fetching ${packageName}@${packageVersion}`); - - resolvedMigrationConfiguration = await getPackageMigrationsUsingRegistry( - packageName, - resolvedVersion - ); - } catch { - logger.info(`Fetching ${packageName}@${packageVersion}`); - - resolvedMigrationConfiguration = await getPackageMigrationsUsingInstall( - packageName, - packageVersion - ); - - resolvedVersion = resolvedMigrationConfiguration.version; + let migrations: Promise; + function setCache(packageName: string, packageVersion: string) { + migrationsCache[packageName + '-' + packageVersion] = migrations; } + migrations = fetchMigrations(packageName, packageVersion, setCache).then( + (result) => { + if (result.schematics) { + result.generators = result.schematics; + delete result.schematics; + } + resolvedVersion = result.version; + return result; + } + ); - resolvedMigrationConfiguration = { - ...resolvedMigrationConfiguration, - generators: - resolvedMigrationConfiguration.generators ?? - resolvedMigrationConfiguration.schematics, - }; - - cache[`${packageName}-${packageVersion}`] = cache[ - `${packageName}-${resolvedVersion}` - ] = resolvedMigrationConfiguration; + setCache(packageName, packageVersion); - return resolvedMigrationConfiguration; + return migrations; }; } // testing-fetch-end @@ -494,6 +536,12 @@ async function getPackageMigrationsUsingRegistry( packageVersion ); + if (!migrationsConfig) { + return { + version: packageVersion, + }; + } + if (!migrationsConfig.migrations) { return { version: packageVersion, @@ -501,6 +549,8 @@ async function getPackageMigrationsUsingRegistry( }; } + logger.info(`Fetching ${packageName}@${packageVersion}`); + // try to obtain the migrations from the registry directly return await downloadPackageMigrationsFromRegistry( packageName, @@ -511,8 +561,11 @@ async function getPackageMigrationsUsingRegistry( function resolveNxMigrationConfig(json: Partial) { const parseNxMigrationsConfig = ( - fromJson: string | NxMigrationsConfiguration + fromJson?: string | NxMigrationsConfiguration ): NxMigrationsConfiguration => { + if (!fromJson) { + return {}; + } if (typeof fromJson === 'string') { return { migrations: fromJson, packageGroup: [] }; } diff --git a/packages/nx/src/utils/package-json.ts b/packages/nx/src/utils/package-json.ts index 7133e50f3ef8e..2b7a506b7b060 100644 --- a/packages/nx/src/utils/package-json.ts +++ b/packages/nx/src/utils/package-json.ts @@ -9,9 +9,13 @@ export interface NxProjectPackageJsonConfiguration { targets?: Record; } +export type PackageGroup = + | (string | { package: string; version: string })[] + | Record; + export interface NxMigrationsConfiguration { migrations?: string; - packageGroup?: (string | { package: string; version: string })[]; + packageGroup?: PackageGroup; } export interface PackageJson { diff --git a/packages/workspace/package.json b/packages/workspace/package.json index d29e5aafce465..c91732313e6c9 100644 --- a/packages/workspace/package.json +++ b/packages/workspace/package.json @@ -31,29 +31,28 @@ "ng-update": { "requirements": {}, "migrations": "./migrations.json", - "packageGroup": [ - "@nrwl/workspace", - "@nrwl/angular", - "nx", - "@nrwl/cypress", - "@nrwl/devkit", - "@nrwl/eslint-plugin-nx", - "@nrwl/express", - "@nrwl/jest", - "@nrwl/linter", - "@nrwl/nest", - "@nrwl/next", - "@nrwl/node", - "@nrwl/nx-plugin", - "@nrwl/react", - "@nrwl/storybook", - "@nrwl/web", - "@nrwl/js", - "@nrwl/cli", - "@nrwl/nx-cloud", - "@nrwl/react-native", - "@nrwl/detox" - ] + "packageGroup": { + "@nrwl/angular": "*", + "@nrwl/cypress": "*", + "@nrwl/devkit": "*", + "@nrwl/eslint-plugin-nx": "*", + "@nrwl/express": "*", + "@nrwl/jest": "*", + "@nrwl/linter": "*", + "@nrwl/nest": "*", + "@nrwl/next": "*", + "@nrwl/node": "*", + "@nrwl/nx-plugin": "*", + "@nrwl/react": "*", + "@nrwl/storybook": "*", + "@nrwl/web": "*", + "@nrwl/js": "*", + "@nrwl/cli": "*", + "@nrwl/nx-cloud": "latest", + "@nrwl/react-native": "*", + "@nrwl/detox": "*", + "nx": "*" + } }, "peerDependencies": { "prettier": "^2.5.1" @@ -85,34 +84,5 @@ "minimatch": "3.0.4", "enquirer": "~2.3.6", "tslib": "^2.3.0" - }, - "nx-migrations": { - "migrations": "./migrations.json", - "packageGroup": [ - "@nrwl/workspace", - "@nrwl/angular", - "nx", - "@nrwl/cypress", - "@nrwl/devkit", - "@nrwl/eslint-plugin-nx", - "@nrwl/express", - "@nrwl/jest", - "@nrwl/linter", - "@nrwl/nest", - "@nrwl/next", - "@nrwl/node", - "@nrwl/nx-plugin", - "@nrwl/react", - "@nrwl/storybook", - "@nrwl/web", - "@nrwl/js", - "@nrwl/cli", - "@nrwl/react-native", - "@nrwl/detox", - { - "package": "@nrwl/nx-cloud", - "version": "latest" - } - ] } }