Skip to content

feat: add Node.js ESM support via --add-js-extension flag for NestJS 12 compatibility - #114

Merged
KevinEdry merged 5 commits into
KevinEdry:mainfrom
Angel-Mario:feat--add-Node.js-ESM-support-via---experimental-vm-modules-flag-for-NestJS-12-compatibility
Jul 1, 2026
Merged

KevinEdry merged 5 commits into
KevinEdry:mainfrom
Angel-Mario:feat--add-Node.js-ESM-support-via---experimental-vm-modules-flag-for-NestJS-12-compatibility

Conversation

@Angel-Mario

Copy link
Copy Markdown
Contributor

Summary

Adds full ESM ("type": "module") support to the test suite by fixing type declarations in tsconfig.spec.json, enabling compatibility with Node.js ESM resolution and the upcoming NestJS 12 release.
( --add-js-extension Add .js extension to local import paths for ESM compatibility ).

Maybe we can use another flag ^_^ --esm ? --nest-esm ?

Other Changes

  • Added "reflect-metadata" to the types array in packages/nestjs-trpc/tsconfig.spec.json
  • This ensures Reflect.getMetadata / Reflect.defineMetadata type declarations from the reflect-metadata package are included during ts-jest compilation with --coverage
  • Previously, 3 decorator test suites failed with TS2339 errors under coverage mode because the types restriction (["jest", "node"]) excluded reflect-metadata augmentations

Why

NestJS 12 moves toward full ESM support. I`m using custom scripts to achieve similar behavior of this PR goal, but runs after generation, its not idoneous. See (not important just for reference of the workround for esm support without flag --add-js-extension)
https://github.com/Angel-Mario/nest-auth-trpc-rest-swagger-single/blob/master/src/libs/trpc/scripts/fix-trpc-generated.mjs
https://github.com/Angel-Mario/nest-auth-trpc-rest-swagger-single/blob/master/src/libs/trpc/scripts/watch-trpc.mjs

Running tests with --add-js-extension (required for ESM compatibility) triggers stricter type checking in ts-jest, which exposed this missing type declaration. Fixing it ensures the test suite works correctly in both CommonJS and ESM ("type": "module") environments.

Test Results

All 17 test suites pass, 78/78 tests green with full coverage.

Help changes

help-info-after-changes

Battle-tested with the build package after changes against this nestjs 12 project:

https://github.com/Angel-Mario/nest-auth-trpc-rest-swagger-single

…bility

Include "reflect-metadata" in the types array of tsconfig.spec.json to
ensure Reflect.getMetadata/defineMetadata type declarations are available
during test compilation with --coverage. This resolves TS2339 errors in
decorator source files when ts-jest performs stricter type checking.
@vercel

vercel Bot commented Jun 23, 2026

Copy link
Copy Markdown

@Angel-Mario is attempting to deploy a commit to the Kevin's Projects Team on Vercel.

A member of the Team first needs to authorize it.

@KevinEdry

Copy link
Copy Markdown
Owner

Thanks for this @Angel-Mario! The ESM gap is a real pain point and the implementation itself is solid — good test coverage and you threaded it through the watch config correctly, which is easy to miss.

Before I merge it though, I'd like to reshape the public-facing flag a bit. Could you rename --add-js-extension to --import-extension and have it take a value (js or none) instead of being a bare boolean? My reasoning is that a boolean locks us into a single behavior, whereas a valued flag like --import-extension=js describes what it actually does and leaves room to grow later without me having to keep bolting on more boolean flags. I'd also steer away from the --esm naming you floated, because ESM isn't really the right signal — a project on ESM with moduleResolution: "bundler" actually must not have the extensions, while NodeNext/Node16 requires them. So the thing that really decides this is the resolution mode, not whether you're on ESM, and naming the flag after the output (import-extension) keeps it honest.

The thing I'd love to see, but won't block on, is making this auto-detect by default. Whether you need .js extensions is fully determined by compilerOptions.moduleResolution in the user's tsconfig.json (NodeNext/Node16 → yes, bundler/classic → no), and since the CLI already reads package.json for entrypoint discovery, reading the nearby tsconfig for this is a pretty natural extension. That would make the whole thing zero-config — you'd just run generate and the output matches your tsconfig automatically, no flag to remember.

If you'd rather not take tsconfig parsing on in this PR, totally fine — just the rename to js/none is enough for me to merge, and I'll fast-follow with the auto default myself. The valued-flag shape is exactly what makes that follow-up clean, since auto becomes just another value rather than a breaking change. Wdyt? 👍

@Angel-Mario

Angel-Mario commented Jun 25, 2026 •

Copy link
Copy Markdown
Contributor Author

@KevinEdry That's a great idea for future-proofing. I also thought about implementing an auto-detection mechanism. I'm building a tool for the NestJS community, and with this approach, it will be easy to support v11 as a legacy version (from a post-v12 perspective).

I'll send the changes later.

@Angel-Mario Angel-Mario left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The auto detection imp went more hard than i expected, a lot of edge cases mainly with tsconfig "extends" support. I also added a simple change to the docs.

Image

Here`s the summary of changes (Ai generated) :

