fix(core): single-inference convert-to-inferred engine with centralized config - #36547
Open
meeroslav wants to merge 26 commits into
Open
fix(core): single-inference convert-to-inferred engine with centralized config#36547meeroslav wants to merge 26 commits into
meeroslav wants to merge 26 commits into
Conversation
👷 Deploy Preview for nx-docs processing.
|
👷 Deploy Preview for nx-dev processing.
|
Contributor
|
View your CI Pipeline Execution ↗ for commit 064e3db
☁️ Nx Cloud last updated this comment at |
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
meeroslav
changed the title
…count Add a synthetic-plugin test harness + a counter test documenting the status quo: the engine runs ~(targets + 2*projects) whole-workspace inference passes (7 for a 3-project fixture). Task 2 flips this to a constant (distinctOptionSets + 1). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ange) Fold forEachExecutorOptions over every migration/executor into a single collectMigrationScope pass — the sole authority for skip filtering (same warn-vs-throw semantics, specific-project skip still throws). It returns targetsToMigrate, per-project registration options, distinct inference option sets, per-executor slices, and skipped projects. The migrator now receives its pre-filtered target/project slice instead of re-deriving + re-warning; migrateProjects builds the returned map from the collected scope. Emitted config is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…one-shot registration) Replace the quadratic inference in the shared convert-to-inferred engine with O(1)-in-project-count whole-workspace inference: - Phase 1 `inferOncePerOptionSet`: run the plugin's createNodes once per distinct option-set (usually one), building `inferredByRoot` (root -> target -> stripped inferred target). The migrator now reads residuals from this shared map instead of re-inferring per target/project. - Replace the per-project registration inference + per-project `arePluginIncludesRequired` with one-shot analytic include computation: a registration can be left unscoped iff its include globs cover every config file owned by an inferred project root (judged against inferred roots, not the raw glob which also matches package.json/project.json). - Add a single verification inference pass over the whole workspace with the updated nx.json (equivalence oracle + fallback land in a later task). - Recover the plugin-filled default option keys that the old per-project inference incidentally added by mutating options in place, so emitted registration options stay byte-identical (no extra inference). Whole-workspace inference passes drop from ~(targets + 2*projects) to distinctOptionSets + 1. Emitted config is unchanged; the counter test now asserts a constant pass count independent of project count. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Split the per-project residual computation out of the (now removed)
ExecutorToPluginMigrator class into a standalone Phase 2:
- computeResidualByProject builds
Map<project, Map<target, { residual, baselineFinal }>>. `residual` is
byte-for-byte the target the previous engine writes to project.json;
`baselineFinal = mergeTargetConfigurations(residual, fullInferred)` is
the migrated command-based effective config used as the Phase 4
equivalence oracle.
- inferredByRoot now stores the FULL inferred target; residual computation
strips command/options.cwd at the point of use.
- writeResiduals (Phase 3, residual-only variant) writes the full residual
per project.json, reproducing the previous write sequence.
Emitted config is unchanged across every plugin convert-to-inferred spec.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Centralize configuration shared across migrated projects instead of duplicating it per project.json: - computeStrictCommon derives the values deep-equal across ALL migrated projects for a target (whole-value for top-level props; per-key for options). Hoist only when >=2 projects share the target — a lone migrated project keeps its full residual (no de-bloat benefit and avoids leaking its config onto sibling inferred targets). - Write targetDefaults[targetName] = merge(common, existing default); write only per-project deviations (residual minus common) to project.json, deleting the target when the deviation is empty. - Remove the now-dead executor-keyed target default that Phase 2 inlined into every project, but only once no target still uses that executor. - Single-project (--project) mode never hoists (unchanged behavior). Tests A (uniform hoist), B (executor-default de-dup + dead-entry removal), C (single-project no hoist), and a computeStrictCommon unit test. Whole-workspace plugin specs now legitimately diff (config in nx.json targetDefaults, project.json deviations only) — updated in a later task; equivalence is formally verified in the next task. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Verify every centralized migration is behaviorally equivalent to what the previous engine produced, and conservatively fall back where it is not: - The single verification inference pass already merges targetDefaults into the inferred targets, so the real post-migration effective config is merge(project.json deviation, verified inferred+targetDefaults). It must deep-equal baselineFinal (the previous engine's migrated config, captured in Phase 2). - Any project that fails the oracle — or whose intended target no longer infers at all (leak/coverage) — is reverted to a full project.json override (dropping only its hoist; project.json wins over the shared targetDefaults). All fallbacks are summarized in one logger.warn. - Single-project mode skips the oracle (it never hoists). Across all 15 plugin convert-to-inferred suites the oracle triggers zero fallbacks, confirming the hoisting is verified-equivalent. Tests: forced fallback (one project diverges on the verify pass -> keeps its override, others centralized, warned once) and partial-migration include correctness (root project -> '*'). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…hape Update the plugin convert-to-inferred specs whose emitted config legitimately moved to the new centralized shape (shared config in nx.json targetDefaults, project.json carries deviations only): - vite / storybook / remix: update the --all/all-projects inline snapshots; document where the shared config went (targetDefaults assertions in vite + remix). - eslint / jest / webpack: add explicit effective-equivalence tests — two projects share a non-inferred option, so it is centralized exactly once into targetDefaults and the merged (effective) config still resolves it per project, with no per-project duplication. Guards against a shape update hiding a behavioral regression. No generator changes. webpack/rollup/rspack/next keep their pre-existing prettier/formatting failures (env-specific, empty diff vs master) — not touched here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: meeroslav <meeroslav@users.noreply.github.com>
Add a synthetic large-workspace benchmark covering 600 projects with lint and jest-style executor targets. It pins the single-digit whole-workspace inference count and verifies that centralizing shared residual config does not increase total emitted config bytes. The benchmark exposed two non-inference O(projects^2+) paths in the rewritten engine, so keep the implementation scalable by: - caching project configurations once for residual computation, writes, and the equivalence verification fallback loop instead of rebuilding the full project map per target; - grouping plugin registrations by identical options before computing include coverage, so include coverage is checked once per registration group rather than after every project append. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
polygraph-snapshot-app
Bot
force-pushed
the
feat/convert-to-inferred-single-inference
branch
from
August 5, 2026 08:34
f7f9394 to
47baa8a
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Co-authored-by: meeroslav <meeroslav@users.noreply.github.com>
… plugin
Hoist the strict-common residual as a targetDefaults array entry with
filter: { plugin }, appended after any existing value, instead of merging
into an unfiltered target-name key. An unfiltered key applies to every
same-named target in the workspace, so the previous shape leaked migrated
config onto non-migrated executor targets, projects skipped by
skip filters, and other plugins' targets in sequential migrations — and
could overwrite a user's conflicting default value. Filtered entries
never match targets without a source plugin, existing values are never
modified, and array-shaped existing defaults now centralize normally
instead of mass-falling-back.
The verification pass additionally reverts a target's hoist (restoring
full residuals + one warn) when a non-migrated root still infers that
target, e.g. under a pre-existing unscoped registration — the oracle now
covers the hoist's whole blast radius, not only migrated pairs.
Dead executor-keyed entries are removed via updateTargetDefault instead
of a hand-rolled normalize/collapse.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The fallback warn claimed a project.json override was kept and behavior preserved — false when the residual is empty (the target is removed) and unverifiable when the inferred configuration itself diverged. Reword to state what actually happened (pre-centralization output restored, centralized defaults shadowed where keys overlap) and ask for manual review instead of asserting equivalence the pass could not establish. Verification-pass errors are no longer discarded: when a fallback is due to a target missing from the (partial) verification result, the pass's error messages are appended to the warn as the probable cause. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…lude coverage - includeCoversAllConfigFiles compiled every glob on every minimatch() call; both the include list and the config-file list scale with project count (~P^2/2 compilations on large workspaces). Compile each glob once. - inferOncePerOptionSet re-materialized the inferred-roots set inside the filter predicate — one O(P) allocation per matched config file. Hoist it. - isExecutorStillUsed ran a whole-workspace forEachExecutorOptions scan per migrated executor with no early exit; the cached project map already carries the post-write state, so scan that instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…t typing
Comment fixes: writeResiduals and runVerificationPass carried
series-development scaffolding ('until centralization lands', 'added in
a later task') describing a state that never ships; inferOncePerOptionSet
JSDoc said stripped/raw-glob where the code stores full targets and the
filtered file subset; the inference-pass counter docs named
getCreateNodesResultsForPlugin, which Phase 4 does not go through — the
counter counts whole-workspace passes (one per registration group on
verify); include-coverage and plugin-filled-defaults JSDoc dropped their
false 'exactly'/'never' absolutes.
Typing: distinctOptionSets and InferenceOptionSet.options are raw mapper
output that never receives defaults — type them Partial<T> and drop the
casts; collectMigrationScope's trailing params become optional.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
…xecutor/command
A hoisted `filter: { plugin }` targetDefault is rejected by Nx's
resolveSourcePlugin whenever the project.json (default) layer's merged
target carries an `executor` or `command` — the plugin no longer owns the
target's identity. So centralizing a target whose residual carries one of
those keys silently drops the hoisted keys, and the verification pass
cannot see it (that pass has no project.json layer). @nx/detox stamps a
per-project `command`, so its shared `options.args` / `configurations`
were dropped once two projects made the hoist trigger.
Refuse to hoist a target whose residual carries executor/command; keep the
full residual per project instead. Adds a synthetic engine regression and a
two-detox-project fixture (the single-project shipped fixture structurally
cannot reach the hoist). Corrects the coupled hoist-site and
verifyAndFallback comments.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… broken config The hoist-revert guard keyed on `reachesNonMigratedRoot`, which iterates the verification result's projects — a root the plugin ERRORED on is absent from that map, so an inferred-only root that failed to infer during verification never triggered the revert. The plugin-scoped default then survived and would be inherited by that root once its config was fixed, silently shipping one project's option onto an unrelated project. runVerificationPass now surfaces the offending config files from AggregateCreateNodesError, and the guard reverts whenever an errored config file sits outside the target's migrated roots. It also filters ProjectsWithNoNameError / MultipleProjectsWithSameNameError (artifacts of the no-project.json verification world) out of the reported errors, so a non-empty error list means inference genuinely broke; those errors are surfaced in the revert warning and in a new standalone warning when an error-degraded verification produced no revert and no fallback. Adds a fail-closed regression fixture (control arm L already covered the clean-inference case). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Remove the write-only `skipped` set (no readers, and it under-recorded
target-filter skips) and the spec-only `targetsToMigrate` /
`distinctOptionSets` fields from the exported `MigrationScope`; the spec
derives the same facts from `optionSetGroups` / `executorScopes`.
- collectMigrationScope now calls each `targetPluginOptionMapper(target)`
once and reuses it for both the option-set id and the registration
options, so an impure mapper can no longer diverge them.
- Correct the MigrationScope ("single pass" -> per migration/executor) and
inferOncePerOptionSet ("every later phase" -> Phase 2 / Phase 3 registration)
comments, and disambiguate InferenceOptionSet.options.
- getFullInferredTarget now distinguishes "no project inside root" from
"project found but target not inferred" instead of folding both into a
file-an-issue message about project structure.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…oject count Both the matched-config-file filter (config files owned by an inferred root) and include-coverage scaled O(files * roots), and coverage additionally ran minimatch per (file, glob). On the path this PR exists to scale, that was a clean quadratic: measured include coverage 43.5ms -> 1484ms and the ownership filter 11.2ms -> 551ms across 600 -> 4000 projects. File-to-root ownership is now an O(path depth) ancestor walk against the root Set. Every include glob this generator emits scopes to a single root (`*` or a nested-root globstar), so coverage reduces to root ownership with no glob engine; only a user-authored include or any `exclude` falls through to minimatch. Measured: coverage 43.5ms -> 0.3ms and filter 11.2ms -> 0.3ms at 600, 1484ms -> 1.1ms and 551ms -> 1.0ms at 4000, with identical results. All convert-to-inferred suites unchanged, including the root-project `*` include test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… resolution The plugin `(equivalence)` tests merged the hoisted default and the project deviation with a JS spread — which always reports the hoisted value present, because both objects come from JSON.parse (no `undefined`-valued own keys) and the spread never applies Nx's resolveSourcePlugin gate. Run against a command-carrying residual they would report a value that real resolution drops. Add `resolveThroughRealPipeline` to the engine test harness: it flushes the migrated Tree to disk, refreshes the workspace context, and runs retrieveProjectConfigurations with the migrated plugin registrations plus the real `project.json` default plugin — so targetDefaults synthesis, including the `filter.plugin` gate, runs exactly as at runtime. The command-residual regression and the uniform-hoist test now assert the effective resolved target through this path (the former resolves to `undefined` without the BLOCKING-1 fix). The plugin specs keep their de-bloat shape checks and drop the spread. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The benchmark asserted only `postBytes <= preBytes`, which passes even if nothing is centralized. Replace it with a compression-ratio bound: on the 600-project fixture the emitted config drops from 361459 to 126905 bytes (2.85x smaller), so assert the ratio exceeds 2x to actually pin the de-bloat claim. The pass-count and hoisted-entry-shape assertions are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The convert-to-inferred knowledge-base page said a target is removed from project.json only when "options and configurations are not customized". That is no longer the removal condition for whole-workspace migrations: shared customized options are lifted into a plugin-scoped `targetDefaults` entry in nx.json, so the target can be removed even when it was customized. Correct the removal condition, add the new targetDefaults write to the list of what the generator does, and add a section describing the plugin-scoped `filter` entry, that project.json keeps only per-project deviations, and that single-project (`--project`) migrations skip centralization to avoid leaking one project's config onto its siblings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The real-resolution test helper imports the project.json default plugin and the workspace-context reset — both restricted imports, allowed here because the file is test-only and excluded from the published lib build. Disable the restricted-imports rule for those two lines (matching the existing createTreeWithEmptyWorkspace import) and drop an unused no-console directive in the benchmark. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Status
Implementation stages are complete for Tasks 0-8, and the follow-up review fixes are now included. The latest pushed head is
9cf249e fix(devkit): isolate inferred migrator option sets.Summary
Rewrites the shared
convert-to-inferredmigration engine inpackages/devkit/src/generators/plugin-migrations/executor-to-plugin-migrator.ts. This engine is used by every@nx/*:convert-to-inferredgenerator and by@nx/workspace:infer-targets.The rewrite keeps migration output behaviorally equivalent while addressing two large-workspace problems:
nx.json.targetDefaults[targetName], leavingproject.jsonwith only per-project deviations.What Changed
inferred -> targetDefaults -> project.jsonmerge.targetDefaults[targetName]array entries withfilter: { plugin }, preserving existing defaults instead of merging over them.migrateProjectExecutorsToPlugin/migrateProjectExecutorsToPluginV1signatures and existing plugin generator APIs.targetDefaultsshape, including eslint, jest, remix, storybook, vite, webpack, and rspack coverage.origin/masterand fixed the webpack/rspack spec expectations from the PR failure rerun.newProject()now disable immutable installs and run a Yarn install before caching/resetting, avoiding the@proj/source@workspace:.lockfile error in the Remix Yarn suite.Verification
Fresh local verification on the updated branch:
pnpm exec jest packages/devkit/src/generators/plugin-migrations/executor-to-plugin-migrator.spec.ts --config packages/devkit/jest.config.cts --runInBand: 31/31 passed.pnpm exec tsc -b packages/devkit/tsconfig.lib.json --pretty false: passed.pnpm exec tsc -p packages/devkit/tsconfig.spec.json --pretty false --noEmit: passed.npx prettier --check packages/devkit/src/generators/plugin-migrations/executor-to-plugin-migrator.ts packages/devkit/src/generators/plugin-migrations/executor-to-plugin-migrator.spec.ts packages/devkit/src/generators/plugin-migrations/executor-to-plugin-migrator.test-utils.ts: passed.pnpm exec jest packages/devkit/src/generators/plugin-migrations/executor-to-plugin-migrator.bench.spec.ts --config packages/devkit/jest.config.cts --runInBand: 1/1 passed.pnpm nx lint devkit: passed.git diff --check -- packages/devkit/src/generators/plugin-migrations/executor-to-plugin-migrator.ts packages/devkit/src/generators/plugin-migrations/executor-to-plugin-migrator.spec.ts packages/devkit/src/generators/plugin-migrations/executor-to-plugin-migrator.test-utils.ts: passed.Earlier branch verification also covered webpack, rspack, eslint, jest, remix, storybook, vite, cypress, playwright, detox, expo, react-native, and vitest converter suites. The known webpack/rspack/next/devkit aggregate-log environment-specific snapshot failures matched master and were not touched by this PR.
Benchmarks
Test description:
@nx/eslintconverter, 527 projects are lintable/use@nx/eslint:lintexecutor.