Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): handle existing plugins failed with imported project #28893

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion docs/generated/devkit/AggregateCreateNodesError.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
- [pluginName](../../devkit/documents/AggregateCreateNodesError#pluginname): string
- [stack](../../devkit/documents/AggregateCreateNodesError#stack): string
- [prepareStackTrace](../../devkit/documents/AggregateCreateNodesError#preparestacktrace): Function
- [stackTraceLimit](../../devkit/documents/AggregateCreateNodesError#stacktracelimit): number
Expand All @@ -34,7 +35,7 @@ It allows Nx to recieve partial results and continue processing for better UX.

### constructor

• **new AggregateCreateNodesError**(`errors`, `partialResults`): [`AggregateCreateNodesError`](../../devkit/documents/AggregateCreateNodesError)
• **new AggregateCreateNodesError**(`errors`, `partialResults`, `pluginName?`): [`AggregateCreateNodesError`](../../devkit/documents/AggregateCreateNodesError)

Throwing this error from a `createNodesV2` function will allow Nx to continue processing and recieve partial results from your plugin.

Expand All @@ -44,6 +45,7 @@ Throwing this error from a `createNodesV2` function will allow Nx to continue pr
| :--------------- | :------------------------------------------------------------------ | :--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `errors` | [file: string, error: Error][] | An array of tuples that represent errors encountered when processing a given file. An example entry might look like ['path/to/project.json', [Error: 'Invalid JSON. Unexpected token 'a' in JSON at position 0]] |
| `partialResults` | [`CreateNodesResultV2`](../../devkit/documents/CreateNodesResultV2) | The partial results of the `createNodesV2` function. This should be the results for each file that didn't encounter an issue. |
| `pluginName?` | `string` | - |

#### Returns

Expand Down Expand Up @@ -124,6 +126,12 @@ The partial results of the `createNodesV2` function. This should be the results

---

### pluginName

• `Optional` **pluginName**: `string`

---

### stack

• `Optional` **stack**: `string`
Expand Down
10 changes: 7 additions & 3 deletions e2e/nx/src/import.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,13 @@ describe('Nx Import', () => {
execSync(`git commit -am "initial commit"`, {
cwd: repoPath,
});
execSync(`git checkout -b main`, {
cwd: repoPath,
});
try {
execSync(`git checkout -b main`, {
cwd: repoPath,
});
} catch {
// This fails if git is already configured to have `main` branch, but that's OK
}
mkdirSync(join(repoPath, 'packages/a'), { recursive: true });
writeFileSync(join(repoPath, 'packages/a/README.md'), `# A`);
execSync(`git add .`, {
Expand Down
30 changes: 29 additions & 1 deletion packages/nx/src/command-line/import/import.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ import { tmpdir } from 'tmp';
import { prompt } from 'enquirer';
import { output } from '../../utils/output';
import * as createSpinner from 'ora';
import { detectPlugins, installPlugins } from '../init/init-v2';
import {
detectPlugins,
installPlugins,
packageToPluginPath,
} from '../init/init-v2';
import { readNxJson } from '../../config/nx-json';
import { workspaceRoot } from '../../utils/workspace-root';
import {
Expand All @@ -29,6 +33,10 @@ import {
needsInstall,
} from './utils/needs-install';
import { readPackageJson } from '../../project-graph/file-utils';
import {
checkCompatibleWithPlugins,
updatePluginsInNxJson,
} from '../init/implementation/check-compatible-with-plugins';

const importRemoteName = '__tmp_nx_import__';

Expand Down Expand Up @@ -282,6 +290,17 @@ export async function importHandler(options: ImportOptions) {

// If install fails, we should continue since the errors could be resolved later.
let installFailed = false;
if (nxJson.plugins?.length > 0) {
// Check compatibility with existing plugins for the workspace included new imported projects
const incompatiblePlugins = await checkCompatibleWithPlugins(
nxJson.plugins,
workspaceRoot
);
if (Object.keys(incompatiblePlugins).length > 0) {
updatePluginsInNxJson(workspaceRoot, incompatiblePlugins);
await destinationGitClient.amendCommit();
}
}
if (plugins.length > 0) {
try {
output.log({ title: 'Installing Plugins' });
Expand All @@ -295,6 +314,15 @@ export async function importHandler(options: ImportOptions) {
bodyLines: [e.stack],
});
}
// Check compatibility with new plugins for the workspace included new imported projects
const incompatiblePlugins = await checkCompatibleWithPlugins(
plugins.map((plugin) => packageToPluginPath[plugin]), // plugins contains package name, but we need plugin path
workspaceRoot
);
if (Object.keys(incompatiblePlugins).length > 0) {
updatePluginsInNxJson(workspaceRoot, incompatiblePlugins);
await destinationGitClient.amendCommit();
}
} else if (await needsInstall(packageManager, originalPackageWorkspaces)) {
try {
output.log({
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
import {
AggregateCreateNodesError,
MergeNodesError,
ProjectConfigurationsError,
ProjectsWithNoNameError,
} from '../../../project-graph/error-types';
import { checkCompatibleWithPlugins } from './check-compatible-with-plugins';
import { retrieveProjectConfigurations } from '../../../project-graph/utils/retrieve-workspace-files';
import { tmpdir } from 'os';

jest.mock('../../../project-graph/plugins/internal-api', () => ({
loadNxPlugins: jest.fn().mockReturnValue([[], []]),
}));
jest.mock('../../../project-graph/utils/retrieve-workspace-files', () => ({
retrieveProjectConfigurations: jest.fn(),
}));

describe('checkCompatibleWithPlugins', () => {
beforeEach(() => {
jest.clearAllMocks();
});

it('should return empty object if no errors are thrown', async () => {
(retrieveProjectConfigurations as any).mockReturnValueOnce(
Promise.resolve({})
);
const result = await checkCompatibleWithPlugins(
['plugin1', 'plugin2'],
'projectRootPathToCheck',
tmpdir()
);
expect(result).toEqual({});
});

it('should return empty object if error is not ProjectConfigurationsError', async () => {
(retrieveProjectConfigurations as any).mockReturnValueOnce(
Promise.reject(new Error('random error'))
);
const result = await checkCompatibleWithPlugins(
['plugin1', 'plugin2'],
'projectRootPathToCheck',
tmpdir()
);
expect(result).toEqual({});
});

it('should return empty object if error is ProjectsWithNoNameError', async () => {
(retrieveProjectConfigurations as any).mockReturnValueOnce(
Promise.reject(
new ProjectConfigurationsError(
[
new ProjectsWithNoNameError([], {
project1: { root: 'root1' },
}),
],
undefined
)
)
);
const result = await checkCompatibleWithPlugins(
['plugin1', 'plugin2'],
'projectRootPathToCheck',
tmpdir()
);
expect(result).toEqual({});
});

it('should return incompatible plugin with excluded files if error is AggregateCreateNodesError', async () => {
(retrieveProjectConfigurations as any).mockReturnValueOnce(
Promise.reject(
new ProjectConfigurationsError(
[
new AggregateCreateNodesError(
[
['file1', undefined],
['file2', undefined],
],
[],
'plugin1'
),
],
undefined
)
)
);
const result = await checkCompatibleWithPlugins(
['plugin1', 'plugin2'],
'projectRootPathToCheck',
tmpdir()
);
expect(result).toEqual({ plugin1: new Set(['file1', 'file2']) });
});

it('should return true if error is MergeNodesError', async () => {
(retrieveProjectConfigurations as any).mockReturnValueOnce(
Promise.reject(
new ProjectConfigurationsError(
[
new MergeNodesError({
file: 'file2',
pluginName: 'plugin2',
error: new Error(),
}),
],
undefined
)
)
);
const result = await checkCompatibleWithPlugins(
['plugin1', 'plugin2'],
'projectRootPathToCheck',
tmpdir()
);
expect(result).toEqual({ plugin2: new Set(['file2']) });
});

it('should handle multiple errors', async () => {
(retrieveProjectConfigurations as any).mockReturnValueOnce(
Promise.reject(
new ProjectConfigurationsError(
[
new ProjectsWithNoNameError([], {
project1: { root: 'root1' },
}),
new AggregateCreateNodesError([], [], 'randomPlugin'),
new AggregateCreateNodesError(
[
['file1', undefined],
['file2', undefined],
],
[],
'plugin1'
),
new MergeNodesError({
file: 'file2',
pluginName: 'plugin2',
error: new Error(),
}),
new AggregateCreateNodesError(
[
['file3', undefined],
['file4', undefined],
],
[],
'plugin2'
),
],
undefined
)
)
);
const result = await checkCompatibleWithPlugins(
['plugin1', 'plugin2'],
'projectRootPathToCheck',
tmpdir()
);
expect(result).toEqual({
plugin1: new Set(['file1', 'file2']),
plugin2: new Set(['file2', 'file3', 'file4']),
});
});
});
Loading