From 02c4bf2af8ef6cb848f111005a96d7359f15dca4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leosvel=20P=C3=A9rez=20Espinosa?= Date: Thu, 10 Nov 2022 12:28:06 +0100 Subject: [PATCH] cleanup(angular): add @angular-devkit/build-angular:karma builder migrator (#13101) --- .../ng-add/migrate-from-angular-cli.ts | 13 +- .../builders/angular-devkit-karma.migrator.ts | 145 ++++++++++++++++++ .../angular-devkit-ng-packagr.migrator.ts | 1 + .../migrators/builders/builder.migrator.ts | 11 +- .../ng-add/migrators/builders/index.ts | 1 + .../generators/ng-add/migrators/migrator.ts | 104 ++++++++++++- .../migrators/projects/app.migrator.spec.ts | 2 +- .../ng-add/migrators/projects/app.migrator.ts | 136 ++++------------ .../migrators/projects/lib.migrator.spec.ts | 8 +- .../ng-add/migrators/projects/lib.migrator.ts | 96 +----------- .../migrators/projects/project.migrator.ts | 104 ++----------- .../src/generators/ng-add/utilities/types.ts | 6 +- .../generators/ng-add/utilities/workspace.ts | 49 +++--- 13 files changed, 349 insertions(+), 327 deletions(-) create mode 100644 packages/angular/src/generators/ng-add/migrators/builders/angular-devkit-karma.migrator.ts diff --git a/packages/angular/src/generators/ng-add/migrate-from-angular-cli.ts b/packages/angular/src/generators/ng-add/migrate-from-angular-cli.ts index ddf126b21f..cc8c3c3a86 100755 --- a/packages/angular/src/generators/ng-add/migrate-from-angular-cli.ts +++ b/packages/angular/src/generators/ng-add/migrate-from-angular-cli.ts @@ -19,7 +19,7 @@ import { createWorkspaceFiles, decorateAngularCli, getAllProjects, - getWorkspaceCapabilities, + getWorkspaceRootFileTypesInfo, normalizeOptions, updatePackageJson, updateRootEsLintConfig, @@ -59,7 +59,10 @@ export async function migrateFromAngularCli( // validate all projects validateProjects(migrators); - const workspaceCapabilities = getWorkspaceCapabilities(tree, projects); + const workspaceRootFileTypesInfo = getWorkspaceRootFileTypesInfo( + tree, + migrators + ); /** * Keep a copy of the root eslint config to restore it later. We need to @@ -68,7 +71,7 @@ export async function migrateFromAngularCli( * the app migration. */ let eslintConfig = - workspaceCapabilities.eslint && tree.exists('.eslintrc.json') + workspaceRootFileTypesInfo.eslint && tree.exists('.eslintrc.json') ? readJson(tree, '.eslintrc.json') : undefined; @@ -95,10 +98,10 @@ export async function migrateFromAngularCli( * these files in the root for the root application, so we wait until * those root config files are moved when the projects are migrated. */ - if (workspaceCapabilities.karma) { + if (workspaceRootFileTypesInfo.karma) { createRootKarmaConfig(tree); } - if (workspaceCapabilities.eslint) { + if (workspaceRootFileTypesInfo.eslint) { updateRootEsLintConfig(tree, eslintConfig, options.unitTestRunner); cleanupEsLintPackages(tree); } diff --git a/packages/angular/src/generators/ng-add/migrators/builders/angular-devkit-karma.migrator.ts b/packages/angular/src/generators/ng-add/migrators/builders/angular-devkit-karma.migrator.ts new file mode 100644 index 0000000000..61df820a6a --- /dev/null +++ b/packages/angular/src/generators/ng-add/migrators/builders/angular-devkit-karma.migrator.ts @@ -0,0 +1,145 @@ +import { + joinPathFragments, + ProjectConfiguration, + TargetConfiguration, + Tree, + updateProjectConfiguration, +} from '@nrwl/devkit'; +import { offsetFromRoot } from '@nrwl/devkit'; +import { getRootTsConfigPathInTree } from '@nrwl/workspace/src/utilities/typescript'; +import { basename } from 'path'; +import type { + Logger, + ProjectMigrationInfo, + ValidationError, + ValidationResult, +} from '../../utilities'; +import { arrayToString } from '../../utilities'; +import { BuilderMigrator } from './builder.migrator'; + +export class AngularDevkitKarmaMigrator extends BuilderMigrator { + constructor( + tree: Tree, + project: ProjectMigrationInfo, + projectConfig: ProjectConfiguration, + logger: Logger + ) { + super( + tree, + '@angular-devkit/build-angular:karma', + 'karma', + project, + projectConfig, + logger + ); + } + + override migrate(): void { + for (const [name, target] of this.targets) { + this.moveFilePathsFromTargetToProjectRoot(target, [ + 'karmaConfig', + 'tsConfig', + 'webWorkerTsConfig', + ]); + this.updateTargetConfiguration(name, target); + this.updateTsConfigFileUsedByTestTarget(name, target); + this.updateCacheableOperations([name]); + } + + if (!this.targets.size && this.projectConfig.root === '') { + // there could still be a karma.conf.js file in the root + // so move to new location + const karmaConfig = 'karma.conf.js'; + if (this.tree.exists(karmaConfig)) { + this.logger.info( + 'No "test" target was found, but a root Karma config file was found in the project root. The file will be moved to the new location.' + ); + this.moveProjectRootFile(karmaConfig); + } + } + } + + override validate(): ValidationResult { + const errors: ValidationError[] = []; + // TODO(leo): keeping restriction until the full refactor is done and we start + // expanding what's supported. + if (this.targets.size > 1) { + errors.push({ + message: `There is more than one target using a builder that is used to build the project (${arrayToString( + [...this.targets.keys()] + )}).`, + hint: `Make sure the project only has one target with a builder that is used to build the project.`, + }); + } + + return errors.length ? errors : null; + } + + private updateTargetConfiguration( + targetName: string, + target: TargetConfiguration + ): void { + if (!target.options) { + this.logger.warn( + `The target "${targetName}" is not specifying any options. Skipping updating the target configuration.` + ); + return; + } + + target.options.main = + target.options.main && this.convertAsset(target.options.main); + target.options.polyfills = + target.options.polyfills && this.convertAsset(target.options.polyfills); + target.options.tsConfig = + target.options.tsConfig && + joinPathFragments( + this.project.newRoot, + basename(target.options.tsConfig) + ); + target.options.karmaConfig = + target.options.karmaConfig && + joinPathFragments( + this.project.newRoot, + basename(target.options.karmaConfig) + ); + target.options.assets = + target.options.assets && + target.options.assets.map((asset) => this.convertAsset(asset)); + target.options.styles = + target.options.styles && + target.options.styles.map((style) => this.convertAsset(style)); + target.options.scripts = + target.options.scripts && + target.options.scripts.map((script) => this.convertAsset(script)); + + updateProjectConfiguration(this.tree, this.project.name, { + ...this.projectConfig, + }); + } + + private updateTsConfigFileUsedByTestTarget( + targetName: string, + target: TargetConfiguration + ): void { + if (!target.options?.tsConfig) { + this.logger.warn( + `The "${targetName}" target does not have the "tsConfig" option configured. Skipping updating the tsConfig file.` + ); + return; + } + if (!this.tree.exists(target.options.tsConfig)) { + const originalTsConfigPath = + this.originalProjectConfig.targets[targetName].options.tsConfig; + this.logger.warn( + `The tsConfig file "${originalTsConfigPath}" specified in the "${targetName}" target could not be found. Skipping updating the tsConfig file.` + ); + return; + } + + this.updateTsConfigFile( + target.options.tsConfig, + getRootTsConfigPathInTree(this.tree), + offsetFromRoot(this.projectConfig.root) + ); + } +} diff --git a/packages/angular/src/generators/ng-add/migrators/builders/angular-devkit-ng-packagr.migrator.ts b/packages/angular/src/generators/ng-add/migrators/builders/angular-devkit-ng-packagr.migrator.ts index 4566313a63..2d1544a316 100644 --- a/packages/angular/src/generators/ng-add/migrators/builders/angular-devkit-ng-packagr.migrator.ts +++ b/packages/angular/src/generators/ng-add/migrators/builders/angular-devkit-ng-packagr.migrator.ts @@ -31,6 +31,7 @@ export class AngularDevkitNgPackagrMigrator extends BuilderMigrator { super( tree, '@angular-devkit/build-angular:ng-packagr', + undefined, project, projectConfig, logger diff --git a/packages/angular/src/generators/ng-add/migrators/builders/builder.migrator.ts b/packages/angular/src/generators/ng-add/migrators/builders/builder.migrator.ts index ad945e4b1a..766b8be68c 100644 --- a/packages/angular/src/generators/ng-add/migrators/builders/builder.migrator.ts +++ b/packages/angular/src/generators/ng-add/migrators/builders/builder.migrator.ts @@ -3,7 +3,11 @@ import type { TargetConfiguration, Tree, } from '@nrwl/devkit'; -import type { Logger, ProjectMigrationInfo } from '../../utilities'; +import type { + Logger, + ProjectMigrationInfo, + WorkspaceRootFileType, +} from '../../utilities'; import { Migrator } from '../migrator'; export abstract class BuilderMigrator extends Migrator { @@ -12,6 +16,7 @@ export abstract class BuilderMigrator extends Migrator { constructor( tree: Tree, public readonly builderName: string, + public readonly rootFileType: WorkspaceRootFileType | undefined, project: ProjectMigrationInfo, projectConfig: ProjectConfiguration, logger: Logger @@ -24,6 +29,10 @@ export abstract class BuilderMigrator extends Migrator { this.collectBuilderTargets(); } + isBuilderUsed(): boolean { + return this.targets.size > 0; + } + protected collectBuilderTargets(): void { for (const [name, target] of Object.entries( this.projectConfig.targets ?? {} diff --git a/packages/angular/src/generators/ng-add/migrators/builders/index.ts b/packages/angular/src/generators/ng-add/migrators/builders/index.ts index c3fb0d3d8f..78fe0ab98c 100644 --- a/packages/angular/src/generators/ng-add/migrators/builders/index.ts +++ b/packages/angular/src/generators/ng-add/migrators/builders/index.ts @@ -1,3 +1,4 @@ +export * from './angular-devkit-karma.migrator'; export * from './angular-devkit-ng-packagr.migrator'; export * from './builder-migrator-class.type'; export * from './builder.migrator'; diff --git a/packages/angular/src/generators/ng-add/migrators/migrator.ts b/packages/angular/src/generators/ng-add/migrators/migrator.ts index 81f1365d26..15f3d3396a 100644 --- a/packages/angular/src/generators/ng-add/migrators/migrator.ts +++ b/packages/angular/src/generators/ng-add/migrators/migrator.ts @@ -1,9 +1,15 @@ -import type { ProjectConfiguration, Tree } from '@nrwl/devkit'; +import type { + ProjectConfiguration, + TargetConfiguration, + Tree, +} from '@nrwl/devkit'; import { + joinPathFragments, readWorkspaceConfiguration, updateJson, updateWorkspaceConfiguration, } from '@nrwl/devkit'; +import { basename } from 'path'; import type { Logger } from '../utilities/logger'; import type { ProjectMigrationInfo, @@ -27,6 +33,52 @@ export abstract class Migrator { abstract migrate(): Promise | void; abstract validate(): ValidationResult; + protected convertAsset(asset: string | any): string | any { + if (typeof asset === 'string') { + return this.convertSourceRootPath(asset); + } else { + return { ...asset, input: this.convertSourceRootPath(asset.input) }; + } + } + + protected moveFile(from: string, to: string, required: boolean = true): void { + if (!this.tree.exists(from)) { + if (required) { + this.logger.warn(`The path "${from}" does not exist. Skipping.`); + } + } else if (this.tree.exists(to)) { + if (required) { + this.logger.warn(`The path "${to}" already exists. Skipping.`); + } + } else { + const contents = this.tree.read(from); + this.tree.write(to, contents); + this.tree.delete(from); + } + } + + protected moveFilePathsFromTargetToProjectRoot( + target: TargetConfiguration, + options: string[] + ) { + options.forEach((option) => { + this.getTargetValuesForOption(target, option).forEach((path) => { + this.moveProjectRootFile(path); + }); + }); + } + + protected moveProjectRootFile(filePath: string, isRequired = true): void { + if (!filePath) { + return; + } + + const filename = !!filePath ? basename(filePath) : ''; + const from = filePath; + const to = joinPathFragments(this.project.newRoot, filename); + this.moveFile(from, to, isRequired); + } + // TODO(leo): This should be moved to BuilderMigrator once everything is split into builder migrators. protected updateCacheableOperations(targetNames: string[]): void { if (!targetNames.length) { @@ -63,4 +115,54 @@ export abstract class Migrator { return json; }); } + + private convertSourceRootPath(originalPath: string): string { + return originalPath?.startsWith(this.project.oldSourceRoot) + ? joinPathFragments( + this.project.newSourceRoot, + originalPath.replace(this.project.oldSourceRoot, '') + ) + : originalPath; + } + + private getTargetValuesForOption( + target: TargetConfiguration, + optionPath: string + ): any[] { + const values = new Set(); + const value = this.getValueForOption(target.options, optionPath); + if (value) { + values.add(value); + } + + for (const configuration of Object.values(target.configurations ?? {})) { + const value = this.getValueForOption(configuration, optionPath); + if (value) { + values.add(value); + } + } + + return Array.from(values); + } + + private getValueForOption( + options: Record | undefined, + optionPath: string + ): any { + if (!options) { + return null; + } + + const segments = optionPath.split('.'); + let value = options; + for (const segment of segments) { + if (value && value[segment]) { + value = value[segment]; + } else { + return null; + } + } + + return value; + } } diff --git a/packages/angular/src/generators/ng-add/migrators/projects/app.migrator.spec.ts b/packages/angular/src/generators/ng-add/migrators/projects/app.migrator.spec.ts index 3ad58f0ef0..fa7b0e004b 100644 --- a/packages/angular/src/generators/ng-add/migrators/projects/app.migrator.spec.ts +++ b/packages/angular/src/generators/ng-add/migrators/projects/app.migrator.spec.ts @@ -1577,9 +1577,9 @@ describe('app migrator', () => { 'lint', 'test', 'e2e', + 'myCustomTest', 'myCustomBuild', 'myCustomLint', - 'myCustomTest', ]); }); diff --git a/packages/angular/src/generators/ng-add/migrators/projects/app.migrator.ts b/packages/angular/src/generators/ng-add/migrators/projects/app.migrator.ts index b0c85f440f..c799d2fd43 100644 --- a/packages/angular/src/generators/ng-add/migrators/projects/app.migrator.ts +++ b/packages/angular/src/generators/ng-add/migrators/projects/app.migrator.ts @@ -17,6 +17,8 @@ import type { Target, ValidationResult, } from '../../utilities'; +import type { BuilderMigratorClassType } from '../builders'; +import { AngularDevkitKarmaMigrator } from '../builders'; import { E2eMigrator } from './e2e.migrator'; import { ProjectMigrator } from './project.migrator'; @@ -28,8 +30,7 @@ type SupportedTargets = | 'prerender' | 'serve' | 'server' - | 'serveSsr' - | 'test'; + | 'serveSsr'; const supportedTargets: Record = { build: { builders: ['@angular-devkit/build-angular:browser'] }, e2e: { @@ -45,9 +46,13 @@ const supportedTargets: Record = { serve: { builders: ['@angular-devkit/build-angular:dev-server'] }, server: { builders: ['@angular-devkit/build-angular:server'] }, serveSsr: { builders: ['@nguniversal/builders:ssr-dev-server'] }, - test: { builders: ['@angular-devkit/build-angular:karma'] }, }; +// TODO(leo): this will replace `supportedTargets` once the full refactor is done. +const supportedBuilderMigrators: BuilderMigratorClassType[] = [ + AngularDevkitKarmaMigrator, +]; + export class AppMigrator extends ProjectMigrator { private e2eMigrator: E2eMigrator; private newEsLintConfigPath: string; @@ -59,7 +64,15 @@ export class AppMigrator extends ProjectMigrator { project: MigrationProjectConfiguration, logger?: Logger ) { - super(tree, options, supportedTargets, project, 'apps', logger); + super( + tree, + options, + supportedTargets, + project, + 'apps', + logger, + supportedBuilderMigrators + ); this.e2eMigrator = new E2eMigrator( tree, @@ -82,25 +95,33 @@ export class AppMigrator extends ProjectMigrator { this.moveProjectFiles(); await this.updateProjectConfiguration(); + + for (const builderMigrator of this.builderMigrators ?? []) { + await builderMigrator.migrate(); + } + this.updateTsConfigs(); this.updateEsLintConfig(); this.updateCacheableOperations( [ this.targetNames.build, this.targetNames.lint, - this.targetNames.test, this.targetNames.e2e, ].filter(Boolean) ); } override validate(): ValidationResult { - const result: ValidationResult = [ + const errors: ValidationResult = [ ...(super.validate() ?? []), ...(this.e2eMigrator.validate() ?? []), ]; - return result.length ? result : null; + for (const builderMigrator of this.builderMigrators) { + errors.push(...(builderMigrator.validate() ?? [])); + } + + return errors.length ? errors : null; } private moveProjectFiles(): void { @@ -123,23 +144,6 @@ export class AppMigrator extends ProjectMigrator { ); } - if (this.targetNames.test) { - this.moveFilePathsFromTargetToProjectRoot( - this.projectConfig.targets[this.targetNames.test], - ['karmaConfig', 'tsConfig', 'webWorkerTsConfig'] - ); - } else { - // there could still be a karma.conf.js file in the root - // so move to new location - const karmaConfig = 'karma.conf.js'; - if (this.tree.exists(karmaConfig)) { - this.logger.info( - 'No "test" target was found, but a root Karma config file was found in the project root. The file will be moved to the new location.' - ); - this.moveProjectRootFile(karmaConfig); - } - } - if (this.targetNames.server) { this.moveFilePathsFromTargetToProjectRoot( this.projectConfig.targets[this.targetNames.server], @@ -178,7 +182,6 @@ export class AppMigrator extends ProjectMigrator { } else { this.updateBuildTargetConfiguration(); this.updateLintTargetConfiguration(); - this.updateTestTargetConfiguration(); this.updateServerTargetConfiguration(); this.updatePrerenderTargetConfiguration(); this.updateServeSsrTargetConfiguration(); @@ -202,10 +205,6 @@ export class AppMigrator extends ProjectMigrator { rootTsConfigFile, projectOffsetFromRoot ); - this.updateTsConfigFileUsedByTestTarget( - rootTsConfigFile, - projectOffsetFromRoot - ); this.updateTsConfigFileUsedByServerTarget(projectOffsetFromRoot); } @@ -303,17 +302,6 @@ export class AppMigrator extends ProjectMigrator { })); } - private moveFilePathsFromTargetToProjectRoot( - target: TargetConfiguration, - options: string[] - ) { - options.forEach((option) => { - this.getTargetValuesForOption(target, option).forEach((path) => { - this.moveProjectRootFile(path); - }); - }); - } - private updateBuildTargetConfiguration(): void { if (!this.targetNames.build) { this.logger.warn( @@ -521,56 +509,6 @@ export class AppMigrator extends ProjectMigrator { }); } - private updateTestTargetConfiguration(): void { - if (!this.targetNames.test) { - return; - } - - const testOptions = - this.projectConfig.targets[this.targetNames.test].options; - if (!testOptions) { - this.logger.warn( - `The target "${this.targetNames.test}" is not specifying any options. Skipping updating the target configuration.` - ); - return; - } - - if (!testOptions.tsConfig) { - this.logger.warn( - `The "${this.targetNames.test}" target does not have the "tsConfig" option configured. Skipping updating the tsConfig file.` - ); - } else { - const newTestTsConfig = this.convertPath(testOptions.tsConfig); - if (!this.tree.exists(newTestTsConfig)) { - this.logger.warn( - `The tsConfig file "${testOptions.tsConfig}" specified in the "${this.targetNames.test}" target could not be found. Skipping updating the tsConfig file.` - ); - } - } - - testOptions.main = testOptions.main && this.convertAsset(testOptions.main); - testOptions.polyfills = - testOptions.polyfills && this.convertAsset(testOptions.polyfills); - testOptions.tsConfig = - testOptions.tsConfig && - joinPathFragments(this.project.newRoot, basename(testOptions.tsConfig)); - testOptions.karmaConfig = - testOptions.karmaConfig && - joinPathFragments( - this.project.newRoot, - basename(testOptions.karmaConfig) - ); - testOptions.assets = - testOptions.assets && - testOptions.assets.map((asset) => this.convertAsset(asset)); - testOptions.styles = - testOptions.styles && - testOptions.styles.map((style) => this.convertAsset(style)); - testOptions.scripts = - testOptions.scripts && - testOptions.scripts.map((script) => this.convertAsset(script)); - } - private updateTsConfigFileUsedByBuildTarget( rootTsConfigFile: string, projectOffsetFromRoot: string @@ -619,22 +557,4 @@ export class AppMigrator extends ProjectMigrator { return json; }); } - - private updateTsConfigFileUsedByTestTarget( - rootTsConfigFile: string, - projectOffsetFromRoot: string - ): void { - if (!this.targetNames.test) { - return; - } - - const tsConfig = - this.projectConfig.targets[this.targetNames.test].options?.tsConfig; - if (!tsConfig || !this.tree.exists(tsConfig)) { - // we already logged a warning for these cases, so just return - return; - } - - this.updateTsConfigFile(tsConfig, rootTsConfigFile, projectOffsetFromRoot); - } } diff --git a/packages/angular/src/generators/ng-add/migrators/projects/lib.migrator.spec.ts b/packages/angular/src/generators/ng-add/migrators/projects/lib.migrator.spec.ts index b2cc8e2c18..6b76634ec2 100644 --- a/packages/angular/src/generators/ng-add/migrators/projects/lib.migrator.spec.ts +++ b/packages/angular/src/generators/ng-add/migrators/projects/lib.migrator.spec.ts @@ -129,7 +129,7 @@ describe('lib migrator', () => { 'The "build" target is using an unsupported builder "@not/supported:builder".', ]); expect(result[0].hint).toMatchInlineSnapshot( - `"The supported builders for libraries are: \\"@angular-devkit/build-angular:karma\\", \\"@angular-eslint/builder:lint\\" and \\"@angular-devkit/build-angular:ng-packagr\\"."` + `"The supported builders for libraries are: \\"@angular-eslint/builder:lint\\", \\"@angular-devkit/build-angular:ng-packagr\\" and \\"@angular-devkit/build-angular:karma\\"."` ); }); @@ -152,7 +152,7 @@ describe('lib migrator', () => { 'The "test" target is using an unsupported builder "@other/not-supported:builder".', ]); expect(result[0].hint).toMatchInlineSnapshot( - `"The supported builders for libraries are: \\"@angular-devkit/build-angular:karma\\", \\"@angular-eslint/builder:lint\\" and \\"@angular-devkit/build-angular:ng-packagr\\"."` + `"The supported builders for libraries are: \\"@angular-eslint/builder:lint\\", \\"@angular-devkit/build-angular:ng-packagr\\" and \\"@angular-devkit/build-angular:karma\\"."` ); }); @@ -171,7 +171,7 @@ describe('lib migrator', () => { 'The "my-build" target is using an unsupported builder "@not/supported:builder".', ]); expect(result[0].hint).toMatchInlineSnapshot( - `"The supported builders for libraries are: \\"@angular-devkit/build-angular:karma\\", \\"@angular-eslint/builder:lint\\" and \\"@angular-devkit/build-angular:ng-packagr\\"."` + `"The supported builders for libraries are: \\"@angular-eslint/builder:lint\\", \\"@angular-devkit/build-angular:ng-packagr\\" and \\"@angular-devkit/build-angular:karma\\"."` ); }); @@ -1284,8 +1284,8 @@ describe('lib migrator', () => { 'test', 'e2e', 'myCustomBuild', - 'myCustomLint', 'myCustomTest', + 'myCustomLint', ]); }); diff --git a/packages/angular/src/generators/ng-add/migrators/projects/lib.migrator.ts b/packages/angular/src/generators/ng-add/migrators/projects/lib.migrator.ts index f889e6546d..25f137de7c 100644 --- a/packages/angular/src/generators/ng-add/migrators/projects/lib.migrator.ts +++ b/packages/angular/src/generators/ng-add/migrators/projects/lib.migrator.ts @@ -8,8 +8,6 @@ import { } from '@nrwl/devkit'; import { hasRulesRequiringTypeChecking } from '@nrwl/linter'; import { convertToNxProjectGenerator } from '@nrwl/workspace/generators'; -import { getRootTsConfigPathInTree } from '@nrwl/workspace/src/utilities/typescript'; -import { basename } from 'path'; import type { GeneratorOptions } from '../../schema'; import type { Logger, @@ -19,17 +17,20 @@ import type { ValidationResult, } from '../../utilities'; import type { BuilderMigratorClassType } from '../builders'; -import { AngularDevkitNgPackagrMigrator } from '../builders'; +import { + AngularDevkitKarmaMigrator, + AngularDevkitNgPackagrMigrator, +} from '../builders'; import { ProjectMigrator } from './project.migrator'; -type SupportedTargets = 'test' | 'lint'; +type SupportedTargets = 'lint'; const supportedTargets: Record = { - test: { builders: ['@angular-devkit/build-angular:karma'] }, lint: { builders: ['@angular-eslint/builder:lint'] }, }; // TODO(leo): this will replace `supportedTargets` once the full refactor is done. const supportedBuilderMigrators: BuilderMigratorClassType[] = [ AngularDevkitNgPackagrMigrator, + AngularDevkitKarmaMigrator, ]; export class LibMigrator extends ProjectMigrator { @@ -69,11 +70,8 @@ export class LibMigrator extends ProjectMigrator { await builderMigrator.migrate(); } - this.updateTsConfigs(); this.updateEsLintConfig(); - this.updateCacheableOperations( - [this.targetNames.lint, this.targetNames.test].filter(Boolean) - ); + this.updateCacheableOperations([this.targetNames.lint].filter(Boolean)); } override validate(): ValidationResult { @@ -103,7 +101,6 @@ export class LibMigrator extends ProjectMigrator { ); } else { this.updateLintTargetConfiguration(); - this.updateTestTargetConfiguration(); } updateProjectConfiguration(this.tree, this.project.name, { @@ -116,16 +113,6 @@ export class LibMigrator extends ProjectMigrator { }); } - private updateTsConfigs(): void { - const rootTsConfigFile = getRootTsConfigPathInTree(this.tree); - const projectOffsetFromRoot = offsetFromRoot(this.projectConfig.root); - - this.updateTsConfigFileUsedByTestTarget( - rootTsConfigFile, - projectOffsetFromRoot - ); - } - private updateEsLintConfig(): void { if (!this.targetNames.lint || !this.tree.exists(this.newEsLintConfigPath)) { return; @@ -232,73 +219,4 @@ export class LibMigrator extends ProjectMigrator { lintOptions.hasTypeAwareRules = true; } } - - private updateTestTargetConfiguration(): void { - if (!this.targetNames.test) { - return; - } - - const testOptions = - this.projectConfig.targets[this.targetNames.test].options; - if (!testOptions) { - this.logger.warn( - `The target "${this.targetNames.test}" is not specifying any options. Skipping updating the target configuration.` - ); - return; - } - - if (!testOptions.tsConfig) { - this.logger.warn( - `The "${this.targetNames.test}" target does not have the "tsConfig" option configured. Skipping updating the tsConfig file.` - ); - } else if (!this.tree.exists(testOptions.tsConfig)) { - this.logger.warn( - `The tsConfig file "${testOptions.tsConfig}" specified in the "${this.targetNames.test}" target could not be found. Skipping updating the tsConfig file.` - ); - } - - testOptions.main = testOptions.main && this.convertAsset(testOptions.main); - testOptions.polyfills = - testOptions.polyfills && this.convertAsset(testOptions.polyfills); - testOptions.tsConfig = - testOptions.tsConfig && - joinPathFragments(this.project.newRoot, basename(testOptions.tsConfig)); - testOptions.karmaConfig = - testOptions.karmaConfig && - joinPathFragments( - this.project.newRoot, - basename(testOptions.karmaConfig) - ); - testOptions.assets = - testOptions.assets && - testOptions.assets.map((asset) => this.convertAsset(asset)); - testOptions.styles = - testOptions.styles && - testOptions.styles.map((style) => this.convertAsset(style)); - testOptions.scripts = - testOptions.scripts && - testOptions.scripts.map((script) => this.convertAsset(script)); - } - - private updateTsConfigFileUsedByTestTarget( - rootTsConfigFile: string, - projectOffsetFromRoot: string - ): void { - if (!this.targetNames.test) { - return; - } - - const tsConfig = - this.projectConfig.targets[this.targetNames.test].options?.tsConfig; - if (!tsConfig || !this.tree.exists(tsConfig)) { - // we already logged a warning for these cases, so just return - return; - } - - this.updateTsConfigFile( - this.projectConfig.targets[this.targetNames.test].options.tsConfig, - rootTsConfigFile, - projectOffsetFromRoot - ); - } } diff --git a/packages/angular/src/generators/ng-add/migrators/projects/project.migrator.ts b/packages/angular/src/generators/ng-add/migrators/projects/project.migrator.ts index 1f930b83a4..1ed7b5687b 100644 --- a/packages/angular/src/generators/ng-add/migrators/projects/project.migrator.ts +++ b/packages/angular/src/generators/ng-add/migrators/projects/project.migrator.ts @@ -1,17 +1,18 @@ -import type { TargetConfiguration, Tree } from '@nrwl/devkit'; +import type { Tree } from '@nrwl/devkit'; import { joinPathFragments, normalizePath, offsetFromRoot, visitNotIgnoredFiles, } from '@nrwl/devkit'; -import { basename, dirname } from 'path'; +import { dirname } from 'path'; import type { GeneratorOptions } from '../../schema'; import type { MigrationProjectConfiguration, Target, ValidationError, ValidationResult, + WorkspaceRootFileTypesInfo, } from '../../utilities'; import { arrayToString, Logger } from '../../utilities'; import type { BuilderMigratorClassType } from '../builders'; @@ -55,6 +56,20 @@ export abstract class ProjectMigrator< this.createBuilderMigrators(supportedBuilderMigrators); } + getWorkspaceRootFileTypesInfo(): WorkspaceRootFileTypesInfo { + const workspaceRootFileTypesInfo: WorkspaceRootFileTypesInfo = { + eslint: + Boolean(this.projectConfig.targets?.lint) || + this.tree.exists(`${this.projectConfig.root}/.eslintrc.json`), + karma: this.builderMigrators.some( + (migrator) => + migrator.rootFileType === 'karma' && migrator.isBuilderUsed() + ), + }; + + return workspaceRootFileTypesInfo; + } + override validate(): ValidationResult { const errors: ValidationError[] = []; @@ -170,14 +185,6 @@ export abstract class ProjectMigrator< return errors.length ? errors : null; } - protected convertAsset(asset: string | any): string | any { - if (typeof asset === 'string') { - return this.convertSourceRootPath(asset); - } else { - return { ...asset, input: this.convertSourceRootPath(asset.input) }; - } - } - protected convertEsLintConfigExtendToNewPath( eslintConfigPath: string, extendPath: string @@ -196,15 +203,6 @@ export abstract class ProjectMigrator< ); } - protected convertSourceRootPath(originalPath: string): string { - return originalPath?.startsWith(this.project.oldSourceRoot) - ? joinPathFragments( - this.project.newSourceRoot, - originalPath.replace(this.project.oldSourceRoot, '') - ) - : originalPath; - } - protected convertRootPath(originalPath: string): string { return originalPath?.startsWith(this.project.oldRoot) ? joinPathFragments( @@ -232,80 +230,12 @@ export abstract class ProjectMigrator< return originalPath; } - protected getTargetValuesForOption( - target: TargetConfiguration, - optionPath: string - ): any[] { - const values = new Set(); - const value = this.getValueForOption(target.options, optionPath); - if (value) { - values.add(value); - } - - for (const configuration of Object.values(target.configurations ?? {})) { - const value = this.getValueForOption(configuration, optionPath); - if (value) { - values.add(value); - } - } - - return Array.from(values); - } - - protected getValueForOption( - options: Record | undefined, - optionPath: string - ): any { - if (!options) { - return null; - } - - const segments = optionPath.split('.'); - let value = options; - for (const segment of segments) { - if (value && value[segment]) { - value = value[segment]; - } else { - return null; - } - } - - return value; - } - - protected moveProjectRootFile(filePath: string, isRequired = true): void { - if (!filePath) { - return; - } - - const filename = !!filePath ? basename(filePath) : ''; - const from = filePath; - const to = joinPathFragments(this.project.newRoot, filename); - this.moveFile(from, to, isRequired); - } - protected moveDir(from: string, to: string): void { visitNotIgnoredFiles(this.tree, from, (file) => { this.moveFile(file, normalizePath(file).replace(from, to), true); }); } - protected moveFile(from: string, to: string, required: boolean = true): void { - if (!this.tree.exists(from)) { - if (required) { - this.logger.warn(`The path "${from}" does not exist. Skipping.`); - } - } else if (this.tree.exists(to)) { - if (required) { - this.logger.warn(`The path "${to}" already exists. Skipping.`); - } - } else { - const contents = this.tree.read(from); - this.tree.write(to, contents); - this.tree.delete(from); - } - } - private collectTargetNames(): void { const targetTypes = Object.keys(this.targets) as TargetType[]; diff --git a/packages/angular/src/generators/ng-add/utilities/types.ts b/packages/angular/src/generators/ng-add/utilities/types.ts index c5d703d86e..c5b5aea7c0 100644 --- a/packages/angular/src/generators/ng-add/utilities/types.ts +++ b/packages/angular/src/generators/ng-add/utilities/types.ts @@ -18,10 +18,8 @@ export type ProjectMigrationInfo = { newSourceRoot: string; }; -export type WorkspaceCapabilities = { - karma: boolean; - eslint: boolean; -}; +export type WorkspaceRootFileType = 'karma' | 'eslint'; +export type WorkspaceRootFileTypesInfo = Record; export type ValidationError = { message?: string; diff --git a/packages/angular/src/generators/ng-add/utilities/workspace.ts b/packages/angular/src/generators/ng-add/utilities/workspace.ts index b3d14e9c00..66cb7437ef 100644 --- a/packages/angular/src/generators/ng-add/utilities/workspace.ts +++ b/packages/angular/src/generators/ng-add/utilities/workspace.ts @@ -18,8 +18,9 @@ import { readFileSync } from 'fs'; import { readModulePackageJson } from 'nx/src/utils/package-json'; import { dirname, join } from 'path'; import { angularDevkitVersion, nxVersion } from '../../../utils/versions'; +import type { ProjectMigrator } from '../migrators'; import type { GeneratorOptions } from '../schema'; -import type { WorkspaceCapabilities, WorkspaceProjects } from './types'; +import type { WorkspaceRootFileTypesInfo } from './types'; import { workspaceMigrationErrorHeading } from './validation-logging'; export function validateWorkspace(tree: Tree): void { @@ -257,45 +258,39 @@ export function createRootKarmaConfig(tree: Tree): void { ); } -export function getWorkspaceCapabilities( +export function getWorkspaceRootFileTypesInfo( tree: Tree, - projects: WorkspaceProjects -): WorkspaceCapabilities { - const capabilities: WorkspaceCapabilities = { eslint: false, karma: false }; + migrators: ProjectMigrator[] +): WorkspaceRootFileTypesInfo { + const workspaceRootFileTypesInfo: WorkspaceRootFileTypesInfo = { + eslint: false, + karma: false, + }; if (tree.exists('.eslintrc.json')) { - capabilities.eslint = true; + workspaceRootFileTypesInfo.eslint = true; } if (tree.exists('karma.conf.js')) { - capabilities.karma = true; + workspaceRootFileTypesInfo.karma = true; } - if (capabilities.eslint && capabilities.karma) { - return capabilities; + if (workspaceRootFileTypesInfo.eslint && workspaceRootFileTypesInfo.karma) { + return workspaceRootFileTypesInfo; } - for (const project of [...projects.apps, ...projects.libs]) { - if ( - !capabilities.eslint && - (project.config.targets?.lint || - tree.exists(`${project.config.root}/.eslintrc.json`)) - ) { - capabilities.eslint = true; - } - if ( - !capabilities.karma && - (project.config.targets?.test || - tree.exists(`${project.config.root}/karma.conf.js`)) - ) { - capabilities.karma = true; - } + for (const migrator of migrators) { + const projectInfo = migrator.getWorkspaceRootFileTypesInfo(); + workspaceRootFileTypesInfo.eslint = + workspaceRootFileTypesInfo.eslint || projectInfo.eslint; + workspaceRootFileTypesInfo.karma = + workspaceRootFileTypesInfo.karma || projectInfo.karma; - if (capabilities.eslint && capabilities.karma) { - return capabilities; + if (workspaceRootFileTypesInfo.eslint && workspaceRootFileTypesInfo.karma) { + return workspaceRootFileTypesInfo; } } - return capabilities; + return workspaceRootFileTypesInfo; } function updateVsCodeRecommendedExtensions(tree: Tree): void {