Repository navigation
feat: add Node.js ESM support via --add-js-extension flag for NestJS 12 compatibility - #114
Conversation
…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.
|
@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. |
|
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 The thing I'd love to see, but won't block on, is making this auto-detect by default. Whether you need If you'd rather not take tsconfig parsing on in this PR, totally fine — just the rename to |
|
@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
left a comment
There was a problem hiding this comment.
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.
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 theextendschain exactly like TypeScript, supporting:extendsas a string or array (TS 5+)- Relative paths (with/without
.jsonextension) - Package references (
@scope/pkg/tsconfig.json) - Node.js
node_modulesresolution for packages - Cycle detection via visited-path tracking
- JSONC parsing (comments, trailing commas)
Key changes
cli/src/config.rs— Addeddetect_import_extension()function that resolves the full tsconfig chain and returnsSome(true)forNodeNext/Node16,Some(false)otherwise,Noneif no tsconfig exists. Includes 10 new tests covering all resolution scenarios.cli/src/cli/mod.rs— Replaced boolean--add-js-extensionwith--import-extension <js|none|auto>using clapValueEnum.cli/src/main.rs— Addedresolve_import_extension()that maps the 3-way enum + auto-detection to a boolean.- Renamed
add_js_extension→import_extensionacrossgenerate.rs,watch.rs,generation.rs,generator/mod.rs,generator/server.rs,watcher/mod.rs. cli/Cargo.toml— Addedjsonc-parserdependency.cli/src/lib.rs— Registered newtsconfigmodule.cli/src/validation/tsc.rs— Refactoredfind_tsc_in_path()to extractfind_tsc_executable_in_directory()helper and usefind_map.- Tests: Updated all test call sites to pass the new
import_extensionparameter; added 3 new CLI integration tests (--import-extensionin help,js,none). cli/tests/generation.rs&cli/tests/validation.rs— Addedfalsearg torun_generationcalls.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-extensionflag and auto-detection.
Edge cases covered
- No tsconfig.json →
detect_import_extensionreturnsNone, falls back tofalse - Invalid JSON/JSONC → returns
None, generates without extensions - Partial config (only
module, nomoduleResolution, or vice versa) → correctly infers from either extendschain inheritance — resolves through parent tsconfigs to find the effectivemodule/moduleResolution- Child override — if a child tsconfig overrides to
ESNext/bundler, the parent'sNodeNextis correctly overridden - Cyclic extends — cycle detection prevents infinite loops
- Package extends — resolves
@scope/pkg/tsconfig.jsonvianode_modulestraversal - TS 5+ extends arrays — multiple base configs merged in order
- Relative extends without
.json— TypeScript-like resolution (tries file, file +.json, directory +tsconfig.json)
|
This is really nice work @Angel-Mario — the I do think there's one edge case still hiding though, and it's a sneaky one. Right now the auto-detection runs against Where it bites:
The fix is small and makes the whole thing way more robust: run detection from the generation One last tiny thing, not a blocker: |
|
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 The It's a relatively small change, I`m going to implement it now |
|
I changed the branch name and did'nt know it'll close the PR. It was my first time doing that .... sorry |
|
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
left a comment
There was a problem hiding this comment.
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
KevinEdry
left a comment
There was a problem hiding this comment.
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 🎉
|
I know the repetition in 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 |
Summary
Adds full ESM (
"type": "module") support to the test suite by fixing type declarations intsconfig.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
"reflect-metadata"to thetypesarray inpackages/nestjs-trpc/tsconfig.spec.jsonReflect.getMetadata/Reflect.defineMetadatatype declarations from thereflect-metadatapackage are included during ts-jest compilation with--coverageTS2339errors under coverage mode because thetypesrestriction (["jest", "node"]) excludedreflect-metadataaugmentationsWhy
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
Battle-tested with the build package after changes against this nestjs 12 project:
https://github.com/Angel-Mario/nest-auth-trpc-rest-swagger-single