Skip to content

build: migrate deprecated moduleResolution node to modern equivalents - #5202

Open
robert-md-or wants to merge 1 commit into
ngrx:mainfrom
robert-md-or:fix/typescript-7-moduleresolution
Open

build: migrate deprecated moduleResolution node to modern equivalents#5202
robert-md-or wants to merge 1 commit into
ngrx:mainfrom
robert-md-or:fix/typescript-7-moduleresolution

Conversation

@robert-md-or

Copy link
Copy Markdown
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[x] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Several tsconfig.json files across the workspace set moduleResolution: "node", which TypeScript has marked for removal as part of the
target/moduleResolution deprecation cleanup tracked in #5163. Left as-is, these configs
will start failing once the workspace upgrades to a TypeScript version
where the value is removed.

What is the new behavior?

Migrates moduleResolution: "node" away from the 5 affected tsconfig
files:

  • projects/www/src/app/examples/__base/tsconfig.json: already used
    module: "ES2022", so moduleResolution moved directly to "bundler".
  • tsconfig.docs.json, modules/schematics/tsconfig.build.json,
    modules/eslint-plugin/tsconfig.schematics.json,
    modules/eslint-plugin/spec/fixtures/tsconfig.json: these use (or
    default to) module: "commonjs" for code executed directly via Node
    (schematics, docs generation, test fixtures). Migrating these to
    "bundler" or "nodenext" surfaced unrelated pre-existing type
    errors or ECMAScript import-extension requirements incompatible with
    the current dependency graph. Moved moduleResolution to "node10"
    instead — TypeScript's renamed, non-deprecated equivalent of the old
    "node" behavior, with no change in resolution semantics.

One file from the original scope,
modules/store/spec/ngc/tsconfig.ngc.json, is intentionally left
untouched: it isn't referenced by any Nx target and appears to be dead
code left over from an old ngc/Ivy AOT compiler test setup. Left for
maintainers to decide whether to fix or remove.

Verified with:

  • tsc --noEmit on tsconfig.docs.json,
    modules/schematics/tsconfig.build.json, and
    modules/eslint-plugin/tsconfig.schematics.json (no new errors
    introduced versus the pre-existing baseline)
  • pnpm nx run eslint-plugin:test (78 test files, 554 tests, 0 type
    errors)

This PR addresses the moduleResolution portion of the broader scope
outlined in #5163. Remaining categories from that issue
(the one remaining target: "ES5" config, and baseUrl/rootDir) are
being tracked separately and are not part of this PR.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

Related to #5163 (partial — see scope note above; not marked as closing
since further work from that issue is still in progress).

@robert-md-or

Copy link
Copy Markdown
Contributor Author

Verification Guide for Reviewers

Why two different target values (bundler vs node10) instead of one consistent replacement?

moduleResolution: "node" is deprecated, but the correct modern replacement depends on how each file's code actually runs:

  • Files bundled/transpiled by a bundler (webpack, esbuild, etc.) can safely move to moduleResolution: "bundler".
  • Files whose code is executed directly by Node (schematics, docs generation scripts, test fixtures) generally should not use "bundler", since it assumes bundler-style resolution (e.g. extensionless imports resolved through package.json "exports") that plain Node execution doesn't provide the same way.

I tested both directions before deciding per file:

  1. projects/www/src/app/examples/__base/tsconfig.json already had module: "ES2022", so moduleResolution: "bundler" was a direct, safe match — verified with a clean tsc --noEmit.

  2. For the remaining 4 files (tsconfig.docs.json, modules/schematics/tsconfig.build.json, modules/eslint-plugin/tsconfig.schematics.json, modules/eslint-plugin/spec/fixtures/tsconfig.json), I first tried migrating to "nodenext" (paired with module: "nodenext"), since these use CommonJS today. This surfaced real compile errors from @angular/compiler-cli's own .d.ts files not using explicit .js extensions in relative imports — a strict ECMAScript-import requirement of node16/nodenext that a third-party dependency doesn't meet. Not something fixable from this repo.

    I then tried "bundler" on tsconfig.docs.json specifically, to compare: it produced the same set of 16 pre-existing errors (confirmed by running tsc --noEmit against the untouched, original file too — identical errors, so these aren't introduced by this change).

    Given both alternatives either broke things or offered no improvement over the status quo, I used moduleResolution: "node10" — TypeScript's renamed, non-deprecated identifier for the exact same resolution algorithm previously named "node". No behavior changes, only the deprecated name is removed.

How I verified each file:

File Verification
projects/www/src/app/examples/__base/tsconfig.json tsc --noEmit — clean
tsconfig.docs.json tsc --noEmit — same 16 pre-existing errors as the unmodified baseline, nothing new
modules/schematics/tsconfig.build.json tsc --noEmit — clean
modules/eslint-plugin/tsconfig.schematics.json tsc --noEmit — clean
modules/eslint-plugin/spec/fixtures/tsconfig.json Can't run tsc --noEmit directly (fixture-only files entry), so ran the full eslint-plugin test suite instead: pnpm nx run eslint-plugin:test — 78 test files, 554 tests, 0 type errors

Left untouched: modules/store/spec/ngc/tsconfig.ngc.json — confirmed via grep that it isn't referenced by any project.json/angular.json/nx.json target. Appears to be dead code from an old ngc/Ivy AOT test setup. Flagging for maintainer input rather than guessing at removal.

Comment thread modules/eslint-plugin/spec/fixtures/tsconfig.json
@markostanimirovic

Copy link
Copy Markdown
Member

@robert-md-or

After merging your previous PR, this one has conflicts.

@robert-md-or

Copy link
Copy Markdown
Contributor Author

Thanks @markostanimirovic, I'm already on it.

I think the conflict happened because my previous PR modified the same tsconfig/config files (or the surrounding configs) that this change touches, so merging it changed the base branch and now this branch can't be fast-forwarded cleanly. I'm already on it, I'll rebase/merge the latest main into this branch, resolve the conflicts (keeping the intended moduleResolution changes), run tsc --noEmit and the relevant test/linters to make sure nothing regresses, and push an updated branch shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants