diff --git a/packages/nx-plugin/migrations.json b/packages/nx-plugin/migrations.json index e5ba051377bcf2..d3a52894714325 100644 --- a/packages/nx-plugin/migrations.json +++ b/packages/nx-plugin/migrations.json @@ -29,6 +29,12 @@ "cli": "nx", "description": "Update nx plugin jest test files to support jest 29 changes (https://jestjs.io/docs/upgrading-to-jest29)", "factory": "./src/migrations/update-15-9-0/jest-29-tests" + }, + "update-remove-cli-prop": { + "version": "16.0.0-beta.1", + "cli": "nx", + "description": "Removes CLI property within schema.json files and moves generators and schematics to the proper root node in migrations.json", + "factory": "./src/migrations/update-16-0-0/cli-in-schema-json" } } } diff --git a/packages/nx-plugin/src/generators/migration/migration.ts b/packages/nx-plugin/src/generators/migration/migration.ts index ab6a757731d274..e604e94e9cdfc4 100644 --- a/packages/nx-plugin/src/generators/migration/migration.ts +++ b/packages/nx-plugin/src/generators/migration/migration.ts @@ -68,11 +68,6 @@ function updateMigrationsJson(host: Tree, options: NormalizedSchema) { ? readJson(host, migrationsPath) : {}; - if (migrations.schematics) { - migrations.generators = migrations.schematics; - delete migrations.schematics; - } - const generators = migrations.generators ?? {}; generators[options.name] = { version: options.packageVersion, diff --git a/packages/nx-plugin/src/migrations/update-16-0-0/cli-in-schema-json.spec.ts b/packages/nx-plugin/src/migrations/update-16-0-0/cli-in-schema-json.spec.ts new file mode 100644 index 00000000000000..6a0d175216700c --- /dev/null +++ b/packages/nx-plugin/src/migrations/update-16-0-0/cli-in-schema-json.spec.ts @@ -0,0 +1,266 @@ +import { + ExecutorsJson, + GeneratorsJson, + joinPathFragments, + MigrationsJson, + readJson, + readProjectConfiguration, + Tree, + updateJson, + writeJson, +} from '@nrwl/devkit'; +import { createTreeWithEmptyWorkspace } from '@nrwl/devkit/testing'; +import { Linter } from '@nrwl/linter'; +import { PackageJson } from 'nx/src/utils/package-json'; +import executorGenerator from '../../generators/executor/executor'; +import generatorGenerator from '../../generators/generator/generator'; +import pluginGenerator from '../../generators/plugin/plugin'; +import { updateCliPropsForPlugins } from './cli-in-schema-json'; + +describe('updateCliPropsForPlugins', () => { + it('should move non-nx generators to schematics for migrations.json', async () => { + const tree = createTreeWithEmptyWorkspace(); + const { root } = await createPlugin(tree); + updatePluginPackageJson(tree, { + 'nx-migrations': 'migrations.json', + }); + writeJson( + tree, + joinPathFragments(root, 'migrations.json'), + { + version: '1.0.0', + generators: { + 'migration-1': { + version: '1.0.0', + description: 'My Plugin 1', + factory: './migrations/my-plugin-1', + }, + }, + } + ); + await updateCliPropsForPlugins(tree); + const updated = readJson( + tree, + joinPathFragments(root, 'migrations.json') + ); + expect(updated.generators).not.toHaveProperty('migration-1'); + expect(updated.schematics).toHaveProperty('migration-1'); + }); + + it('should move nx generators to generators for migrations.json', async () => { + const tree = createTreeWithEmptyWorkspace(); + const { root } = await createPlugin(tree); + updatePluginPackageJson(tree, { + 'nx-migrations': 'migrations.json', + }); + writeJson( + tree, + joinPathFragments(root, 'migrations.json'), + { + version: '1.0.0', + schematics: { + 'migration-1': { + version: '1.0.0', + description: 'My Plugin 1', + factory: './migrations/my-plugin-1', + cli: 'nx', + }, + }, + } + ); + await updateCliPropsForPlugins(tree); + const updated = readJson( + tree, + joinPathFragments(root, 'migrations.json') + ); + expect(updated.schematics).not.toHaveProperty('migration-1'); + expect(updated.generators).toHaveProperty('migration-1'); + }); + + it('should move both nx generators to generators and non-nx schematics to schematics for migrations.json', async () => { + const tree = createTreeWithEmptyWorkspace(); + const { root } = await createPlugin(tree); + updatePluginPackageJson(tree, { + 'nx-migrations': 'migrations.json', + }); + writeJson( + tree, + joinPathFragments(root, 'migrations.json'), + { + version: '1.0.0', + schematics: { + 'migration-1': { + version: '1.0.0', + description: 'My Plugin 1', + factory: './migrations/my-plugin-1', + cli: 'nx', + }, + 'migration-2': { + version: '1.0.0', + description: 'My Plugin 2', + factory: './migrations/my-plugin-2', + }, + }, + generators: { + 'migration-3': { + version: '1.0.0', + description: 'My Plugin 3', + factory: './migrations/my-plugin-3', + cli: 'nx', + }, + 'migration-4': { + version: '1.0.0', + description: 'My Plugin 4', + factory: './migrations/my-plugin-4', + }, + }, + } + ); + await updateCliPropsForPlugins(tree); + const updated = readJson( + tree, + joinPathFragments(root, 'migrations.json') + ); + expect(updated.schematics).not.toHaveProperty('migration-1'); + expect(updated.generators).toHaveProperty('migration-1'); + expect(updated.schematics).toHaveProperty('migration-2'); + expect(updated.generators).not.toHaveProperty('migration-2'); + expect(updated.schematics).not.toHaveProperty('migration-3'); + expect(updated.generators).toHaveProperty('migration-3'); + expect(updated.schematics).toHaveProperty('migration-4'); + expect(updated.generators).not.toHaveProperty('migration-4'); + }); + + it('should remove cli property from executors', async () => { + const tree = createTreeWithEmptyWorkspace(); + const { root, name } = await createPlugin(tree); + executorGenerator(tree, { + name: 'my-executor', + project: name, + unitTestRunner: 'jest', + includeHasher: false, + }); + const schemaPath = joinPathFragments( + root, + 'src/executors/my-executor/schema.json' + ); + updateJson(tree, schemaPath, (schema) => { + schema.cli = 'nx'; + return schema; + }); + await updateCliPropsForPlugins(tree); + const updated = readJson(tree, schemaPath); + expect(updated).not.toHaveProperty('cli'); + }); + + it('should remove cli property from builders', async () => { + const tree = createTreeWithEmptyWorkspace(); + const { root, name } = await createPlugin(tree); + executorGenerator(tree, { + name: 'my-executor', + project: name, + unitTestRunner: 'jest', + includeHasher: false, + }); + updateJson( + tree, + joinPathFragments(root, 'executors.json'), + (json) => { + json.builders = json.executors; + delete json.builders; + return json; + } + ); + const schemaPath = joinPathFragments( + root, + 'src/executors/my-executor/schema.json' + ); + updateJson(tree, schemaPath, (schema) => { + schema.cli = 'nx'; + return schema; + }); + await updateCliPropsForPlugins(tree); + const updated = readJson(tree, schemaPath); + expect(updated).not.toHaveProperty('cli'); + }); + + it('should remove cli property from generators', async () => { + const tree = createTreeWithEmptyWorkspace(); + const { root, name } = await createPlugin(tree); + generatorGenerator(tree, { + name: 'my-generator', + project: name, + unitTestRunner: 'jest', + }); + const schemaPath = joinPathFragments( + root, + 'src/generators/my-generator/schema.json' + ); + updateJson(tree, schemaPath, (schema) => { + schema.cli = 'nx'; + return schema; + }); + await updateCliPropsForPlugins(tree); + const updated = readJson(tree, schemaPath); + expect(updated).not.toHaveProperty('cli'); + }); + + it('should remove cli property from schematics', async () => { + const tree = createTreeWithEmptyWorkspace(); + const { root, name } = await createPlugin(tree); + generatorGenerator(tree, { + name: 'my-schematic', + project: name, + unitTestRunner: 'jest', + }); + updateJson( + tree, + joinPathFragments(root, 'generators.json'), + (json) => { + json.schematics = json.generators; + delete json.generators; + return json; + } + ); + const schemaPath = joinPathFragments( + root, + 'src/generators/my-schematic/schema.json' + ); + updateJson(tree, schemaPath, (schema) => { + schema.cli = 'nx'; + return schema; + }); + await updateCliPropsForPlugins(tree); + const updated = readJson(tree, schemaPath); + expect(updated).not.toHaveProperty('cli'); + }); +}); + +async function createPlugin(tree: Tree) { + await pluginGenerator(tree, { + name: 'my-plugin', + compiler: 'tsc', + linter: Linter.EsLint, + unitTestRunner: 'jest', + skipFormat: true, + skipLintChecks: false, + skipTsConfig: false, + }); + return readProjectConfiguration(tree, 'my-plugin'); +} + +function updatePluginPackageJson( + tree: Tree, + packageJsonProps: Partial +) { + const { root } = readProjectConfiguration(tree, 'my-plugin'); + updateJson(tree, root + '/package.json', (json) => { + const base = { json, ...packageJsonProps }; + for (const prop in base) { + if (base[prop] === null || base[prop] === undefined) { + delete json[prop]; + } + } + return base; + }); +} diff --git a/packages/nx-plugin/src/migrations/update-16-0-0/cli-in-schema-json.ts b/packages/nx-plugin/src/migrations/update-16-0-0/cli-in-schema-json.ts new file mode 100644 index 00000000000000..8be51fe2013c4a --- /dev/null +++ b/packages/nx-plugin/src/migrations/update-16-0-0/cli-in-schema-json.ts @@ -0,0 +1,126 @@ +import { + GeneratorsJson, + getProjects, + joinPathFragments, + MigrationsJson, + ExecutorsJson, + readJson, + Tree, + updateJson, +} from '@nrwl/devkit'; +import { + ExecutorsJsonEntry, + GeneratorsJsonEntry, +} from 'nx/src/config/misc-interfaces'; +import { PackageJson, readNxMigrateConfig } from 'nx/src/utils/package-json'; +import { dirname } from 'path'; + +export function updateCliPropsForPlugins(tree: Tree) { + const projects = getProjects(tree); + for (const [name, project] of projects.entries()) { + if (tree.exists(joinPathFragments(project.root, 'package.json'))) { + const packageJson: PackageJson = readJson( + tree, + joinPathFragments(project.root, 'package.json') + ); + const migrateConfig = readNxMigrateConfig(packageJson); + if (migrateConfig.migrations) { + updateMigrationsJsonForPlugin( + tree, + joinPathFragments(project.root, migrateConfig.migrations) + ); + } + if (packageJson.generators) { + removeCliFromGeneratorSchemaJsonFiles( + tree, + joinPathFragments(project.root, packageJson.generators) + ); + } + if (packageJson.executors) { + removeCliFromExecutorSchemaJsonFiles( + tree, + joinPathFragments(project.root, packageJson.executors) + ); + } + if (packageJson.builders) { + removeCliFromExecutorSchemaJsonFiles( + tree, + joinPathFragments(project.root, packageJson.builders) + ); + } + if (packageJson.schematics) { + removeCliFromGeneratorSchemaJsonFiles( + tree, + joinPathFragments(project.root, packageJson.schematics) + ); + } + } + } +} + +function removeCliFromExecutorSchemaJsonFiles( + tree: Tree, + collectionPath: string +) { + const collection: ExecutorsJson = readJson(tree, collectionPath); + for (const [name, entry] of Object.entries(collection.executors ?? {}).concat( + Object.entries(collection.builders ?? {}) + )) { + deleteCliPropFromSchemaFile(collectionPath, entry, tree); + } +} + +function removeCliFromGeneratorSchemaJsonFiles( + tree: Tree, + collectionPath: string +) { + const collection: GeneratorsJson = readJson(tree, collectionPath); + for (const [name, entry] of Object.entries( + collection.generators ?? {} + ).concat(Object.entries(collection.schematics ?? {}))) { + deleteCliPropFromSchemaFile(collectionPath, entry, tree); + } +} + +function updateMigrationsJsonForPlugin(tree: Tree, collectionPath: string) { + updateJson(tree, collectionPath, (json) => { + for (const migration in json.generators ?? {}) { + if (!(json.generators[migration].cli === 'nx')) { + json.schematics ??= {}; + json.schematics[migration] = json.generators[migration]; + delete json.generators[migration]; + } + } + for (const migration in json.schematics ?? {}) { + if (json.schematics[migration].cli === 'nx') { + json.generators ??= {}; + json.generators[migration] = json.schematics[migration]; + delete json.schematics[migration]; + } + } + return json; + }); +} + +export default updateCliPropsForPlugins; + +function deleteCliPropFromSchemaFile( + collectionPath: string, + entry: ExecutorsJsonEntry | GeneratorsJsonEntry, + tree: Tree +) { + if (!entry.schema) { + return; + } + const schemaPath = joinPathFragments(dirname(collectionPath), entry.schema); + if (tree.exists(schemaPath)) { + updateJson(tree, schemaPath, (json) => { + if (json.cli) { + delete json.cli; + } + return json; + }); + } else { + console.warn(`Could not find schema file ${schemaPath}`); + } +} diff --git a/packages/nx/src/command-line/migrate.ts b/packages/nx/src/command-line/migrate.ts index 52dc38d6466576..6be6413ca54442 100644 --- a/packages/nx/src/command-line/migrate.ts +++ b/packages/nx/src/command-line/migrate.ts @@ -50,7 +50,7 @@ import { connectToNxCloudCommand } from './connect'; import { output } from '../utils/output'; import { messages, recordStat } from '../utils/ab-testing'; import { nxVersion } from '../utils/versions'; -import { existsSync } from 'fs'; +import { existsSync, readFileSync } from 'fs'; import { workspaceRoot } from '../utils/workspace-root'; import { isCI } from '../utils/is-ci'; import { getNxRequirePaths } from '../utils/installation-directory'; @@ -1321,16 +1321,19 @@ export async function executeMigrations( const migrationsWithNoChanges: typeof migrations = []; - let ngCliAdapter: typeof import('../adapter/ngcli-adapter'); - if (migrations.some((m) => m.cli !== 'nx')) { - ngCliAdapter = await import('../adapter/ngcli-adapter'); - require('../adapter/compat'); - } - for (const m of migrations) { try { - if (m.cli === 'nx') { - const changes = await runNxMigration(root, m.package, m.name); + const { collection, collectionPath } = readMigrationCollection( + m.package, + root + ); + if (!isAngularMigration(collection, collectionPath, m.name)) { + const changes = await runNxMigration( + root, + collectionPath, + collection, + m.name + ); if (changes.length < 1) { migrationsWithNoChanges.push(m); @@ -1342,6 +1345,7 @@ export async function executeMigrations( logger.info(` ${m.description}\n`); printChanges(changes, ' '); } else { + const ngCliAdapter = await getNgCompatLayer(); const { madeChanges, loggingQueue } = await ngCliAdapter.runMigration( root, m.package, @@ -1507,35 +1511,13 @@ function getLatestCommitSha(): string | null { } } -async function runNxMigration(root: string, packageName: string, name: string) { - const collectionPath = readPackageMigrationConfig( - packageName, - root - ).migrations; - - const collection = readJsonFile(collectionPath); - const g = collection.generators || collection.schematics; - if (!g[name]) { - const source = collection.generators ? 'generators' : 'schematics'; - throw new Error( - `Unable to determine implementation path for "${collectionPath}:${name}" using collection.${source}` - ); - } - const implRelativePath = g[name].implementation || g[name].factory; - - let implPath: string; - - try { - implPath = require.resolve(implRelativePath, { - paths: [dirname(collectionPath)], - }); - } catch (e) { - // workaround for a bug in node 12 - implPath = require.resolve( - `${dirname(collectionPath)}/${implRelativePath}` - ); - } - +async function runNxMigration( + root: string, + collectionPath: string, + collection: MigrationsJson, + name: string +) { + const implPath = getImplementationPath(collection, collectionPath, name); const fn = require(implPath).default; const host = new FsTree(root, false); await fn(host, {}); @@ -1572,3 +1554,120 @@ export async function migrate( } }); } + +function readMigrationCollection(packageName: string, root: string) { + const collectionPath = readPackageMigrationConfig( + packageName, + root + ).migrations; + return { + collection: readJsonFile(collectionPath), + collectionPath, + }; +} + +function getImplementationPath( + collection: MigrationsJson, + collectionPath: string, + name: string +) { + const g = collection.generators || collection.schematics; + if (!g[name]) { + const source = collection.generators ? 'generators' : 'schematics'; + throw new Error( + `Unable to determine implementation path for "${collectionPath}:${name}" using collection.${source}` + ); + } + const implRelativePath = g[name].implementation || g[name].factory; + + let implPath: string; + + try { + implPath = require.resolve(implRelativePath, { + paths: [dirname(collectionPath)], + }); + } catch (e) { + // workaround for a bug in node 12 + implPath = require.resolve( + `${dirname(collectionPath)}/${implRelativePath}` + ); + } + + return implPath; +} + +// TODO (v17): This should just become something like: +// ``` +// return !collection.generators[name] && collection.schematics[name] +// ``` +function isAngularMigration( + collection: MigrationsJson, + collectionPath: string, + name: string +) { + const entry = collection.generators?.[name] || collection.schematics?.[name]; + + // In the future we will determine this based on the location of the entry in the collection. + // If the entry is under `schematics`, it will be assumed to be an angular cli migration. + // If the entry is under `generators`, it will be assumed to be an nx migration. + // For now, we will continue to obey the cli property, if it exists. + // If it doesn't exist, we will check if the implementation references @angular/devkit. + const shouldBeNx = !!collection.generators?.[name]; + const shouldBeNg = !!collection.schematics?.[name]; + let useAngularDevkitToRunMigration = false; + + const implementationPath = getImplementationPath( + collection, + collectionPath, + name + ); + const implStringContents = readFileSync(implementationPath, 'utf-8'); + // TODO (v17): Remove this check and the cli property access - it is only here for backwards compatibility. + if ( + [ + "import('@angular-devkit", + 'import("@angular-devkit', + "require('@angular-devkit", + 'require("@angular-devkit', + "from '@angular-devkit", + 'from "@angular-devkit', + ].some((s) => implStringContents.includes(s)) + ) { + useAngularDevkitToRunMigration = true; + } + + if (useAngularDevkitToRunMigration && shouldBeNx) { + output.warn({ + title: `The migration '${collectionPath}:${name}' appears to be an Angular CLI migration, but is located in the 'generators' section of migrations.json.`, + bodyLines: [ + 'In the future, migrations in the generators section will be assumed to be Nx migrations.', + "Please open an issue on the Nx repository if you believe this is an error, or on the plugin's repository so that the author can move it to the appropriate section.", + ], + }); + } + + if (!useAngularDevkitToRunMigration && entry.cli === 'nx' && shouldBeNg) { + output.warn({ + title: `The migration '${collectionPath}:${name}' appears to be an Nx migration, but is located in the 'schematics' section of migrations.json.`, + bodyLines: [ + 'In the future, migrations in the schematics section will be assumed to be Angular CLI migrations.', + "Please open an issue on the Nx repository if you believe this is an error, or on the plugin's repository so that the author can move it to the appropriate section.", + ], + }); + } + + // Currently, if the cli property exists we listen to it. If its nx, its not an ng cli migration. + // If the property is not set, we will fall back to our intuition. + return entry.cli ? entry.cli !== 'nx' : useAngularDevkitToRunMigration; +} + +const getNgCompatLayer = (() => { + let _ngCliAdapter: typeof import('../adapter/ngcli-adapter'); + return async function getNgCompatLayer() { + if (!_ngCliAdapter) { + _ngCliAdapter = await import('../adapter/ngcli-adapter'); + require('../adapter/compat'); + } + return _ngCliAdapter; + }; +})();