Skip to content

Commit 0fd6d23

Browse files
AgentEnderAgentEnder
authored
fix(core): ensure create nodes functions are properly parallelized (#23005)
1 parent c6fe969 commit 0fd6d23

File tree

4 files changed

+159
-37
lines changed

4 files changed

+159
-37
lines changed

‎docs/generated/devkit/CreateNodesContext.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,15 @@ Context for [CreateNodesFunction](../../devkit/documents/CreateNodesFunction)
66

77
### Properties
88

9-
- [configFiles](../../devkit/documents/CreateNodesContext#configfiles): string[]
9+
- [configFiles](../../devkit/documents/CreateNodesContext#configfiles): readonly string[]
1010
- [nxJsonConfiguration](../../devkit/documents/CreateNodesContext#nxjsonconfiguration): NxJsonConfiguration<string[] | "\*">
1111
- [workspaceRoot](../../devkit/documents/CreateNodesContext#workspaceroot): string
1212

1313
## Properties
1414

1515
### configFiles
1616

17-
• `Readonly` **configFiles**: `string`[]
17+
• `Readonly` **configFiles**: readonly `string`[]
1818

1919
The subset of configuration files which match the createNodes pattern
2020

‎packages/nx/src/project-graph/plugins/public-api.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ export interface CreateNodesContext {
2222
/**
2323
* The subset of configuration files which match the createNodes pattern
2424
*/
25-
readonly configFiles: string[];
25+
readonly configFiles: readonly string[];
2626
}
2727

2828
/**

‎packages/nx/src/project-graph/plugins/utils.spec.ts

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
import { runCreateNodesInParallel } from './utils';
2+
3+
const configFiles = ['file1', 'file2'] as const;
4+
5+
const context = {
6+
file: 'file1',
7+
nxJsonConfiguration: {},
8+
workspaceRoot: '',
9+
configFiles,
10+
} as const;
11+
12+
describe('createNodesInParallel', () => {
13+
it('should return results with context', async () => {
14+
const plugin = {
15+
name: 'test',
16+
createNodes: [
17+
'*/**/*',
18+
async (file: string) => {
19+
return {
20+
projects: {
21+
[file]: {
22+
root: file,
23+
},
24+
},
25+
};
26+
},
27+
],
28+
} as const;
29+
const options = {};
30+
31+
const results = await runCreateNodesInParallel(
32+
configFiles,
33+
plugin,
34+
options,
35+
context
36+
);
37+
38+
expect(results).toMatchInlineSnapshot(`
39+
[
40+
{
41+
"file": "file1",
42+
"pluginName": "test",
43+
"projects": {
44+
"file1": {
45+
"root": "file1",
46+
},
47+
},
48+
},
49+
{
50+
"file": "file2",
51+
"pluginName": "test",
52+
"projects": {
53+
"file2": {
54+
"root": "file2",
55+
},
56+
},
57+
},
58+
]
59+
`);
60+
});
61+
62+
it('should handle async errors', async () => {
63+
const plugin = {
64+
name: 'test',
65+
createNodes: [
66+
'*/**/*',
67+
async () => {
68+
throw new Error('Async Error');
69+
},
70+
],
71+
} as const;
72+
const options = {};
73+
74+
const error = await runCreateNodesInParallel(
75+
configFiles,
76+
plugin,
77+
options,
78+
context
79+
).catch((e) => e);
80+
81+
expect(error).toMatchInlineSnapshot(
82+
`[AggregateCreateNodesError: Failed to create nodes]`
83+
);
84+
85+
expect(error.errors).toMatchInlineSnapshot(`
86+
[
87+
[CreateNodesError: The "test" plugin threw an error while creating nodes from file1:],
88+
[CreateNodesError: The "test" plugin threw an error while creating nodes from file2:],
89+
]
90+
`);
91+
});
92+
93+
it('should handle sync errors', async () => {
94+
const plugin = {
95+
name: 'test',
96+
createNodes: [
97+
'*/**/*',
98+
() => {
99+
throw new Error('Sync Error');
100+
},
101+
],
102+
} as const;
103+
const options = {};
104+
105+
const error = await runCreateNodesInParallel(
106+
configFiles,
107+
plugin,
108+
options,
109+
context
110+
).catch((e) => e);
111+
112+
expect(error).toMatchInlineSnapshot(
113+
`[AggregateCreateNodesError: Failed to create nodes]`
114+
);
115+
116+
expect(error.errors).toMatchInlineSnapshot(`
117+
[
118+
[CreateNodesError: The "test" plugin threw an error while creating nodes from file1:],
119+
[CreateNodesError: The "test" plugin threw an error while creating nodes from file2:],
120+
]
121+
`);
122+
});
123+
});

‎packages/nx/src/project-graph/plugins/utils.ts

Lines changed: 33 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,12 @@ import type {
99
LoadedNxPlugin,
1010
NormalizedPlugin,
1111
} from './internal-api';
12-
import type { CreateNodesContext, NxPlugin, NxPluginV2 } from './public-api';
12+
import {
13+
CreateNodesResult,
14+
type CreateNodesContext,
15+
type NxPlugin,
16+
type NxPluginV2,
17+
} from './public-api';
1318
import { AggregateCreateNodesError, CreateNodesError } from '../error-types';
1419

1520
export function isNxPluginV2(plugin: NxPlugin): plugin is NxPluginV2 {
@@ -49,7 +54,7 @@ export function normalizeNxPlugin(plugin: NxPlugin): NormalizedPlugin {
4954
}
5055

5156
export async function runCreateNodesInParallel(
52-
configFiles: string[],
57+
configFiles: readonly string[],
5358
plugin: NormalizedPlugin,
5459
options: unknown,
5560
context: CreateNodesContext
@@ -59,39 +64,33 @@ export async function runCreateNodesInParallel(
5964
const errors: CreateNodesError[] = [];
6065
const results: CreateNodesResultWithContext[] = [];
6166

62-
const promises: Array<Promise<void>> = configFiles.map((file) => {
67+
const promises: Array<Promise<void>> = configFiles.map(async (file) => {
6368
performance.mark(`${plugin.name}:createNodes:${file} - start`);
64-
// Result is either static or a promise, using Promise.resolve lets us
65-
// handle both cases with same logic
66-
const value = Promise.resolve(
67-
plugin.createNodes[1](file, options, context)
68-
);
69-
return value
70-
.catch((e) => {
71-
performance.mark(`${plugin.name}:createNodes:${file} - end`);
72-
errors.push(
73-
new CreateNodesError({
74-
error: e,
75-
pluginName: plugin.name,
76-
file,
77-
})
78-
);
79-
return null;
80-
})
81-
.then((r) => {
82-
performance.mark(`${plugin.name}:createNodes:${file} - end`);
83-
performance.measure(
84-
`${plugin.name}:createNodes:${file}`,
85-
`${plugin.name}:createNodes:${file} - start`,
86-
`${plugin.name}:createNodes:${file} - end`
87-
);
88-
89-
// Existing behavior is to ignore null results of
90-
// createNodes function.
91-
if (r) {
92-
results.push({ ...r, file, pluginName: plugin.name });
93-
}
94-
});
69+
try {
70+
const value = await plugin.createNodes[1](file, options, context);
71+
if (value) {
72+
results.push({
73+
...value,
74+
file,
75+
pluginName: plugin.name,
76+
});
77+
}
78+
} catch (e) {
79+
errors.push(
80+
new CreateNodesError({
81+
error: e,
82+
pluginName: plugin.name,
83+
file,
84+
})
85+
);
86+
} finally {
87+
performance.mark(`${plugin.name}:createNodes:${file} - end`);
88+
performance.measure(
89+
`${plugin.name}:createNodes:${file}`,
90+
`${plugin.name}:createNodes:${file} - start`,
91+
`${plugin.name}:createNodes:${file} - end`
92+
);
93+
}
9594
});
9695

9796
await Promise.all(promises).then(() => {

0 commit comments

Comments
 (0)