feat(devkit): migrate @nx/devkit/src/... deep imports#35541
feat(devkit): migrate @nx/devkit/src/... deep imports#35541FrozenPandaz merged 5 commits intomasterfrom
@nx/devkit/src/... deep imports#35541Conversation
…ernal entry points Adds an `nx migrate` step that rewrites every `@nx/devkit/src/...` import in a workspace's `.ts`/`.tsx`/`.cts`/`.mts` files to a supported entry point, since the new `exports` map closes off the deep-import paths. The rewrite uses the TypeScript compiler API (lazy-loaded via `ensurePackage`) to bucket each named specifier by symbol name: - Public-API names land on `@nx/devkit`. - Names that were previously only deep-importable land on `@nx/devkit/internal`. Default, namespace, side-effect, `require(...)`, and dynamic `import(...)` forms can't be split by symbol; they get rewritten to `@nx/devkit/internal`, which re-exports every previously deep-importable symbol. A final collapse pass merges any duplicate `@nx/devkit` and `@nx/devkit/internal` named-only imports (including pre-existing ones the user already had) into a single declaration per `(specifier, isTypeOnly)` group, deduping specifiers by rendered text.
✅ 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. |
|
View your CI Pipeline Execution ↗ for commit 3b90afb
☁️ 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.
The migration imported from '@nx/devkit' which is self-referential inside the devkit package — @nx/dependency-checks flagged it as a missing dependency, and the eslint config bans non-relative @nx/devkit imports inside the package. Rewrite to use the same relative paths that public-api.ts uses, plus nx/src/devkit-exports for Tree/logger. Also add the missing Promise<void> return type on the migration default export.
AgentEnder
left a comment
AgentEnder
left a comment
There was a problem hiding this comment.
Seems mostly fine, left a few comments but nothing blocking
| // calls, etc.) get rewritten to `/internal`. We can't bucket by symbol for | ||
| // those forms, so `/internal` is the safe default since it re-exports every | ||
| // symbol that was deep-importable. |
There was a problem hiding this comment.
Is it true that internal is every symbol that was previously available?
There was a problem hiding this comment.
Good catch — it's not. internal.ts covers every symbol that was previously deep-importable AND that we wanted to keep around as semi-private, but a deep-importable name that ended up in the stable public API (or that we removed entirely) wouldn't be there. Pointing default / namespace / require() / dynamic-import() shapes at /internal could silently land them on a broken path.
Dropped the fallback in df70f84. Now only plain named imports are rewritten (those are the ones we can bucket per-symbol via DEVKIT_INTERNAL_SYMBOLS); everything else is left untouched, and the migration's user-facing doc calls them out as a manual step.
There was a problem hiding this comment.
Walked this back in 4072ea2. You're right that /internal isn't literally every symbol — but dropping the fallback meant any non-named deep import (default / namespace / side-effect / require() / dynamic import()) left a @nx/devkit/src/... specifier the new exports map rejects, so workspaces with any of those shapes wouldn't compile post-migration.
Restored the fallback as a best-guess rewrite to /internal, since that's where the bulk of previously deep-importable symbols actually live. If a user's symbol turns out to be public API instead, they get a clear "no exported member" error pointing at the new entry point — which is a much better signal than an unresolvable @nx/devkit/src/... path. Tightened the migration doc to call it a best guess rather than a guarantee, with instructions for the manual fix.
| // at the time this migration was authored). Anything imported from a | ||
| // `@nx/devkit/src/...` path whose name is NOT in this set is assumed to be | ||
| // part of the stable public `@nx/devkit` API. | ||
| export const DEVKIT_INTERNAL_SYMBOLS: ReadonlySet<string> = new Set([ |
There was a problem hiding this comment.
Do you think it may be better to read these imports w/ the ts APIs since we are already bringing them in? I'm back-and-forth on it, as it may be easier to just hardcode like this and it shouldn't shift too much.
There was a problem hiding this comment.
Yeah I think hard coding them is fine
`/internal` is not guaranteed to re-export every symbol that was previously reachable through `@nx/devkit/src/...`, so blindly rewriting non-named-import shapes (default / namespace / side-effect / require() / dynamic import()) to `/internal` could silently send users to a broken path. Only plain named imports — which we can bucket per-symbol against the explicit DEVKIT_INTERNAL_SYMBOLS set — are rewritten now; everything else is left untouched and called out as a manual-migration step in the migration's user-facing doc.
Reverts df70f84. Without the fallback, any non-named deep import (default / namespace / side-effect / require() / dynamic import()) left a `@nx/devkit/src/...` specifier in the file that the new exports map flat-out rejects, so workspaces with any of those shapes wouldn't compile post-migration. Restoring the fallback rewrites those shapes to `@nx/devkit/internal` — which is where the bulk of previously deep-importable symbols actually live. If a user's symbol isn't in /internal (e.g. it's part of the public API), they get a clear "no exported member" error pointing at the new entry point, which is a much better signal than an unresolvable @nx/devkit/src/... path. The migration's user-facing doc now spells this out as a best-guess rather than a guarantee, with instructions for the manual fix.
…-Healing CI Rerun]
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.
🎓 Learn more about Self-Healing CI on nx.dev
) ## Current Behavior The 23.0.0-beta.6 `@nx/devkit` deep-import migration (#35541) catches non-named-import shapes (default / namespace / side-effect / `require()` / dynamic `import()` / `jest.mock`-style calls) by running a regex sweep over the file: ```ts const FALLBACK_RE = /(['"])@nx\/devkit\/src\/[^'"\n]+?\1/g; updated = updated.replace( FALLBACK_RE, (_match, quote: string) => `${quote}${INTERNAL_SPECIFIER}${quote}` ); ``` That sweep matches **any** `'@nx/devkit/src/...'` literal anywhere in the file, regardless of context. As a result the migration mangled: - **Test fixtures inside template literals** — including the migration's own `update-deep-imports.spec.ts`. Examples observed in the wild: `packages/devkit/src/migrations/update-23-0-0/update-deep-imports.spec.ts`, `packages/expo/src/utils/expo-project-detection.spec.ts`, `packages/nuxt/src/plugins/plugin.spec.ts`, `packages/react-native/src/utils/react-native-project-detection.spec.ts`, etc. ([#35565](#35565)) - **`typeof import('@nx/devkit/src/...')` type queries** — e.g. `libs/shared/npm/src/lib/local-nx-utils/parse-target-string.ts` in nrwl/nx-console, where the type now claims `@nx/devkit/internal` while the runtime `importPath` next to it is built dynamically and still points at `src/...`. ([nrwl/nx-console#3131](nrwl/nx-console#3131)) - **Deep-import paths in comments**, doc strings, and arbitrary string-literal arguments to unrelated functions. ## Expected Behavior The migration only rewrites deep-import paths that are *actually* import sites. Everything else (template strings, type queries, comments, unrelated calls) is left alone. This is implemented by replacing the regex sweep with a TypeScript-AST visitor that only rewrites the string-literal argument of `CallExpression` nodes whose callee is one of: - `require` (identifier) - the dynamic-`import` keyword - `jest.mock` / `jest.unmock` / `jest.doMock` / `jest.dontMock` / `jest.requireActual` / `jest.requireMock` - `vi.mock` / `vi.unmock` / `vi.doMock` / `vi.dontMock` / `vi.requireActual` / `vi.requireMock` / `vi.importActual` / `vi.importMock` Type queries (`ImportTypeNode`), template literals, and comments are all naturally untouched because they are not `CallExpression` nodes — no allowlist needed. Quote style is preserved per literal. The named-import bucketing pass and the duplicate-collapse pass are unchanged. ### Tests 10 new unit tests: - 5 in a new `non-runtime string literals` block guarding template literals, `typeof import(...)` type queries, block comments, line comments, and unrelated call expressions. - 5 in a new `mock helper calls` block covering `jest.mock`, `jest.requireActual`, `vi.mock`, `vi.importActual`, and a paired `import` + `jest.mock` + `jest.requireActual` combination. All 39 unit tests pass; `nx build devkit` is clean. ## Related Issue(s) Follow-up to #35541. Workspaces that have already merged the bad rewrites need to revert those files by hand — there's no general way to undo the over-rewrites without losing the legitimate ones. --------- Co-authored-by: nx-cloud[bot] <71083854+nx-cloud[bot]@users.noreply.github.com> Co-authored-by: FrozenPandaz <FrozenPandaz@users.noreply.github.com>
2 participants
Current Behavior
After #34946,
@nx/devkitships a strictexportsmap. Deep imports like@nx/devkit/src/utils/...and@nx/devkit/src/generators/...are no longer reachable through Node module resolution. Workspaces upgrading to23.xthat reference those paths break with module-resolution errors at runtime / type-check time.Expected Behavior
nx migrateruns an automated rewrite that covers the realistic shapes of these deep imports.The migration walks every
.ts/.tsx/.cts/.mtsfile in the workspace and:@nx/devkit/src/...import declaration is parsed via the TypeScript compiler API (lazy-loaded withensurePackage). Specifiers in theinternal.tsre-export list go to@nx/devkit/internal; all others go to@nx/devkit. Mixed imports split into two declarations.require(...)calls, and dynamicimport(...)get the specifier swapped to@nx/devkit/internal(the safe default — it re-exports every previously deep-importable symbol).import { ... } from '@nx/devkit'andimport { ... } from '@nx/devkit/internal'declarations by(specifier, isTypeOnly), merging each 2+ group into one declaration with deduplicated specifiers. This handles both the duplicates the rewrite produced and any pre-existing devkit imports the user already had.Edits are stacked via
applyChangesToString(devkit's offset-tracking text-mutation helper), thenformatFilesnormalizes formatting.Example
Before:
After:
Tests
29 unit tests cover: single-bucket and mixed-bucket rewrites,
asaliases,import typeand inlinetypemodifiers, multi-line imports, side-effect / default / namespace fallbacks,require()and dynamicimport()fallbacks, quote-style preservation, pre-existing-import merge, public/internal independence, value-vs-type segregation, specifier deduplication, and a sanity test that every name inDEVKIT_INTERNAL_SYMBOLSis bucketed as internal.A user-facing
update-deep-imports.mdlives next to the implementation; the astro-docs build picks it up via the existingpackages/*/src/migrations/**/*.mdinput glob.Related Issue(s)
Follow-up to #34946.