fix(core): preserve input order in createNodes plugin results#35595
Open
AgentEnder wants to merge 6 commits intomasterfrom
Open
fix(core): preserve input order in createNodes plugin results#35595AgentEnder wants to merge 6 commits intomasterfrom
AgentEnder wants to merge 6 commits intomasterfrom
Conversation
✅ Deploy Preview for nx-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for nx-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
View your CI Pipeline Execution ↗ for commit 6b0fef0
☁️ Nx Cloud last updated this comment at |
`createNodesFromFiles` (used by ~20 plugins via `@nx/devkit`) ran each file's callback inside `Promise.all(configFiles.map(...))` and pushed the result tuple from inside the async callback. The push order was therefore the resolution order of the callbacks, not the input order of `configFiles`. When two matched files contribute to the same project root (e.g. sibling `package.json` + `tsconfig.json` configs handled by the same plugin), `mergeCreateNodesResultsFromSinglePlugin` merges them in the order they appear in the plugin result array. Race-dependent push order meant later/faster callbacks could win merges that the sorted file list said should lose, producing different project graphs across runs. Settle each input into a discriminated tuple and bin into results/errors in a synchronous post-pass so both arrays follow input order. Add regression tests where later inputs resolve faster.
`internalCreateNodesV2` mutated `projects[projectRoot] = project` from inside `Promise.all(...)`, so the `projects` object's key insertion order tracked which async branch (`eslint.isPathIgnored` or `getProjectUsingESLintConfig`) finished first. Downstream merge order in `mergeCreateNodesResultsFromSinglePlugin` walks `for (const root in projectNodes)` in insertion order, so a race between sibling project roots could swap which root's contribution won when fields overlapped. Have each parallel branch return its contribution and assemble `projects` in a synchronous post-pass over the original `projectRootsByEslintRoots.get(configDir)` array.
The jest plugin's runtime branch (`disableJestRuntime: false`) discovers test files via `jest.SearchSource.getTestPaths()`, which walks the filesystem through jest-haste-map's parallel workers and does not guarantee a sorted output. The result was wrapped in `new Set(...)`, which preserves insertion order, then iterated to build atomized `<ciTargetName>--<relativePath>` targets. Insertion order leaks into `targets`, `dependsOn`, and `targetGroups[group]`, all observable via `nx graph` and CI workflow generators. Across runs the same workspace could emit the same set of atomized targets in different orders. Sort the discovered paths before constructing the Set so the target-name insertion order is stable. The other branch (`disableJestRuntime: true`) already sources from `globWithWorkspaceContext`, which is sorted by the Rust glob implementation.
`getTestPathsRelativeToProjectRoot` returns the result of `vitest.getRelevantTestSpecifications()` (which walks the filesystem via tinyglobby, unordered) directly to the atomized target loop. The atomizer then iterates these paths to insert `<ciTargetName>--<relativePath>` keys into the project's `targets` record, push to `dependsOn`, and push to `targetGroups[group]`. Insertion order leaks through all three to the project graph, so two runs over the same workspace could produce identically-shaped but differently-ordered atomization output. Sort relative paths before returning so atomized target insertion is stable across runs.
Mirror of the vitest fix. The `@nx/vite` plugin carries a duplicate `getTestPathsRelativeToProjectRoot` helper that returns the unsorted output of `vitest.getRelevantTestSpecifications()` (driven by tinyglobby) straight into the atomized target loop. Sort relative paths before returning so atomized target insertion order is stable across runs.
10ace74 to
6b0fef0
Compare
Contributor
There was a problem hiding this comment.
Nx Cloud has identified a flaky task in your failed CI:
🔂 Since the failure was identified as flaky, we triggered a CI rerun by adding an empty commit to this branch.
🔔 Heads up, your workspace has pending recommendations ↗ to auto-apply fixes for similar failures.
🎓 Learn more about Self-Healing CI on nx.dev
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Current Behavior
Two structural sources of non-determinism existed inside Nx's
createNodes/createNodesV2pipeline. Both are invisible to plugin authors but produce different project graphs across runs on the same workspace.1.
createNodesFromFilesresolution-order racecreateNodesFromFiles(inpackages/nx/src/project-graph/plugins/utils.ts) — the helper that ~20 first-party plugins use to fan out per-config-file work — runs callbacks in parallel viaPromise.all(configFiles.map(async (file, idx) => { ... results.push([file, value]) })). Tuples are pushed into the sharedresultsarray in the resolution order of the callbacks, not the input order ofconfigFiles.The matched file list arriving at the plugin is sorted (the Rust
glob_filesimplpar_sorts, and Rayon'spar_iter().filter().collect()preserves order). The downstream merge (mergeCreateNodesResultsFromSinglePlugin→for (const result of pluginResults)→for (const root in projectNodes)) walks results in array / insertion order. So if any plugin returns multiple contributions for the same project root, the only place the order can scramble is the helper'sPromise.all + .pushrace.A second instance of the same pattern lives in
@nx/eslint'sinternalCreateNodesV2, which mutatesprojects[projectRoot] = projectfrom inside aPromise.all. Theprojectsobject's key-insertion order then trackseslint.isPathIgnored/getProjectUsingESLintConfigresolution races and propagates the same non-determinism.2. Atomized target name insertion order
For atomizing plugins, the order of dynamically-generated target names (
<ciTargetName>--<relativePath>) leaks into the project graph throughtargets[name]insertion order,dependsOn[], andtargetGroups[group][]. The order is deterministic when the file list comes from Nx's Rust glob (sorted), but not when it comes from a non-Nx file-discovery layer:@nx/jest(runtime branch,disableJestRuntime: false) — usesjest.SearchSource.getTestPaths()which walks via jest-haste-map's parallel workers; ordering not guaranteed. ThedisableJestRuntime: truebranch was already fine (sorted glob).@nx/vitestand@nx/vite— both have agetTestPathsRelativeToProjectRoothelper that returnsvitest.getRelevantTestSpecifications()directly. Vitest uses tinyglobby internally, which doesn't sort.@nx/cypress,@nx/playwright,@nx/gradle(v1 + v2), and@nx/eslint's atomizer paths all source from sorted Rust glob and iterate synchronously — they're fine.Expected Behavior
Project graph construction is deterministic across runs given a deterministic input file list. Specifically:
createNodesFromFilesreturnsresultsanderrorsarrays inconfigFilesinput order, regardless of which callback resolves first.@nx/eslint'sprojectsmap keys are inserted in input order ofprojectRootsByEslintRoots.get(configDir).@nx/jest,@nx/vitest, and@nx/viteare inserted in lexicographic order of relative path.How
packages/nx/src/project-graph/plugins/utils.ts— settle each callback into a discriminated tuple{ kind: 'value' | 'empty' | 'error', ... }from insidePromise.all.await Promise.all(arr.map(...))returns an array indexed by input position, so a synchronous post-pass over that array bins values and errors in input order. No change to public API or error semantics.packages/eslint/src/plugins/plugin.ts— each parallel branch returns its contribution (ornull) instead of mutating the sharedprojectsobject. A synchronous post-pass overorderedProjectRootsObject.assigns contributions intoprojectsin input order.packages/jest/src/plugins/plugin.ts— sortspecs.tests.map(({ path }) => path)before constructing theSetof test paths.packages/vitest/src/plugins/plugin.ts+packages/vite/src/plugins/plugin.ts—.sort()the relative paths returned bygetTestPathsRelativeToProjectRootbefore they reach the atomizer loop.Audit of other createNodes implementations
@nx/jest(disableJestRuntime: true) — sources fromglobWithWorkspaceContext(sorted by Rust glob).@nx/cypress,@nx/playwright— sources fromglobWithWorkspaceContext/getFilesInDirectoryUsingContext(both deterministic;get_child_filesis a sequentialinto_iter().filter().collect()over a sorted file list) and iterates withfor (const ... of ...).@nx/gradlev1 —splitConfigFiles+forEachover already-sorted glob output.@nx/gradlev2 — synchronousfor...ofoverArray.from(new Set([...]));Setiterates in insertion order, source arrays deterministic.@nx/maven,@nx/nuxt,@nx/remix,@nx/rollup,@nx/detox,@nx/dotnet,@nx/react/router-plugin, and the nx-coreproject-json/package-json/jsplugins — no parallel-write-to-shared-object patterns.Tests
Two new regression tests in
packages/nx/src/project-graph/plugins/utils.spec.tsforce later inputs to resolve faster (e.g.file1waits 30ms,file2resolves immediately) and assert thatresultsanderrorsboth follow input order. Existing snapshot tests continue to pass — they had been passing only coincidentally because trivial sync paths happened to push in input order; now the guarantee is structural.For the atomizer sort fixes, existing snapshot tests pass (jest 54/54, vitest 7/7, vite 22/22, cypress, playwright, eslint). The fixes are pure ordering — no observable change when test discovery happens to already be sorted.
Related Issue(s)