Skip to content

Commit

Permalink
fix(core): ensure create nodes functions are properly parallelized (#…
Browse files Browse the repository at this point in the history
…23005)

(cherry picked from commit 0fd6d23)
  • Loading branch information
AgentEnder authored and FrozenPandaz committed Apr 25, 2024
1 parent 3771061 commit 76a62bd
Show file tree
Hide file tree
Showing 4 changed files with 159 additions and 37 deletions.
4 changes: 2 additions & 2 deletions docs/generated/devkit/CreateNodesContext.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@ Context for [CreateNodesFunction](../../devkit/documents/CreateNodesFunction)

### Properties

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

## Properties

### configFiles

`Readonly` **configFiles**: `string`[]
`Readonly` **configFiles**: readonly `string`[]

The subset of configuration files which match the createNodes pattern

Expand Down
2 changes: 1 addition & 1 deletion packages/nx/src/project-graph/plugins/public-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export interface CreateNodesContext {
/**
* The subset of configuration files which match the createNodes pattern
*/
readonly configFiles: string[];
readonly configFiles: readonly string[];
}

/**
Expand Down
123 changes: 123 additions & 0 deletions packages/nx/src/project-graph/plugins/utils.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
import { runCreateNodesInParallel } from './utils';

const configFiles = ['file1', 'file2'] as const;

const context = {
file: 'file1',
nxJsonConfiguration: {},
workspaceRoot: '',
configFiles,
} as const;

describe('createNodesInParallel', () => {
it('should return results with context', async () => {
const plugin = {
name: 'test',
createNodes: [
'*/**/*',
async (file: string) => {
return {
projects: {
[file]: {
root: file,
},
},
};
},
],
} as const;
const options = {};

const results = await runCreateNodesInParallel(
configFiles,
plugin,
options,
context
);

expect(results).toMatchInlineSnapshot(`
[
{
"file": "file1",
"pluginName": "test",
"projects": {
"file1": {
"root": "file1",
},
},
},
{
"file": "file2",
"pluginName": "test",
"projects": {
"file2": {
"root": "file2",
},
},
},
]
`);
});

it('should handle async errors', async () => {
const plugin = {
name: 'test',
createNodes: [
'*/**/*',
async () => {
throw new Error('Async Error');
},
],
} as const;
const options = {};

const error = await runCreateNodesInParallel(
configFiles,
plugin,
options,
context
).catch((e) => e);

expect(error).toMatchInlineSnapshot(
`[AggregateCreateNodesError: Failed to create nodes]`
);

expect(error.errors).toMatchInlineSnapshot(`
[
[CreateNodesError: The "test" plugin threw an error while creating nodes from file1:],
[CreateNodesError: The "test" plugin threw an error while creating nodes from file2:],
]
`);
});

it('should handle sync errors', async () => {
const plugin = {
name: 'test',
createNodes: [
'*/**/*',
() => {
throw new Error('Sync Error');
},
],
} as const;
const options = {};

const error = await runCreateNodesInParallel(
configFiles,
plugin,
options,
context
).catch((e) => e);

expect(error).toMatchInlineSnapshot(
`[AggregateCreateNodesError: Failed to create nodes]`
);

expect(error.errors).toMatchInlineSnapshot(`
[
[CreateNodesError: The "test" plugin threw an error while creating nodes from file1:],
[CreateNodesError: The "test" plugin threw an error while creating nodes from file2:],
]
`);
});
});
67 changes: 33 additions & 34 deletions packages/nx/src/project-graph/plugins/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@ import type {
LoadedNxPlugin,
NormalizedPlugin,
} from './internal-api';
import type { CreateNodesContext, NxPlugin, NxPluginV2 } from './public-api';
import {
CreateNodesResult,
type CreateNodesContext,
type NxPlugin,
type NxPluginV2,
} from './public-api';
import { AggregateCreateNodesError, CreateNodesError } from '../error-types';

export function isNxPluginV2(plugin: NxPlugin): plugin is NxPluginV2 {
Expand Down Expand Up @@ -49,7 +54,7 @@ export function normalizeNxPlugin(plugin: NxPlugin): NormalizedPlugin {
}

export async function runCreateNodesInParallel(
configFiles: string[],
configFiles: readonly string[],
plugin: NormalizedPlugin,
options: unknown,
context: CreateNodesContext
Expand All @@ -59,39 +64,33 @@ export async function runCreateNodesInParallel(
const errors: CreateNodesError[] = [];
const results: CreateNodesResultWithContext[] = [];

const promises: Array<Promise<void>> = configFiles.map((file) => {
const promises: Array<Promise<void>> = configFiles.map(async (file) => {
performance.mark(`${plugin.name}:createNodes:${file} - start`);
// Result is either static or a promise, using Promise.resolve lets us
// handle both cases with same logic
const value = Promise.resolve(
plugin.createNodes[1](file, options, context)
);
return value
.catch((e) => {
performance.mark(`${plugin.name}:createNodes:${file} - end`);
errors.push(
new CreateNodesError({
error: e,
pluginName: plugin.name,
file,
})
);
return null;
})
.then((r) => {
performance.mark(`${plugin.name}:createNodes:${file} - end`);
performance.measure(
`${plugin.name}:createNodes:${file}`,
`${plugin.name}:createNodes:${file} - start`,
`${plugin.name}:createNodes:${file} - end`
);

// Existing behavior is to ignore null results of
// createNodes function.
if (r) {
results.push({ ...r, file, pluginName: plugin.name });
}
});
try {
const value = await plugin.createNodes[1](file, options, context);
if (value) {
results.push({
...value,
file,
pluginName: plugin.name,
});
}
} catch (e) {
errors.push(
new CreateNodesError({
error: e,
pluginName: plugin.name,
file,
})
);
} finally {
performance.mark(`${plugin.name}:createNodes:${file} - end`);
performance.measure(
`${plugin.name}:createNodes:${file}`,
`${plugin.name}:createNodes:${file} - start`,
`${plugin.name}:createNodes:${file} - end`
);
}
});

await Promise.all(promises).then(() => {
Expand Down

0 comments on commit 76a62bd

Please sign in to comment.