From 43141334a802d718ce6c78e7a5acb35fc70d3072 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Jona=C5=A1?= Date: Wed, 13 Dec 2023 15:18:17 +0100 Subject: [PATCH] fix(linter): move should migrate all eslint configs (#20709) --- .../src/generators/init/init-migration.ts | 71 +++++++++++----- .../src/generators/utils/eslint-file.ts | 4 +- .../convert-to-monorepo.spec.ts | 3 - .../convert-to-monorepo.ts | 8 +- .../move/lib/extract-base-configs.ts | 15 ++-- .../src/generators/move/move.spec.ts | 82 ++++++++++++++++++- .../workspace/src/generators/move/move.ts | 8 +- 7 files changed, 151 insertions(+), 40 deletions(-) diff --git a/packages/eslint/src/generators/init/init-migration.ts b/packages/eslint/src/generators/init/init-migration.ts index 0a78ae6804d4a..94468632a7b92 100644 --- a/packages/eslint/src/generators/init/init-migration.ts +++ b/packages/eslint/src/generators/init/init-migration.ts @@ -29,28 +29,43 @@ export function migrateConfigToMonorepoStyle( tree: Tree, unitTestRunner: string ): void { - if (useFlatConfig(tree)) { - // we need this for the compat - addDependenciesToPackageJson( - tree, - {}, - { - '@eslint/js': eslintVersion, - } - ); - tree.write( - 'eslint.base.config.js', - getGlobalFlatEslintConfiguration(unitTestRunner) - ); + const rootEslintConfig = findEslintFile(tree); + let skipCleanup = false; + if ( + rootEslintConfig?.match(/\.base\./) && + !projects.some((p) => p.root === '.') + ) { + // if the migration has been run already, we need to rename the base config + // and only update the extends paths + tree.rename(rootEslintConfig, rootEslintConfig.replace('.base.', '.')); + skipCleanup = true; } else { - writeJson( - tree, - '.eslintrc.base.json', - getGlobalEsLintConfiguration(unitTestRunner) - ); + if (useFlatConfig(tree)) { + // we need this for the compat + addDependenciesToPackageJson( + tree, + {}, + { + '@eslint/js': eslintVersion, + } + ); + tree.write( + tree.exists('eslint.config.js') + ? 'eslint.base.config.js' + : 'eslint.config.js', + getGlobalFlatEslintConfiguration(unitTestRunner) + ); + } else { + const eslintFile = findEslintFile(tree, '.'); + writeJson( + tree, + eslintFile ? '.eslintrc.base.json' : '.eslintrc.json', + getGlobalEsLintConfiguration(unitTestRunner) + ); + } } - // update extens in all projects' eslint configs + // update extends in all projects' eslint configs projects.forEach((project) => { const lintTarget = findLintTarget(project); if (lintTarget) { @@ -58,7 +73,18 @@ export function migrateConfigToMonorepoStyle( lintTarget.options?.eslintConfig || findEslintFile(tree, project.root); if (eslintFile) { const projectEslintPath = joinPathFragments(project.root, eslintFile); - migrateEslintFile(projectEslintPath, tree); + if (skipCleanup) { + const content = tree.read(projectEslintPath, 'utf-8'); + tree.write( + projectEslintPath, + content.replace( + rootEslintConfig, + rootEslintConfig.replace('.base.', '.') + ) + ); + } else { + migrateEslintFile(projectEslintPath, tree); + } } } }); @@ -76,6 +102,7 @@ export function findLintTarget( } function migrateEslintFile(projectEslintPath: string, tree: Tree) { + const baseFile = findEslintFile(tree); if (isEslintConfigSupported(tree)) { if (useFlatConfig(tree)) { let config = tree.read(projectEslintPath, 'utf-8'); @@ -85,7 +112,7 @@ function migrateEslintFile(projectEslintPath: string, tree: Tree) { config = addImportToFlatConfig( config, 'baseConfig', - `${offsetFromRoot(dirname(projectEslintPath))}eslint.base.config.js` + `${offsetFromRoot(dirname(projectEslintPath))}${baseFile}` ); config = addBlockToFlatConfigExport( config, @@ -117,7 +144,7 @@ function migrateEslintFile(projectEslintPath: string, tree: Tree) { json.extends = json.extends || []; const pathToRootConfig = `${offsetFromRoot( dirname(projectEslintPath) - )}.eslintrc.base.json`; + )}${baseFile}`; if (json.extends.indexOf(pathToRootConfig) === -1) { json.extends.push(pathToRootConfig); } diff --git a/packages/eslint/src/generators/utils/eslint-file.ts b/packages/eslint/src/generators/utils/eslint-file.ts index 85e659d190521..c18ea2b8dd900 100644 --- a/packages/eslint/src/generators/utils/eslint-file.ts +++ b/packages/eslint/src/generators/utils/eslint-file.ts @@ -3,10 +3,10 @@ import { names, offsetFromRoot, readJson, - Tree, updateJson, } from '@nx/devkit'; -import { Linter } from 'eslint'; +import type { Tree } from '@nx/devkit'; +import type { Linter } from 'eslint'; import { useFlatConfig } from '../../utils/flat-config'; import { addBlockToFlatConfigExport, diff --git a/packages/workspace/src/generators/convert-to-monorepo/convert-to-monorepo.spec.ts b/packages/workspace/src/generators/convert-to-monorepo/convert-to-monorepo.spec.ts index 9cea3b3d64c27..358f2455d7b10 100644 --- a/packages/workspace/src/generators/convert-to-monorepo/convert-to-monorepo.spec.ts +++ b/packages/workspace/src/generators/convert-to-monorepo/convert-to-monorepo.spec.ts @@ -74,7 +74,6 @@ describe('monorepo generator', () => { // Extracted base config files expect(tree.exists('tsconfig.base.json')).toBeTruthy(); - expect(tree.exists('.eslintrc.base.json')).toBeTruthy(); }); it('should respect nested libraries', async () => { @@ -140,7 +139,6 @@ describe('monorepo generator', () => { // Extracted base config files expect(tree.exists('tsconfig.base.json')).toBeTruthy(); - expect(tree.exists('.eslintrc.base.json')).toBeTruthy(); expect(tree.exists('jest.config.ts')).toBeTruthy(); }); @@ -169,7 +167,6 @@ describe('monorepo generator', () => { // Extracted base config files expect(tree.exists('tsconfig.base.json')).toBeTruthy(); - expect(tree.exists('.eslintrc.base.json')).toBeTruthy(); expect(tree.exists('jest.config.ts')).toBeTruthy(); }); }); diff --git a/packages/workspace/src/generators/convert-to-monorepo/convert-to-monorepo.ts b/packages/workspace/src/generators/convert-to-monorepo/convert-to-monorepo.ts index 8f7c1f2568163..b44523fe14592 100644 --- a/packages/workspace/src/generators/convert-to-monorepo/convert-to-monorepo.ts +++ b/packages/workspace/src/generators/convert-to-monorepo/convert-to-monorepo.ts @@ -19,9 +19,13 @@ export async function monorepoGenerator(tree: Tree, options: {}) { // Need to determine libs vs packages directory base on the type of root project. for (const [, project] of projects) { - if (project.root === '.') rootProject = project; - projectsToMove.unshift(project); // move the root project 1st + if (project.root === '.') { + rootProject = project; + } else { + projectsToMove.push(project); + } } + projectsToMove.unshift(rootProject); // move the root project 1st // Currently, Nx only handles apps+libs or packages. You cannot mix and match them. // If the standalone project is an app (React, Angular, etc), then use apps+libs. diff --git a/packages/workspace/src/generators/move/lib/extract-base-configs.ts b/packages/workspace/src/generators/move/lib/extract-base-configs.ts index 37b9c4affab6d..852c3c652f748 100644 --- a/packages/workspace/src/generators/move/lib/extract-base-configs.ts +++ b/packages/workspace/src/generators/move/lib/extract-base-configs.ts @@ -1,4 +1,9 @@ -import { joinPathFragments, ProjectConfiguration, Tree } from '@nx/devkit'; +import { + getProjects, + joinPathFragments, + ProjectConfiguration, + Tree, +} from '@nx/devkit'; export function maybeExtractTsConfigBase(tree: Tree): void { let extractTsConfigBase: any; @@ -22,12 +27,10 @@ export async function maybeExtractJestConfigBase(tree: Tree): Promise { await jestInitGenerator(tree, {}); } -export function maybeExtractEslintConfigIfRootProject( +export function maybeMigrateEslintConfigIfRootProject( tree: Tree, rootProject: ProjectConfiguration ): void { - if (rootProject.root !== '.') return; - if (tree.exists('.eslintrc.base.json')) return; let migrateConfigToMonorepoStyle: any; try { migrateConfigToMonorepoStyle = require('@nx/' + @@ -35,10 +38,8 @@ export function maybeExtractEslintConfigIfRootProject( } catch { // eslint not installed } - // Only need to handle migrating the root rootProject. - // If other libs/apps exist, then this migration is already done by `@nx/eslint:lint-rootProject` generator. migrateConfigToMonorepoStyle?.( - [rootProject], + Array.from(getProjects(tree).values()), tree, tree.exists(joinPathFragments(rootProject.root, 'jest.config.ts')) || tree.exists(joinPathFragments(rootProject.root, 'jest.config.js')) diff --git a/packages/workspace/src/generators/move/move.spec.ts b/packages/workspace/src/generators/move/move.spec.ts index 3b825105ab8c1..e65032d09233f 100644 --- a/packages/workspace/src/generators/move/move.spec.ts +++ b/packages/workspace/src/generators/move/move.spec.ts @@ -1,4 +1,4 @@ -import { readJson, Tree, updateJson } from '@nx/devkit'; +import { readJson, Tree, updateJson, writeJson } from '@nx/devkit'; import { createTreeWithEmptyWorkspace } from '@nx/devkit/testing'; import { moveGenerator } from './move'; // nx-ignore-next-line @@ -173,7 +173,18 @@ describe('move', () => { // Test that root configs are extracted expect(tree.exists('tsconfig.base.json')).toBeTruthy(); expect(tree.exists('jest.config.ts')).toBeTruthy(); - expect(tree.exists('.eslintrc.base.json')).toBeTruthy(); + expect(tree.exists('.eslintrc.base.json')).not.toBeTruthy(); + expect(tree.exists('.eslintrc.json')).toBeTruthy(); + + // Test that eslint migration was done + expect(readJson(tree, 'my-lib/.eslintrc.json').extends) + .toMatchInlineSnapshot(` + [ + "../.eslintrc.json", + ] + `); + expect(readJson(tree, 'my-lib/.eslintrc.json').plugins).not.toBeDefined(); + expect(readJson(tree, '.eslintrc.json').plugins).toEqual(['@nx']); }); it('should support moving standalone repos', async () => { @@ -190,6 +201,8 @@ describe('move', () => { style: 'css', projectNameAndRootFormat: 'as-provided', }); + expect(readJson(tree, '.eslintrc.json').plugins).toEqual(['@nx']); + expect(readJson(tree, 'e2e/.eslintrc.json').plugins).toEqual(['@nx']); // Test that this does not get moved tree.write('other-lib/index.ts', ''); @@ -200,6 +213,14 @@ describe('move', () => { destination: 'apps/react-app', projectNameAndRootFormat: 'as-provided', }); + + // expect both eslint configs to have been changed + expect(tree.exists('.eslintrc.json')).toBeDefined(); + expect( + readJson(tree, 'apps/react-app/.eslintrc.json').plugins + ).toBeUndefined(); + expect(readJson(tree, 'e2e/.eslintrc.json').plugins).toBeUndefined(); + await moveGenerator(tree, { projectName: 'e2e', updateImportPath: false, @@ -223,6 +244,63 @@ describe('move', () => { `); }); + it('should correctly move standalone repos that have migrated eslint config', async () => { + // Test that these are not moved + tree.write('.gitignore', ''); + tree.write('README.md', ''); + + await applicationGenerator(tree, { + name: 'react-app', + rootProject: true, + unitTestRunner: 'jest', + e2eTestRunner: 'cypress', + linter: 'eslint', + style: 'css', + projectNameAndRootFormat: 'as-provided', + }); + await libraryGenerator(tree, { + name: 'my-lib', + bundler: 'tsc', + buildable: true, + unitTestRunner: 'jest', + linter: 'eslint', + directory: 'my-lib', + projectNameAndRootFormat: 'as-provided', + }); + // assess the correct starting position + expect(tree.exists('.eslintrc.base.json')).toBeTruthy(); + expect(readJson(tree, '.eslintrc.json').plugins).not.toBeDefined(); + expect(readJson(tree, '.eslintrc.json').extends).toEqual([ + 'plugin:@nx/react', + './.eslintrc.base.json', + ]); + expect(readJson(tree, 'e2e/.eslintrc.json').plugins).not.toBeDefined(); + expect(readJson(tree, 'e2e/.eslintrc.json').extends).toEqual([ + 'plugin:cypress/recommended', + '../.eslintrc.base.json', + ]); + + await moveGenerator(tree, { + projectName: 'react-app', + updateImportPath: false, + destination: 'apps/react-app', + projectNameAndRootFormat: 'as-provided', + }); + + // expect both eslint configs to have been changed + expect(tree.exists('.eslintrc.json')).toBeTruthy(); + expect(tree.exists('.eslintrc.base.json')).toBeFalsy(); + + expect(readJson(tree, 'apps/react-app/.eslintrc.json').extends).toEqual([ + 'plugin:@nx/react', + '../../.eslintrc.json', + ]); + expect(readJson(tree, 'e2e/.eslintrc.json').extends).toEqual([ + 'plugin:cypress/recommended', + '../.eslintrc.json', + ]); + }); + it('should move project correctly when --project-name-and-root-format=derived', async () => { await libraryGenerator(tree, { name: 'my-lib', diff --git a/packages/workspace/src/generators/move/move.ts b/packages/workspace/src/generators/move/move.ts index 15f1037038eec..3965f128c0775 100644 --- a/packages/workspace/src/generators/move/move.ts +++ b/packages/workspace/src/generators/move/move.ts @@ -21,7 +21,7 @@ import { updateProjectRootFiles } from './lib/update-project-root-files'; import { updateReadme } from './lib/update-readme'; import { updateStorybookConfig } from './lib/update-storybook-config'; import { - maybeExtractEslintConfigIfRootProject, + maybeMigrateEslintConfigIfRootProject, maybeExtractJestConfigBase, maybeExtractTsConfigBase, } from './lib/extract-base-configs'; @@ -42,7 +42,6 @@ export async function moveGeneratorInternal(tree: Tree, rawSchema: Schema) { if (projectConfig.root === '.') { maybeExtractTsConfigBase(tree); await maybeExtractJestConfigBase(tree); - maybeExtractEslintConfigIfRootProject(tree, projectConfig); // Reload config since it has been updated after extracting base configs projectConfig = readProjectConfiguration(tree, rawSchema.projectName); } @@ -62,6 +61,11 @@ export async function moveGeneratorInternal(tree: Tree, rawSchema: Schema) { updateDefaultProject(tree, schema); updateImplicitDependencies(tree, schema); + if (projectConfig.root === '.') { + // we want to migrate eslint config once the root project files are moved + maybeMigrateEslintConfigIfRootProject(tree, projectConfig); + } + await runAngularPlugin(tree, schema); if (!schema.skipFormat) {