ESM Import Extension Support with Auto-Detection from tsconfig

The Commit replaces the simple --add-js-extension boolean flag with a more intelligent --import-extension option (js | none | auto) that can auto-detect whether .js extensions are needed by resolving the full tsconfig extends chain and checking compilerOptions.module / moduleResolution.

New file

  • cli/src/tsconfig.rs (593 lines, largely tests) — A full tsconfig resolver that follows the extends chain exactly like TypeScript, supporting:
    • extends as a string or array (TS 5+)
    • Relative paths (with/without .json extension)
    • Package references (@scope/pkg/tsconfig.json)
    • Node.js node_modules resolution for packages
    • Cycle detection via visited-path tracking
    • JSONC parsing (comments, trailing commas)

Key changes

  • cli/src/config.rs — Added detect_import_extension() function that resolves the full tsconfig chain and returns Some(true) for NodeNext/Node16, Some(false) otherwise, None if no tsconfig exists. Includes 10 new tests covering all resolution scenarios.
  • cli/src/cli/mod.rs — Replaced boolean --add-js-extension with --import-extension <js|none|auto> using clap ValueEnum.
  • cli/src/main.rs — Added resolve_import_extension() that maps the 3-way enum + auto-detection to a boolean.
  • Renamed add_js_extension → import_extension across generate.rs, watch.rs, generation.rs, generator/mod.rs, generator/server.rs, watcher/mod.rs.
  • cli/Cargo.toml — Added jsonc-parser dependency.
  • cli/src/lib.rs — Registered new tsconfig module.
  • cli/src/validation/tsc.rs — Refactored find_tsc_in_path() to extract find_tsc_executable_in_directory() helper and use find_map.
  • Tests: Updated all test call sites to pass the new import_extension parameter; added 3 new CLI integration tests (--import-extension in help, js, none).
  • cli/tests/generation.rs & cli/tests/validation.rs — Added false arg to run_generation calls.
  • cli/tests/generator/mod.rs — Fixed platform-specific absolute path for Windows in test.
  • docs/pages/docs/index.mdx — Updated docs to describe the new --import-extension flag and auto-detection.

Edge cases covered

  • No tsconfig.json → detect_import_extension returns None, falls back to false
  • Invalid JSON/JSONC → returns None, generates without extensions
  • Partial config (only module, no moduleResolution, or vice versa) → correctly infers from either
  • extends chain inheritance — resolves through parent tsconfigs to find the effective module/moduleResolution
  • Child override — if a child tsconfig overrides to ESNext/bundler, the parent's NodeNext is correctly overridden
  • Cyclic extends — cycle detection prevents infinite loops
  • Package extends — resolves @scope/pkg/tsconfig.json via node_modules traversal
  • TS 5+ extends arrays — multiple base configs merged in order
  • Relative extends without .json — TypeScript-like resolution (tries file, file + .json, directory + tsconfig.json)

@KevinEdry

Copy link
Copy Markdown
Owner

This is really nice work @Angel-Mario — the extends resolver is more thorough than I expected, you covered the chain, arrays, package references and cycles, and the NodeNext/Node16 detection rule is spot on. Tests are green for me too 👍

I do think there's one edge case still hiding though, and it's a sneaky one. Right now the auto-detection runs against current_dir() in main.rs, but generation derives its base from the root module's location (base_directory = root_module_path.parent() in generate.rs), and resolve_compiler_options only looks for a tsconfig.json directly in the directory you hand it — it doesn't walk up the tree the way tsc actually does. Those two things quietly cancel out when you run the CLI from the project root (cwd happens to be where the tsconfig lives), which is why the tests pass — but they all create the tsconfig in the same temp dir they query, so the "tsconfig is at the root while my source is in src/" layout never gets exercised, and that's the normal real-world setup.

Where it bites:

  • Running from a subdirectory → cwd has no tsconfig → resolve_compiler_options returns None → we silently fall back to no extensions, even on a NodeNext project. Which is exactly the pain this PR is trying to kill 😅
  • --entrypoint apps/api/src/app.module.ts from a monorepo root → generation uses apps/api/src, but detection reads the root tsconfig, so if the sub-package's module setting differs from the root's, auto-detect gets it wrong.

The fix is small and makes the whole thing way more robust: run detection from the generation base_directory instead of cwd, and let resolve_compiler_options walk upward to the nearest tsconfig.json (mirroring how TypeScript discovers it). After that it stops mattering where you invoke the CLI from. Could you fold that in? Happy to pair on it or push a patch to the branch if it's easier.

One last tiny thing, not a blocker: moduleResolution is a slightly imperfect signal for whether the runtime needs .js — e.g. module: ESNext + moduleResolution: bundler but actually running on plain Node ESM will still need extensions, and auto will say "no." That's fine since --import-extension=js is the escape hatch, I'd just add a sentence to the docs so nobody's surprised by it. Wdyt?

@Angel-Mario

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback, @KevinEdry. I was always running the CLI from the project root, that edge case slipped past me. The tests also didn't expose it since they create the tsconfig.json in the same temporary directory they're resolving from, but now ill add some new tests to cover that.

The moduleResolution point also makes sense. I'll add a note to the documentation explaining that limitation so the behavior is explicit.

It's a relatively small change, I`m going to implement it now

@Angel-Mario Angel-Mario closed this Jul 1, 2026
@Angel-Mario
Angel-Mario deleted the feat--add-Node.js-ESM-support-via---experimental-vm-modules-flag-for-NestJS-12-compatibility branch July 1, 2026 03:40
@Angel-Mario
Angel-Mario restored the feat--add-Node.js-ESM-support-via---experimental-vm-modules-flag-for-NestJS-12-compatibility branch July 1, 2026 03:43
@Angel-Mario Angel-Mario reopened this Jul 1, 2026
@Angel-Mario

Angel-Mario commented Jul 1, 2026 •

Copy link
Copy Markdown
Contributor Author

I changed the branch name and did'nt know it'll close the PR. It was my first time doing that .... sorry

@Angel-Mario

Angel-Mario commented Jul 1, 2026 •

Copy link
Copy Markdown
Contributor Author

It's late. I'll end this tomorrow (Wednesday) in the morning, check for improvements, find some edge cases, test on some projects, and write the summary of changes.

Sorry again for the lack of professionality (with the Branch rename).

@Angel-Mario Angel-Mario left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix(cli): anchor tsconfig discovery to base_directory and walk up ancestor dirs

resolve_compiler_options previously only checked for tsconfig.json in
the exact directory passed to it, returning None immediately if the
file wasn't there. TypeScript itself walks up the directory tree until
it finds a config file; this commit mirrors that behaviour.

Additionally, resolve_import_extension was called in main.rs using
current_dir() before base_directory was known, meaning detection and
generation were anchored to different directories when the CLI was
invoked from a subdirectory or with a deep --entrypoint path.

Changes:

  • tsconfig.rs: replace direct join() check with upward directory walk
  • main.rs: pass raw Option down to subcommands
    instead of a pre-resolved bool
  • generate.rs: call resolve_import_extension after base_directory is
    computed from root_module_path.parent()
  • watch.rs: same fix as generate.rs
  • tsconfig.rs tests: add test_tsconfig_in_parent_directory,
    test_tsconfig_found_in_grandparent, test_no_tsconfig_anywhere_returns_none
  • config.rs tests: add test_detect_import_extension_from_subdirectory
    covering the common root/tsconfig.json + root/src/ layout
  • cli/mod.rs: expand --import-extension help text with bundler+ESM caveat
  • docs/index.mdx: update import-extension section to document walk-up
    behaviour and add Callout for the bundler mode limitation

I also create a local monorepo example for testing. Everything its working as intend it. If you think that the walk up ancestor dirs for searching the TSC its not necessary let me know to crop it

@vercel

vercel Bot commented Jul 1, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nestjs-trpc-docs Ready Ready Preview, Comment Jul 1, 2026 8:19pm

Request Review

@KevinEdry KevinEdry left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is exactly it @Angel-Mario, really well done 🙌 The walk-up in resolve_compiler_options plus anchoring detection to base_directory after it's derived from the root module closes both the run-from-a-subdirectory and monorepo --entrypoint cases cleanly, and I love that you added tests for the parent/grandparent layouts specifically — that root/tsconfig.json + root/src/ shape is the one that matters most for NestJS and it's now locked in. The docs caveat on bundler mode is a nice touch too.

And to answer your question directly: yes, keep the walk-up — it's not optional, it's the heart of the fix. The most common NestJS setup (tsconfig at the root, source under src/) only resolves correctly because of it, so cropping it would quietly bring back the original bug.

One tiny thing, and genuinely don't let it hold anything up: resolve_import_extension is now copy-pasted into both generate.rs and watch.rs. Since they both already pull ImportExtensionValue from super, it'd be a touch cleaner as a single helper in cli/mod.rs — but that's a nit, not a blocker.

Approving this. Thanks for sticking with it through all the iterations and for building the monorepo example to prove it out — this is a great addition to the library 🎉

@KevinEdry
KevinEdry merged commit aa29b3a into KevinEdry:main Jul 1, 2026
5 checks passed
@Angel-Mario

Copy link
Copy Markdown
Contributor Author

I know the repetition in resolve_import_extension isn't great. I didn't extract it into a helper because I was expecting to do something different in watch.rs. In the end, that wasn't necessary, and I forgot to come back and factor it out.

Thanks for approving the PR @KevinEdry . I'm excited to use this version instead of relying on some lame scripts in a folder.

After graduation I'm planning to move on to @trpc/openapi and work on improving the OpenAPI response generation. Right now it only generates 200 success responses and doesn't infer error responses from context (such as middleware or TRPCError throws). Hopefully I'll be back in the future with another PR to add compatibility with nestjs-trpc 😂

This branch was successfully deployed

1 active deployment
Preview — 5a5bdf80 Deployed Jul 1, 2026 by vercel[bot]
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.

2 participants