From 4f9f0c60f91d700b76248d65f44a7a39d3f0c223 Mon Sep 17 00:00:00 2001 From: Emily Xiong Date: Mon, 11 Nov 2024 12:05:20 -0500 Subject: [PATCH] feat(core): Handle when an existing plugin begins to fail with the new imported project --- .../devkit/AggregateCreateNodesError.md | 7 + packages/nx/src/command-line/import/import.ts | 53 +++++-- .../check-compatible-with-plugins.spec.ts | 138 +++++++++++++++++ .../check-compatible-with-plugins.ts | 145 ++++++++++++++++++ packages/nx/src/project-graph/error-types.ts | 11 +- .../utils/project-configuration-utils.ts | 28 ++-- packages/nx/src/utils/params.ts | 3 - packages/playwright/src/plugins/plugin.ts | 6 +- packages/vite/src/plugins/plugin.ts | 2 +- 9 files changed, 363 insertions(+), 30 deletions(-) create mode 100644 packages/nx/src/command-line/init/implementation/check-compatible-with-plugins.spec.ts create mode 100644 packages/nx/src/command-line/init/implementation/check-compatible-with-plugins.ts diff --git a/docs/generated/devkit/AggregateCreateNodesError.md b/docs/generated/devkit/AggregateCreateNodesError.md index 2fc55fbe796d5..bcff6b91acc07 100644 --- a/docs/generated/devkit/AggregateCreateNodesError.md +++ b/docs/generated/devkit/AggregateCreateNodesError.md @@ -22,6 +22,7 @@ It allows Nx to recieve partial results and continue processing for better UX. - [message](../../devkit/documents/AggregateCreateNodesError#message): string - [name](../../devkit/documents/AggregateCreateNodesError#name): string - [partialResults](../../devkit/documents/AggregateCreateNodesError#partialresults): CreateNodesResultV2 +- [pluginIndex](../../devkit/documents/AggregateCreateNodesError#pluginindex): number - [stack](../../devkit/documents/AggregateCreateNodesError#stack): string - [prepareStackTrace](../../devkit/documents/AggregateCreateNodesError#preparestacktrace): Function - [stackTraceLimit](../../devkit/documents/AggregateCreateNodesError#stacktracelimit): number @@ -124,6 +125,12 @@ The partial results of the `createNodesV2` function. This should be the results --- +### pluginIndex + +• **pluginIndex**: `number` + +--- + ### stack • `Optional` **stack**: `string` diff --git a/packages/nx/src/command-line/import/import.ts b/packages/nx/src/command-line/import/import.ts index 99386c0a24a61..1c25625b45308 100644 --- a/packages/nx/src/command-line/import/import.ts +++ b/packages/nx/src/command-line/import/import.ts @@ -29,6 +29,10 @@ import { configurePlugins, runPackageManagerInstallPlugins, } from '../init/configure-plugins'; +import { + checkCompatibleWithPlugins, + updatePluginsInNxJson, +} from '../init/implementation/check-compatible-with-plugins'; const importRemoteName = '__tmp_nx_import__'; @@ -286,21 +290,30 @@ export async function importHandler(options: ImportOptions) { packageManager, destinationGitClient ); - - if (installed && plugins.length > 0) { - installed = await runPluginsInstall(plugins, pmc, destinationGitClient); - if (installed) { - const { succeededPlugins } = await configurePlugins( - plugins, - updatePackageScripts, - pmc, - workspaceRoot, - verbose - ); - if (succeededPlugins.length > 0) { + if (installed) { + // Check compatibility with existing plugins for the workspace included new imported projects + if (nxJson.plugins?.length > 0) { + const incompatiblePlugins = await checkCompatibleWithPlugins(); + if (Object.keys(incompatiblePlugins).length > 0) { + updatePluginsInNxJson(workspaceRoot, incompatiblePlugins); await destinationGitClient.amendCommit(); } } + if (plugins.length > 0) { + installed = await runPluginsInstall(plugins, pmc, destinationGitClient); + if (installed) { + const { succeededPlugins } = await configurePlugins( + plugins, + updatePackageScripts, + pmc, + workspaceRoot, + verbose + ); + if (succeededPlugins.length > 0) { + await destinationGitClient.amendCommit(); + } + } + } } console.log(await destinationGitClient.showStat()); @@ -313,6 +326,22 @@ export async function importHandler(options: ImportOptions) { `You may need to run "${pmc.install}" manually to resolve the issue. The error is logged above.`, ], }); + if (plugins.length > 0) { + output.error({ + title: `Failed to install plugins`, + bodyLines: [ + 'The following plugins were not installed:', + ...plugins.map((p) => `- ${chalk.bold(p)}`), + ], + }); + output.error({ + title: `To install the plugins manually`, + bodyLines: [ + 'You may need to run commands to install the plugins:', + ...plugins.map((p) => `- ${chalk.bold(pmc.exec + ' nx add ' + p)}`), + ], + }); + } } if (source != destination) { diff --git a/packages/nx/src/command-line/init/implementation/check-compatible-with-plugins.spec.ts b/packages/nx/src/command-line/init/implementation/check-compatible-with-plugins.spec.ts new file mode 100644 index 0000000000000..b4a22a1d0e760 --- /dev/null +++ b/packages/nx/src/command-line/init/implementation/check-compatible-with-plugins.spec.ts @@ -0,0 +1,138 @@ +import { + AggregateCreateNodesError, + CreateMetadataError, + MergeNodesError, + ProjectGraphError, + ProjectsWithNoNameError, +} from '../../../project-graph/error-types'; +import { checkCompatibleWithPlugins } from './check-compatible-with-plugins'; +import { createProjectGraphAsync } from '../../../project-graph/project-graph'; + +jest.mock('../../../project-graph/project-graph', () => ({ + createProjectGraphAsync: jest.fn(), +})); + +describe('checkCompatibleWithPlugins', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should return empty object if no errors are thrown', async () => { + (createProjectGraphAsync as any).mockReturnValueOnce(Promise.resolve({})); + const result = await checkCompatibleWithPlugins(); + expect(result).toEqual({}); + }); + + it('should return empty object if error is not ProjectConfigurationsError', async () => { + (createProjectGraphAsync as any).mockReturnValueOnce( + Promise.reject(new Error('random error')) + ); + const result = await checkCompatibleWithPlugins(); + expect(result).toEqual({}); + }); + + it('should return empty object if error is ProjectsWithNoNameError', async () => { + (createProjectGraphAsync as any).mockReturnValueOnce( + Promise.reject( + new ProjectGraphError( + [ + new ProjectsWithNoNameError([], { + project1: { root: 'root1' }, + }), + ], + undefined, + undefined + ) + ) + ); + const result = await checkCompatibleWithPlugins(); + expect(result).toEqual({}); + }); + + it('should return incompatible plugin with excluded files if error is AggregateCreateNodesError', async () => { + const error = new AggregateCreateNodesError( + [ + ['file1', undefined], + ['file2', undefined], + ], + [] + ); + error.pluginIndex = 0; + (createProjectGraphAsync as any).mockReturnValueOnce( + Promise.reject(new ProjectGraphError([error], undefined, undefined)) + ); + const result = await checkCompatibleWithPlugins(); + expect(result).toEqual({ + 0: [ + { file: 'file1', error: undefined }, + { file: 'file2', error: undefined }, + ], + }); + }); + + it('should return true if error is MergeNodesError', async () => { + let error = new MergeNodesError({ + file: 'file2', + pluginName: 'plugin2', + error: new Error(), + pluginIndex: 1, + }); + (createProjectGraphAsync as any).mockReturnValueOnce( + Promise.reject(new ProjectGraphError([error], undefined, undefined)) + ); + const result = await checkCompatibleWithPlugins(); + expect(result).toEqual({ 1: [{ error, file: 'file2' }] }); + }); + + it('should handle multiple errors', async () => { + const mergeNodesError = new MergeNodesError({ + file: 'file2', + pluginName: 'plugin2', + error: new Error(), + pluginIndex: 2, + }); + const aggregateError0 = new AggregateCreateNodesError( + [ + ['file1', undefined], + ['file2', undefined], + ], + [] + ); + aggregateError0.pluginIndex = 0; + const aggregateError2 = new AggregateCreateNodesError( + [ + ['file3', undefined], + ['file4', undefined], + ], + [] + ); + aggregateError2.pluginIndex = 2; + (createProjectGraphAsync as any).mockReturnValueOnce( + Promise.reject( + new ProjectGraphError( + [ + new ProjectsWithNoNameError([], { + project1: { root: 'root1' }, + }), + new CreateMetadataError(new Error(), 'file1'), + new AggregateCreateNodesError([], []), + aggregateError0, + mergeNodesError, + aggregateError2, + ], + undefined, + undefined + ) + ) + ); + const result = await checkCompatibleWithPlugins(); + expect(result).toEqual({ + 0: [{ file: 'file1' }, { file: 'file2' }], + 2: [ + { file: 'file2', error: mergeNodesError }, + { file: 'file3' }, + { file: 'file4' }, + ], + }); + }); +}); diff --git a/packages/nx/src/command-line/init/implementation/check-compatible-with-plugins.ts b/packages/nx/src/command-line/init/implementation/check-compatible-with-plugins.ts new file mode 100644 index 0000000000000..3f2ccf40c2da4 --- /dev/null +++ b/packages/nx/src/command-line/init/implementation/check-compatible-with-plugins.ts @@ -0,0 +1,145 @@ +import { existsSync } from 'node:fs'; +import { join } from 'node:path'; +import { bold } from 'chalk'; + +import { NxJsonConfiguration } from '../../../config/nx-json'; +import { + isAggregateCreateNodesError, + isMergeNodesError, + isProjectsWithNoNameError, + ProjectGraphError, +} from '../../../project-graph/error-types'; +import { workspaceRoot } from '../../../utils/workspace-root'; +import { readJsonFile, writeJsonFile } from '../../../utils/fileutils'; +import { output } from '../../../utils/output'; +import { createProjectGraphAsync } from '../../../project-graph/project-graph'; + +export interface IncompatibleFiles { + [pluginIndex: number]: { file: string; error?: any }[]; +} + +/** + * This function checks if the imported project is compatible with the plugins. + * @returns a map of plugin names to files that are incompatible with the plugins + */ +export async function checkCompatibleWithPlugins(): Promise { + let pluginToExcludeFiles: IncompatibleFiles = {}; + try { + await createProjectGraphAsync(); + } catch (projectGraphError) { + if (projectGraphError instanceof ProjectGraphError) { + projectGraphError.getErrors()?.forEach((error) => { + const { pluginIndex, excludeFiles } = + findPluginAndFilesWithError(error) ?? {}; + if (pluginIndex !== undefined && excludeFiles?.length) { + pluginToExcludeFiles[pluginIndex] ??= []; + pluginToExcludeFiles[pluginIndex].push(...excludeFiles); + } else if (!isProjectsWithNoNameError(error)) { + // print error if it is not ProjectsWithNoNameError and unable to exclude files + output.error({ + title: error.message, + bodyLines: error.stack?.split('\n'), + }); + } + }); + } else { + output.error({ + title: + 'Failed to process project graph. Run "nx reset" to fix this. Please report the issue if you keep seeing it.', + bodyLines: projectGraphError.stack?.split('\n'), + }); + } + } + return pluginToExcludeFiles; +} + +/** + * This function finds the plugin name and files that caused the error. + * @param error the error to find the plugin name and files for + * @returns pluginName and excludeFiles if found, otherwise undefined + */ +function findPluginAndFilesWithError( + error: any +): + | { pluginIndex: number; excludeFiles: { file: string; error?: any }[] } + | undefined { + let pluginIndex: number | undefined; + let excludeFiles: { file: string; error?: any }[] = []; + if (isAggregateCreateNodesError(error)) { + pluginIndex = error.pluginIndex; + excludeFiles = + error.errors?.map((error) => { + return { + file: error?.[0], + error: error?.[1], + }; + }) ?? []; + } else if (isMergeNodesError(error)) { + pluginIndex = error.pluginIndex; + excludeFiles = [ + { + file: error.file, + error: error, + }, + ]; + } + excludeFiles = excludeFiles.filter(Boolean); + return { + pluginIndex, + excludeFiles, + }; +} + +/** + * This function updates the plugins in the nx.json file with the given plugin names and files to exclude. + */ +export function updatePluginsInNxJson( + root: string = workspaceRoot, + pluginToExcludeFiles: IncompatibleFiles +): void { + const nxJsonPath = join(root, 'nx.json'); + if (!existsSync(nxJsonPath)) { + return; + } + let nxJson: NxJsonConfiguration; + try { + nxJson = readJsonFile(nxJsonPath); + } catch { + // If there is an error reading the nx.json file, no need to update it + return; + } + if (!Object.keys(pluginToExcludeFiles)?.length || !nxJson?.plugins?.length) { + return; + } + Object.entries(pluginToExcludeFiles).forEach( + ([pluginIndex, excludeFiles]) => { + let plugin = nxJson.plugins[pluginIndex]; + if (!plugin || excludeFiles.length === 0) { + return; + } + if (typeof plugin === 'string') { + plugin = { plugin }; + } + output.warn({ + title: `The following files were incompatible with ${plugin.plugin} and has been excluded for now:`, + bodyLines: excludeFiles + .map((file: { file: string; error?: any }) => { + const output = [` - ${bold(file.file)}`]; + if (file.error?.message) { + output.push(` ${file.error.message}`); + } + return output; + }) + .flat(), + }); + + const excludes = new Set(plugin.exclude ?? []); + excludeFiles.forEach((file) => { + excludes.add(file.file); + }); + plugin.exclude = Array.from(excludes); + nxJson.plugins[pluginIndex] = plugin; + } + ); + writeJsonFile(nxJsonPath, nxJson); +} diff --git a/packages/nx/src/project-graph/error-types.ts b/packages/nx/src/project-graph/error-types.ts index 093bcff466a7f..c62ab91237a95 100644 --- a/packages/nx/src/project-graph/error-types.ts +++ b/packages/nx/src/project-graph/error-types.ts @@ -214,6 +214,7 @@ export function isProjectConfigurationsError( * It allows Nx to recieve partial results and continue processing for better UX. */ export class AggregateCreateNodesError extends Error { + public pluginIndex: number | undefined; /** * Throwing this error from a `createNodesV2` function will allow Nx to continue processing and recieve partial results from your plugin. * @example @@ -296,22 +297,30 @@ export function formatAggregateCreateNodesError( export class MergeNodesError extends Error { file: string; pluginName: string; + pluginIndex: number; constructor({ file, pluginName, error, + pluginIndex, }: { file: string; pluginName: string; error: Error; + pluginIndex?: number; }) { - const msg = `The nodes created from ${file} by the "${pluginName}" could not be merged into the project graph:`; + const msg = `The nodes created from ${file} by the "${pluginName}" ${ + pluginIndex === undefined + ? '' + : `at index ${pluginIndex} in nx.json#plugins ` + }could not be merged into the project graph.`; super(msg, { cause: error }); this.name = this.constructor.name; this.file = file; this.pluginName = pluginName; + this.pluginIndex = pluginIndex; this.stack = `${this.message}\n${indentString( formatErrorStackAndCause(error), 2 diff --git a/packages/nx/src/project-graph/utils/project-configuration-utils.ts b/packages/nx/src/project-graph/utils/project-configuration-utils.ts index d779132c06b91..30d68374fed75 100644 --- a/packages/nx/src/project-graph/utils/project-configuration-utils.ts +++ b/packages/nx/src/project-graph/utils/project-configuration-utils.ts @@ -359,7 +359,14 @@ export async function createProjectConfigurations( `Creating project graph nodes with ${plugins.length} plugins` ); - const results: Array> = []; + const results: Promise< + (readonly [ + plugin: string, + file: string, + result: CreateNodesResult, + index?: number + ])[] + >[] = []; const errors: Array< | AggregateCreateNodesError | MergeNodesError @@ -368,12 +375,10 @@ export async function createProjectConfigurations( > = []; // We iterate over plugins first - this ensures that plugins specified first take precedence. - for (const { - createNodes: createNodesTuple, - include, - exclude, - name: pluginName, - } of plugins) { + for (const [ + index, + { createNodes: createNodesTuple, include, exclude, name: pluginName }, + ] of plugins.entries()) { const [pattern, createNodes] = createNodesTuple ?? []; if (!pattern) { @@ -399,11 +404,12 @@ export async function createProjectConfigurations( : // This represents a single plugin erroring out with a hard error. new AggregateCreateNodesError([[null, e]], []); formatAggregateCreateNodesError(error, pluginName); + error.pluginIndex = index; // This represents a single plugin erroring out with a hard error. errors.push(error); // The plugin didn't return partial results, so we return an empty array. return error.partialResults.map( - (r) => [pluginName, r[0], r[1]] as const + (r) => [pluginName, r[0], r[1], index] as const ); }) .finally(() => { @@ -451,7 +457,8 @@ function mergeCreateNodesResults( results: (readonly [ plugin: string, file: string, - result: CreateNodesResult + result: CreateNodesResult, + index?: number ])[][], nxJsonConfiguration: NxJsonConfiguration, errors: ( @@ -470,7 +477,7 @@ function mergeCreateNodesResults( > = {}; for (const result of results.flat()) { - const [pluginName, file, nodes] = result; + const [pluginName, file, nodes, index] = result; const { projects: projectNodes, externalNodes: pluginExternalNodes } = nodes; @@ -499,6 +506,7 @@ function mergeCreateNodesResults( file, pluginName, error, + pluginIndex: index, }) ); } diff --git a/packages/nx/src/utils/params.ts b/packages/nx/src/utils/params.ts index 562736183969e..3ac0bfa313f8b 100644 --- a/packages/nx/src/utils/params.ts +++ b/packages/nx/src/utils/params.ts @@ -4,9 +4,6 @@ import type { ProjectsConfigurations, TargetConfiguration, } from '../config/workspace-json-project-json'; -import { output } from './output'; -import type { ProjectGraphError } from '../project-graph/error-types'; -import { daemonClient } from '../daemon/client/client'; type PropertyDescription = { type?: string | string[]; diff --git a/packages/playwright/src/plugins/plugin.ts b/packages/playwright/src/plugins/plugin.ts index 1786f82e27673..250adb3c1db29 100644 --- a/packages/playwright/src/plugins/plugin.ts +++ b/packages/playwright/src/plugins/plugin.ts @@ -113,7 +113,7 @@ async function createNodesInternal( const hash = await calculateHashForCreateNodes( projectRoot, - options, + normalizedOptions, context, [getLockFileName(detectPackageManager(context.workspaceRoot))] ); @@ -354,8 +354,8 @@ function createMatcher(pattern: string | RegExp | Array) { function normalizeOptions(options: PlaywrightPluginOptions): NormalizedOptions { return { ...options, - targetName: options.targetName ?? 'e2e', - ciTargetName: options.ciTargetName ?? 'e2e-ci', + targetName: options?.targetName ?? 'e2e', + ciTargetName: options?.ciTargetName ?? 'e2e-ci', }; } diff --git a/packages/vite/src/plugins/plugin.ts b/packages/vite/src/plugins/plugin.ts index a684cffe384f0..33654403fe7b1 100644 --- a/packages/vite/src/plugins/plugin.ts +++ b/packages/vite/src/plugins/plugin.ts @@ -157,7 +157,7 @@ async function createNodesInternal( // If project is buildable, then the project type. // If it is not buildable, then leave it to other plugins/project.json to set the project type. - if (project.targets[options.buildTargetName]) { + if (project.targets[normalizedOptions.buildTargetName]) { project.projectType = isLibrary ? 'library' : 'application'; }