From 6c0f2e760c0d37125ece3c70a5ca8294b98429a7 Mon Sep 17 00:00:00 2001 From: Craigory Coppola Date: Fri, 31 May 2024 18:07:58 -0400 Subject: [PATCH] feat(linter): migrate to create-nodes-v2 --- .../convert-to-inferred.ts | 12 +- packages/eslint/src/generators/init/init.ts | 12 +- packages/eslint/src/plugins/plugin.spec.ts | 97 ++++---- packages/eslint/src/plugins/plugin.ts | 224 ++++++++++++------ packages/nx/src/project-graph/error-types.ts | 7 +- 5 files changed, 221 insertions(+), 131 deletions(-) diff --git a/packages/eslint/src/generators/convert-to-inferred/convert-to-inferred.ts b/packages/eslint/src/generators/convert-to-inferred/convert-to-inferred.ts index 2fef8a342582d..faa9f264e42ca 100644 --- a/packages/eslint/src/generators/convert-to-inferred/convert-to-inferred.ts +++ b/packages/eslint/src/generators/convert-to-inferred/convert-to-inferred.ts @@ -5,8 +5,8 @@ import { type TargetConfiguration, type Tree, } from '@nx/devkit'; -import { createNodes, EslintPluginOptions } from '../../plugins/plugin'; -import { migrateExecutorToPluginV1 } from '@nx/devkit/src/generators/plugin-migrations/executor-to-plugin-migrator'; +import { createNodesV2, EslintPluginOptions } from '../../plugins/plugin'; +import { migrateExecutorToPlugin } from '@nx/devkit/src/generators/plugin-migrations/executor-to-plugin-migrator'; import { targetOptionsToCliMap } from './lib/target-options-map'; import { interpolate } from 'nx/src/tasks-runner/utils'; @@ -19,26 +19,26 @@ export async function convertToInferred(tree: Tree, options: Schema) { const projectGraph = await createProjectGraphAsync(); const migratedProjectsModern = - await migrateExecutorToPluginV1( + await migrateExecutorToPlugin( tree, projectGraph, '@nx/eslint:lint', '@nx/eslint/plugin', (targetName) => ({ targetName }), postTargetTransformer, - createNodes, + createNodesV2, options.project ); const migratedProjectsLegacy = - await migrateExecutorToPluginV1( + await migrateExecutorToPlugin( tree, projectGraph, '@nrwl/linter:eslint', '@nx/eslint/plugin', (targetName) => ({ targetName }), postTargetTransformer, - createNodes, + createNodesV2, options.project ); diff --git a/packages/eslint/src/generators/init/init.ts b/packages/eslint/src/generators/init/init.ts index ae271108cdd4e..600d977058142 100644 --- a/packages/eslint/src/generators/init/init.ts +++ b/packages/eslint/src/generators/init/init.ts @@ -8,10 +8,10 @@ import { Tree, updateNxJson, } from '@nx/devkit'; -import { addPluginV1 } from '@nx/devkit/src/utils/add-plugin'; +import { addPlugin } from '@nx/devkit/src/utils/add-plugin'; import { eslintVersion, nxVersion } from '../../utils/versions'; import { findEslintFile } from '../utils/eslint-file'; -import { createNodes } from '../../plugins/plugin'; +import { createNodesV2 } from '../../plugins/plugin'; import { hasEslintPlugin } from '../utils/plugin'; export interface LinterInitOptions { @@ -73,11 +73,11 @@ export async function initEsLint( ]; if (rootEslintFile && options.addPlugin && !hasPlugin) { - await addPluginV1( + await addPlugin( tree, graph, '@nx/eslint/plugin', - createNodes, + createNodesV2, { targetName: lintTargetNames, }, @@ -94,11 +94,11 @@ export async function initEsLint( updateProductionFileset(tree); if (options.addPlugin) { - await addPluginV1( + await addPlugin( tree, graph, '@nx/eslint/plugin', - createNodes, + createNodesV2, { targetName: lintTargetNames, }, diff --git a/packages/eslint/src/plugins/plugin.spec.ts b/packages/eslint/src/plugins/plugin.spec.ts index 86efd9459f1d0..a23e194834d95 100644 --- a/packages/eslint/src/plugins/plugin.spec.ts +++ b/packages/eslint/src/plugins/plugin.spec.ts @@ -1,13 +1,21 @@ -import { CreateNodesContext } from '@nx/devkit'; +import { CreateNodesContextV2 } from '@nx/devkit'; import { minimatch } from 'minimatch'; import { TempFs } from 'nx/src/internal-testing-utils/temp-fs'; -import { createNodes } from './plugin'; +import { createNodesV2 } from './plugin'; +import { mkdirSync, rmdirSync } from 'fs'; + +jest.mock('nx/src/utils/cache-directory', () => ({ + ...jest.requireActual('nx/src/utils/cache-directory'), + projectGraphCacheDirectory: 'tmp/project-graph-cache', +})); describe('@nx/eslint/plugin', () => { - let context: CreateNodesContext; + let context: CreateNodesContextV2; let tempFs: TempFs; + let configFiles: string[] = []; beforeEach(async () => { + mkdirSync('tmp/project-graph-cache', { recursive: true }); tempFs = new TempFs('eslint-plugin'); context = { nxJsonConfiguration: { @@ -24,7 +32,6 @@ describe('@nx/eslint/plugin', () => { }, }, workspaceRoot: tempFs.tempDir, - configFiles: [], }; }); @@ -32,6 +39,7 @@ describe('@nx/eslint/plugin', () => { jest.resetModules(); tempFs.cleanup(); tempFs = null; + rmdirSync('tmp/project-graph-cache', { recursive: true }); }); it('should not create any nodes when there are no eslint configs', async () => { @@ -39,7 +47,7 @@ describe('@nx/eslint/plugin', () => { 'package.json': `{}`, 'project.json': `{}`, }); - expect(await invokeCreateNodesOnMatchingFiles(context, 'lint')) + expect(await invokeCreateNodesV2OnMatchingFiles(context, 'lint')) .toMatchInlineSnapshot(` { "projects": {}, @@ -53,7 +61,7 @@ describe('@nx/eslint/plugin', () => { '.eslintrc.json': `{}`, 'package.json': `{}`, }); - expect(await invokeCreateNodesOnMatchingFiles(context, 'lint')) + expect(await invokeCreateNodesV2OnMatchingFiles(context, 'lint')) .toMatchInlineSnapshot(` { "projects": {}, @@ -70,7 +78,7 @@ describe('@nx/eslint/plugin', () => { 'project.json': `{}`, }); // NOTE: It should set ESLINT_USE_FLAT_CONFIG to true because of the use of eslint.config.js - expect(await invokeCreateNodesOnMatchingFiles(context, 'lint')) + expect(await invokeCreateNodesV2OnMatchingFiles(context, 'lint')) .toMatchInlineSnapshot(` { "projects": {}, @@ -86,7 +94,7 @@ describe('@nx/eslint/plugin', () => { 'src/index.ts': `console.log('hello world')`, }); // NOTE: The command is specifically targeting the src directory in the case of a standalone Nx workspace - expect(await invokeCreateNodesOnMatchingFiles(context, 'lint')) + expect(await invokeCreateNodesV2OnMatchingFiles(context, 'lint')) .toMatchInlineSnapshot(` { "projects": { @@ -127,7 +135,7 @@ describe('@nx/eslint/plugin', () => { 'lib/index.ts': `console.log('hello world')`, }); // NOTE: The command is specifically targeting the src directory in the case of a standalone Nx workspace - expect(await invokeCreateNodesOnMatchingFiles(context, 'lint')) + expect(await invokeCreateNodesV2OnMatchingFiles(context, 'lint')) .toMatchInlineSnapshot(` { "projects": { @@ -169,7 +177,7 @@ describe('@nx/eslint/plugin', () => { 'src/index.ts': `console.log('hello world')`, }); // NOTE: The command is specifically targeting the src directory in the case of a standalone Nx workspace - expect(await invokeCreateNodesOnMatchingFiles(context, 'lint')) + expect(await invokeCreateNodesV2OnMatchingFiles(context, 'lint')) .toMatchInlineSnapshot(` { "projects": {}, @@ -184,7 +192,7 @@ describe('@nx/eslint/plugin', () => { 'src/index.ts': `console.log('hello world')`, }); // NOTE: The command is specifically targeting the src directory in the case of a standalone Nx workspace - expect(await invokeCreateNodesOnMatchingFiles(context, 'lint')) + expect(await invokeCreateNodesV2OnMatchingFiles(context, 'lint')) .toMatchInlineSnapshot(` { "projects": {}, @@ -199,7 +207,7 @@ describe('@nx/eslint/plugin', () => { // This file is lintable so create the target 'apps/my-app/index.ts': `console.log('hello world')`, }); - expect(await invokeCreateNodesOnMatchingFiles(context, 'lint')) + expect(await invokeCreateNodesV2OnMatchingFiles(context, 'lint')) .toMatchInlineSnapshot(` { "projects": { @@ -240,7 +248,7 @@ describe('@nx/eslint/plugin', () => { // This file is lintable so create the target 'apps/my-app/index.ts': `console.log('hello world')`, }); - expect(await invokeCreateNodesOnMatchingFiles(context, 'lint')) + expect(await invokeCreateNodesV2OnMatchingFiles(context, 'lint')) .toMatchInlineSnapshot(` { "projects": { @@ -285,7 +293,7 @@ describe('@nx/eslint/plugin', () => { 'apps/my-app/config-one.yaml': `...`, 'apps/my-app/config-two.yml': `...`, }); - expect(await invokeCreateNodesOnMatchingFiles(context, 'lint')) + expect(await invokeCreateNodesV2OnMatchingFiles(context, 'lint')) .toMatchInlineSnapshot(` { "projects": {}, @@ -304,7 +312,7 @@ describe('@nx/eslint/plugin', () => { 'apps/my-app/config-one.yaml': `...`, 'apps/my-app/config-two.yml': `...`, }); - expect(await invokeCreateNodesOnMatchingFiles(context, 'lint')) + expect(await invokeCreateNodesV2OnMatchingFiles(context, 'lint')) .toMatchInlineSnapshot(` { "projects": {}, @@ -319,7 +327,7 @@ describe('@nx/eslint/plugin', () => { // This file is lintable so create the target 'apps/my-app/index.ts': `console.log('hello world')`, }); - expect(await invokeCreateNodesOnMatchingFiles(context, 'lint')) + expect(await invokeCreateNodesV2OnMatchingFiles(context, 'lint')) .toMatchInlineSnapshot(` { "projects": {}, @@ -334,7 +342,7 @@ describe('@nx/eslint/plugin', () => { // This file is lintable so create the target 'apps/my-app/index.ts': `console.log('hello world')`, }); - expect(await invokeCreateNodesOnMatchingFiles(context, 'lint')) + expect(await invokeCreateNodesV2OnMatchingFiles(context, 'lint')) .toMatchInlineSnapshot(` { "projects": {}, @@ -353,7 +361,7 @@ describe('@nx/eslint/plugin', () => { 'libs/my-lib/project.json': `{}`, 'libs/my-lib/index.ts': `console.log('hello world')`, }); - expect(await invokeCreateNodesOnMatchingFiles(context, 'lint')) + expect(await invokeCreateNodesV2OnMatchingFiles(context, 'lint')) .toMatchInlineSnapshot(` { "projects": { @@ -423,7 +431,7 @@ describe('@nx/eslint/plugin', () => { 'libs/my-lib/project.json': `{}`, 'libs/my-lib/index.ts': `console.log('hello world')`, }); - expect(await invokeCreateNodesOnMatchingFiles(context, 'lint')) + expect(await invokeCreateNodesV2OnMatchingFiles(context, 'lint')) .toMatchInlineSnapshot(` { "projects": {}, @@ -440,7 +448,7 @@ describe('@nx/eslint/plugin', () => { 'libs/my-lib/project.json': `{}`, 'libs/my-lib/index.ts': `console.log('hello world')`, }); - expect(await invokeCreateNodesOnMatchingFiles(context, 'lint')) + expect(await invokeCreateNodesV2OnMatchingFiles(context, 'lint')) .toMatchInlineSnapshot(` { "projects": {}, @@ -462,7 +470,7 @@ describe('@nx/eslint/plugin', () => { 'libs/my-lib/index.ts': `console.log('hello world')`, }); // NOTE: The nested projects have the root level config as an input to their lint targets - expect(await invokeCreateNodesOnMatchingFiles(context, 'lint')) + expect(await invokeCreateNodesV2OnMatchingFiles(context, 'lint')) .toMatchInlineSnapshot(` { "projects": { @@ -531,7 +539,7 @@ describe('@nx/eslint/plugin', () => { 'apps/myapp/index.ts': 'console.log("hello world")', }); // NOTE: The nested projects have the root level config as an input to their lint targets - expect(await invokeCreateNodesOnMatchingFiles(context, 'lint')) + expect(await invokeCreateNodesV2OnMatchingFiles(context, 'lint')) .toMatchInlineSnapshot(` { "projects": { @@ -579,7 +587,7 @@ describe('@nx/eslint/plugin', () => { 'apps/myapp/nested/mylib/project.json': '{}', 'apps/myapp/nested/mylib/index.ts': 'console.log("hello world")', }); - expect(await invokeCreateNodesOnMatchingFiles(context, 'lint')) + expect(await invokeCreateNodesV2OnMatchingFiles(context, 'lint')) .toMatchInlineSnapshot(` { "projects": { @@ -617,27 +625,30 @@ describe('@nx/eslint/plugin', () => { function createFiles(fileSys: Record) { tempFs.createFilesSync(fileSys); - // @ts-expect-error update otherwise readonly property for testing - context.configFiles = getMatchingFiles(Object.keys(fileSys)); + configFiles = getMatchingFiles(Object.keys(fileSys)); } -}); -function getMatchingFiles(allConfigFiles: string[]): string[] { - return allConfigFiles.filter((file) => - minimatch(file, createNodes[0], { dot: true }) - ); -} + function getMatchingFiles(allConfigFiles: string[]): string[] { + return allConfigFiles.filter((file) => + minimatch(file, createNodesV2[0], { dot: true }) + ); + } -async function invokeCreateNodesOnMatchingFiles( - context: CreateNodesContext, - targetName: string -) { - const aggregateProjects: Record = {}; - for (const file of context.configFiles) { - const nodes = await createNodes[1](file, { targetName }, context); - Object.assign(aggregateProjects, nodes.projects); + async function invokeCreateNodesV2OnMatchingFiles( + context: CreateNodesContextV2, + targetName: string + ) { + const aggregateProjects: Record = {}; + const results = await createNodesV2[1]( + configFiles, + { targetName }, + context + ); + for (const [, nodes] of results) { + Object.assign(aggregateProjects, nodes.projects); + } + return { + projects: aggregateProjects, + }; } - return { - projects: aggregateProjects, - }; -} +}); diff --git a/packages/eslint/src/plugins/plugin.ts b/packages/eslint/src/plugins/plugin.ts index 60943ba4474ca..4df5a603ba51d 100644 --- a/packages/eslint/src/plugins/plugin.ts +++ b/packages/eslint/src/plugins/plugin.ts @@ -2,12 +2,20 @@ import { CreateNodes, CreateNodesContext, CreateNodesResult, + CreateNodesV2, TargetConfiguration, + createNodesFromFiles, + logger, + readJsonFile, + writeJsonFile, } from '@nx/devkit'; import { existsSync } from 'node:fs'; import { dirname, join, normalize, sep } from 'node:path'; import { combineGlobPatterns } from 'nx/src/utils/globs'; -import { globWithWorkspaceContext } from 'nx/src/utils/workspace-context'; +import { + globWithWorkspaceContext, + hashWithWorkspaceContext, +} from 'nx/src/utils/workspace-context'; import { ESLINT_CONFIG_FILENAMES, baseEsLintConfigFile, @@ -16,6 +24,8 @@ import { } from '../utils/config-file'; import { resolveESLintClass } from '../utils/resolve-eslint-class'; import { gte } from 'semver'; +import { projectGraphCacheDirectory } from 'nx/src/utils/cache-directory'; +import { hashObject } from 'nx/src/hasher/file-hasher'; export interface EslintPluginOptions { targetName?: string; @@ -23,91 +33,155 @@ export interface EslintPluginOptions { } const DEFAULT_EXTENSIONS = ['ts', 'tsx', 'js', 'jsx', 'html', 'vue']; +const ESLING_CONFIG_GLOB = combineGlobPatterns([ + ...ESLINT_CONFIG_FILENAMES.map((f) => `**/${f}`), + baseEsLintConfigFile, + baseEsLintFlatConfigFile, +]); -export const createNodes: CreateNodes = [ - combineGlobPatterns([ +type EslintProjects = Awaited>; + +function readTargetsCache(cachePath: string): Record { + return existsSync(cachePath) ? readJsonFile(cachePath) : {}; +} + +function writeTargetsToCache( + cachePath: string, + results: Record +) { + writeJsonFile(cachePath, results); +} + +const internalCreateNodes = async ( + configFilePath: string, + options: EslintPluginOptions, + context: CreateNodesContext, + projectsCache: Record +): Promise => { + // This isn't technically correct, but should be good enough for + // consistent results. We'll rebuild project config anytime a project.json, + // package.json, or eslint config changes. This would only be incorrect if + // using flat config and changing imported files or similar. + const hash = await hashWithWorkspaceContext(context.workspaceRoot, [ ...ESLINT_CONFIG_FILENAMES.map((f) => `**/${f}`), baseEsLintConfigFile, baseEsLintFlatConfigFile, - ]), - async (configFilePath, options, context) => { - options = normalizeOptions(options); - const configDir = dirname(configFilePath); - - // Ensure that configFiles are set, e2e-run fails due to them being undefined in CI (does not occur locally) - // TODO(JamesHenry): Further troubleshoot this in CI - (context as any).configFiles = context.configFiles ?? []; - - // Create a Set of all the directories containing eslint configs, and a - // list of globs to exclude from child projects - const eslintRoots = new Set(); - const nestedEslintRootPatterns: string[] = []; - for (const configFile of context.configFiles) { - const eslintRootDir = dirname(configFile); - eslintRoots.add(eslintRootDir); - - if (eslintRootDir !== configDir && isSubDir(configDir, eslintRootDir)) { - nestedEslintRootPatterns.push(`${eslintRootDir}/**/*`); - } + '**/project.json', + '**/package.json', + 'project.json', + 'package.json', + ]); + + if (projectsCache[hash]) { + return { projects: projectsCache[hash] }; + } + + options = normalizeOptions(options); + const configDir = dirname(configFilePath); + + // Ensure that configFiles are set, e2e-run fails due to them being undefined in CI (does not occur locally) + // TODO(JamesHenry): Further troubleshoot this in CI + (context as any).configFiles = context.configFiles ?? []; + + // Create a Set of all the directories containing eslint configs, and a + // list of globs to exclude from child projects + const eslintRoots = new Set(); + const nestedEslintRootPatterns: string[] = []; + for (const configFile of context.configFiles) { + const eslintRootDir = dirname(configFile); + eslintRoots.add(eslintRootDir); + + if (eslintRootDir !== configDir && isSubDir(configDir, eslintRootDir)) { + nestedEslintRootPatterns.push(`${eslintRootDir}/**/*`); } + } - const projectFiles = await globWithWorkspaceContext( - context.workspaceRoot, - [ - 'project.json', - 'package.json', - '**/project.json', - '**/package.json', - ].map((f) => join(configDir, f)), - nestedEslintRootPatterns.length ? nestedEslintRootPatterns : undefined - ); - // dedupe and sort project roots by depth for more efficient traversal - const dedupedProjectRoots = Array.from( - new Set(projectFiles.map((f) => dirname(f))) - ).sort((a, b) => (a !== b && isSubDir(a, b) ? -1 : 1)); - const excludePatterns = dedupedProjectRoots.map((root) => `${root}/**/*`); - - const ESLint = await resolveESLintClass(isFlatConfig(configFilePath)); - const eslintVersion = ESLint.version; - const childProjectRoots = new Set(); - - await Promise.all( - dedupedProjectRoots.map(async (childProjectRoot, index) => { - // anything after is either a nested project or a sibling project, can be excluded - const nestedProjectRootPatterns = excludePatterns.slice(index + 1); - - // Ignore project roots where the project does not contain any lintable files - const lintableFiles = await globWithWorkspaceContext( - context.workspaceRoot, - [join(childProjectRoot, `**/*.{${options.extensions.join(',')}}`)], - // exclude nested eslint roots and nested project roots - [...nestedEslintRootPatterns, ...nestedProjectRootPatterns] - ); - const eslint = new ESLint({ - cwd: join(context.workspaceRoot, childProjectRoot), - }); - for (const file of lintableFiles) { - if ( - !(await eslint.isPathIgnored(join(context.workspaceRoot, file))) - ) { - childProjectRoots.add(childProjectRoot); - break; - } + const projectFiles = await globWithWorkspaceContext( + context.workspaceRoot, + ['project.json', 'package.json', '**/project.json', '**/package.json'].map( + (f) => join(configDir, f) + ), + nestedEslintRootPatterns.length ? nestedEslintRootPatterns : undefined + ); + // dedupe and sort project roots by depth for more efficient traversal + const dedupedProjectRoots = Array.from( + new Set(projectFiles.map((f) => dirname(f))) + ).sort((a, b) => (a !== b && isSubDir(a, b) ? -1 : 1)); + const excludePatterns = dedupedProjectRoots.map((root) => `${root}/**/*`); + + const ESLint = await resolveESLintClass(isFlatConfig(configFilePath)); + const eslintVersion = ESLint.version; + const childProjectRoots = new Set(); + + await Promise.all( + dedupedProjectRoots.map(async (childProjectRoot, index) => { + // anything after is either a nested project or a sibling project, can be excluded + const nestedProjectRootPatterns = excludePatterns.slice(index + 1); + + // Ignore project roots where the project does not contain any lintable files + const lintableFiles = await globWithWorkspaceContext( + context.workspaceRoot, + [join(childProjectRoot, `**/*.{${options.extensions.join(',')}}`)], + // exclude nested eslint roots and nested project roots + [...nestedEslintRootPatterns, ...nestedProjectRootPatterns] + ); + const eslint = new ESLint({ + cwd: join(context.workspaceRoot, childProjectRoot), + }); + for (const file of lintableFiles) { + if (!(await eslint.isPathIgnored(join(context.workspaceRoot, file)))) { + childProjectRoots.add(childProjectRoot); + break; } - }) - ); + } + }) + ); - const uniqueChildProjectRoots = Array.from(childProjectRoots); + const uniqueChildProjectRoots = Array.from(childProjectRoots); - return { - projects: getProjectsUsingESLintConfig( - configFilePath, - uniqueChildProjectRoots, - eslintVersion, + projectsCache[hash] = getProjectsUsingESLintConfig( + configFilePath, + uniqueChildProjectRoots, + eslintVersion, + options, + context + ); + + return { + projects: projectsCache[hash], + }; +}; + +export const createNodesV2: CreateNodesV2 = [ + ESLING_CONFIG_GLOB, + async (configFiles, options, context) => { + const optionsHash = hashObject(options); + const cachePath = join( + projectGraphCacheDirectory, + `eslint-${optionsHash}.hash` + ); + const targetsCache = readTargetsCache(cachePath); + try { + return await createNodesFromFiles( + (configFile, options, context) => + internalCreateNodes(configFile, options, context, targetsCache), + configFiles, options, context - ), - }; + ); + } finally { + writeTargetsToCache(cachePath, targetsCache); + } + }, +]; + +export const createNodes: CreateNodes = [ + ESLING_CONFIG_GLOB, + (configFilePath, options, context) => { + logger.warn( + '`createNodes` is deprecated. Update your plugin to utilize createNodesV2 instead. In Nx 20, this will change to the createNodesV2 API.' + ); + return internalCreateNodes(configFilePath, options, context, {}); }, ]; diff --git a/packages/nx/src/project-graph/error-types.ts b/packages/nx/src/project-graph/error-types.ts index 9cda2cf0230b9..1d86fae1cba11 100644 --- a/packages/nx/src/project-graph/error-types.ts +++ b/packages/nx/src/project-graph/error-types.ts @@ -224,7 +224,12 @@ export class AggregateCreateNodesError extends Error { public readonly errors: Array<[file: string | null, error: Error]>, public readonly partialResults: Awaited> ) { - super('Failed to create nodes'); + super( + [ + 'Failed to create nodes', + ...errors.map(([file, e]) => ` - ${file}: ${e.message}`), + ].join() + ); this.name = this.constructor.name; } }