From de520d00c510896500af6be136005f4169eb925d Mon Sep 17 00:00:00 2001 From: Craigory Coppola Date: Thu, 13 Apr 2023 12:46:42 -0400 Subject: [PATCH] feat(nx-plugin): remove cli property from generators and executors schema.json file (#16259) --- e2e/nx-misc/src/misc.test.ts | 3 + e2e/nx-misc/src/nxw.test.ts | 3 + packages/nx-plugin/migrations.json | 6 + .../executor/__fileName__/schema.json__tmpl__ | 1 - .../__fileName__/schema.json__tmpl__ | 1 - .../src/generators/migration/migration.ts | 13 +- .../update-16-0-0/cli-in-schema-json.spec.ts | 266 ++++++++++++++++++ .../update-16-0-0/cli-in-schema-json.ts | 126 +++++++++ packages/nx/src/command-line/migrate.ts | 174 +++++++++--- 9 files changed, 541 insertions(+), 52 deletions(-) create mode 100644 packages/nx-plugin/src/migrations/update-16-0-0/cli-in-schema-json.spec.ts create mode 100644 packages/nx-plugin/src/migrations/update-16-0-0/cli-in-schema-json.ts diff --git a/e2e/nx-misc/src/misc.test.ts b/e2e/nx-misc/src/misc.test.ts index 72aeda1e86436..9613fcebb5e88 100644 --- a/e2e/nx-misc/src/misc.test.ts +++ b/e2e/nx-misc/src/misc.test.ts @@ -297,6 +297,8 @@ describe('migrate', () => { description: '1.1.0', factory: './run11', }, + }, + generators: { run20: { version: '2.0.0', description: '2.0.0', @@ -309,6 +311,7 @@ describe('migrate', () => { updateFile( `./node_modules/migrate-parent-package/run11.js`, ` + var angular_devkit_core1 = require("@angular-devkit/core"); exports.default = function default_1() { return function(host) { host.create('file-11', 'content11') diff --git a/e2e/nx-misc/src/nxw.test.ts b/e2e/nx-misc/src/nxw.test.ts index 88d5bbe687301..46e20f8c15f5f 100644 --- a/e2e/nx-misc/src/nxw.test.ts +++ b/e2e/nx-misc/src/nxw.test.ts @@ -124,6 +124,8 @@ describe('nx wrapper / .nx installation', () => { description: '1.1.0', factory: './run11', }, + }, + generators: { run20: { version: '2.0.0', description: '2.0.0', @@ -136,6 +138,7 @@ describe('nx wrapper / .nx installation', () => { updateFile( `.nx/installation/node_modules/migrate-parent-package/run11.js`, ` + var angular_devkit_core1 = require("@angular-devkit/core"); exports.default = function default_1() { return function(host) { host.create('file-11', 'content11') diff --git a/packages/nx-plugin/migrations.json b/packages/nx-plugin/migrations.json index e5ba051377bcf..d3a5289471432 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/executor/files/executor/__fileName__/schema.json__tmpl__ b/packages/nx-plugin/src/generators/executor/files/executor/__fileName__/schema.json__tmpl__ index bc334fa30afe7..8b87936519db2 100644 --- a/packages/nx-plugin/src/generators/executor/files/executor/__fileName__/schema.json__tmpl__ +++ b/packages/nx-plugin/src/generators/executor/files/executor/__fileName__/schema.json__tmpl__ @@ -1,7 +1,6 @@ { "$schema": "http://json-schema.org/schema", "version": 2, - "cli": "nx", "title": "<%= className %> executor", "description": "", "type": "object", diff --git a/packages/nx-plugin/src/generators/generator/files/generator/__fileName__/schema.json__tmpl__ b/packages/nx-plugin/src/generators/generator/files/generator/__fileName__/schema.json__tmpl__ index 7ded707fbde36..8a59c30b3f587 100644 --- a/packages/nx-plugin/src/generators/generator/files/generator/__fileName__/schema.json__tmpl__ +++ b/packages/nx-plugin/src/generators/generator/files/generator/__fileName__/schema.json__tmpl__ @@ -1,6 +1,5 @@ { "$schema": "http://json-schema.org/schema", - "cli": "nx", "$id": "<%= className %>", "title": "", "type": "object", diff --git a/packages/nx-plugin/src/generators/migration/migration.ts b/packages/nx-plugin/src/generators/migration/migration.ts index cb4f101d400bd..e604e94e9cdfc 100644 --- a/packages/nx-plugin/src/generators/migration/migration.ts +++ b/packages/nx-plugin/src/generators/migration/migration.ts @@ -14,13 +14,7 @@ import type { Tree } from '@nrwl/devkit'; import type { Schema } from './schema'; import * as path from 'path'; import { addMigrationJsonChecks } from '../lint-checks/generator'; -import type { Linter as EsLint } from 'eslint'; -import { - NxMigrationsConfiguration, - PackageJson, - PackageJsonTargetConfiguration, - readNxMigrateConfig, -} from 'nx/src/utils/package-json'; +import { PackageJson, readNxMigrateConfig } from 'nx/src/utils/package-json'; interface NormalizedSchema extends Schema { projectRoot: string; projectSourceRoot: string; @@ -74,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 0000000000000..6a0d175216700 --- /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 0000000000000..8be51fe2013c4 --- /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 76bb48dc140a8..f54196a82a07b 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,119 @@ 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?.[name] || collection.schematics?.[name]; + if (!g) { + throw new Error( + `Unable to determine implementation path for "${collectionPath}:${name}"` + ); + } + const implRelativePath = g.implementation || g.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; + }; +})();