fix(types): rewrite .d.mts relative specifiers to .mjs#121
Open
fix(types): rewrite .d.mts relative specifiers to .mjs#121
Conversation
`emitDualDeclarations` was producing `.d.mts` files whose relative specifiers ended in `.js`. Under TS's node16+ ESM resolver, a `.js` specifier in a `.d.mts` is paired with the adjacent `.d.ts`, which — in a dual-published package whose root `package.json` has `"type": "commonjs"` — is treated as CJS-flavoured. Strict ESM consumers then surface type-resolution errors against an otherwise valid package. `rewriteSpecifier` is only ever called for the `.d.mts` branch, so the extension it emits should be `.mjs`, which pairs with the `.d.mts` twin and keeps the resolution chain in ESM throughout. The `.d.cts` branch is unaffected — those remain content-identical copies of the source `.d.ts`, which CJS resolution handles via extensionless specifiers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Tests
- Add focused unit test for the file case (was only exercised via the
integration test): `from './foo'` → `from './foo.mjs'` when `./foo.d.ts`
exists.
- Add test for resolving when only the `.d.mts` twin is present alongside
the source, exercising the second branch of `rewriteSpecifier`.
- Add test that runs all three module-specifier shapes through one pass
(`from '...'`, bare `import '...'`, dynamic `import('...')`).
Docs
- Add a `## copy-package-json` subsection covering dual-declaration
emission, the per-condition `exports` shape, and the `.mjs` rewrite
rules with the rationale for `.mjs` over `.js`.
- The original feature (PR #120) shipped without README coverage; this
fills that gap alongside the specifier-extension fix.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Publishes the .d.mts specifier-extension fix so downstream packages can drop their local workarounds (e.g. graphql-apollo-server's build:4a-fix-dmts post-step). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes dual-declaration emission so .d.mts files rewrite extensionless relative specifiers to .mjs (instead of .js), ensuring TypeScript’s node16+/NodeNext resolver pairs ESM declarations with their .d.mts twins rather than adjacent CJS-flavoured .d.ts.
Changes:
- Update
rewriteSpecifierto emit.mjs(and/index.mjs) for resolved relative specifiers in.d.mtsoutput. - Expand/adjust unit + integration test assertions to lock in
.mjsrewriting behavior and cover additional specifier shapes. - Add README documentation for
copy-package-jsondual type declaration emission and the.mjsrewrite rationale.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/util/copy-package-json-from-config.ts | Switches relative specifier rewrite target from .js to .mjs for .d.mts generation and updates inline rationale/comments. |
| src/util/copy-package-json-from-config.spec.ts | Updates assertions to expect .mjs and adds focused tests around file/directory resolution and specifier forms. |
| readme.md | Documents dual declaration emission and the .mjs rewrite rules/rationale. |
| package.json | Bumps version to 5.0.0-beta.1. |
| package-lock.json | Updates lockfile version metadata to 5.0.0-beta.1. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Resolves the moderate XSS advisory (PostCSS Stringify Output unescaped `</style>`) reaching us via vitest > vite > postcss in dev. Within the existing semver range; no package.json change, lockfile-only. Surfaced by the PR check on #121. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses Copilot review comments on #121. `rewriteSpecifier` already resolves both `.d.ts` and `.d.mts` adjacents in the directory branch, but the README bullet and the source-comment example only mentioned `.d.ts`. Updated both to spell out either form. Also adds a sibling unit test covering the case where only an `index.d.mts` is present in a directory (matching the existing "only a .d.mts twin alongside the source" test for the file case). Co-Authored-By: Claude Opus 4.7 (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
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.
Summary
Follow-up to #120.
emitDualDeclarationswas producing.d.mtsfiles whose relative specifiers ended in.js, which under TS's node16+ ESM resolver pairs them with the adjacent.d.ts. In a dual-published package whose rootpackage.jsonhas"type": "commonjs", that.d.tsis treated as CJS-flavoured — so strict-ESM consumers see type-resolution mismatches against an otherwise-valid package.rewriteSpecifieris only ever called from the.d.mtsbranch ofemitDualDeclarations, so the extension it emits should be.mjs, which pairs with the.d.mtstwin and keeps the resolution chain in ESM throughout. The.d.ctsbranch is unaffected.What changed
src/util/copy-package-json-from-config.ts— two-character fix inrewriteSpecifier(.js→.mjsin both the file and directory branches), plus updated comments explaining the rationale.src/util/copy-package-json-from-config.spec.ts— updated the three existing assertions that pinned the buggy.jsoutput, then added focused coverage for: the file case (was only exercised via the integration test), resolving when only the.d.mtstwin exists, and exercising all three module-specifier shapes (from '...', bareimport '...', dynamicimport('...')) in one pass.readme.md— added a## copy-package-jsonsubsection covering dual-declaration emission, the per-conditionexportsshape, and the.mjsrewrite rules with rationale. The original feature shipped without README coverage; this fills that gap.Discovered via
Building @makerx/graphql-apollo-server@2.0.0-beta.0 against the just-published
@makerx/ts-toolkit@5.0.0-beta.0produced.d.mtsfiles like:Strict-NodeNext consumers downstream see mis-resolved declarations. After this fix:
Why
.mjsrather than.jsUnder
moduleResolution: "node16"/"nodenext", TypeScript pairs each module specifier with the declaration whose extension matches the JS extension:.d.mts'./foo.js'./foo.d.tspackage.jsontype)'./foo.mjs'./foo.d.mtsUsing
.mjskeeps the entire declaration chain in one module kind, so ESM consumers don't accidentally pull in CJS-shaped types.Test plan
npm test— 17/17 pass (was 14, added 3)npm run build— ts-toolkit dogfoods: its owndist/index.d.mtsnow readsfrom './util/copy-package-json-from-config.mjs'attw --pack dist— green across node10 / node16 (CJS) / node16 (ESM) / bundler (caveat: attw doesn't actually validate this class of fix, so the byte-level inspection is the real verification).d.mtsfiles now use.mjsspecifiers without any local workaround🤖 Generated with Claude Code