Skip to content

fix: add .js extensions to relative imports (#238) - #239

Open
laazyj wants to merge 1 commit into
dagrejs:masterfrom
laazyj:fix/nodenext-type-resolution-238
Open

laazyj wants to merge 1 commit into
dagrejs:masterfrom
laazyj:fix/nodenext-type-resolution-238

Conversation

@laazyj

@laazyj laazyj commented Aug 18, 2026

Copy link
Copy Markdown

Problem

Since 4.0.5 the package is "type": "module", but the emitted .d.ts files still use extensionless relative specifiers (export { Graph } from './lib/graph'). Under moduleResolution node16/nodenext those are invalid, so a dependent gets:

node_modules/@dagrejs/graphlib/dist/types/index.d.ts(30,23): error TS2834:
Relative import paths need explicit file extensions in ECMAScript imports
when '--moduleResolution' is 'node16' or 'nodenext'.

The bug has a quieter second face that is arguably worse. With skipLibCheck: true — the common default — the error is silenced, but the declarations still fail to resolve and every graphlib export degrades to any. Type safety is gone at every call site with nothing on screen to say so.

Reported in #238. Only index.ts's type-only re-export of ./lib/types.js already carried an extension, so the convention was already half-applied in the repo.

Reproduction

A consumer project resolving @dagrejs/graphlib through node_modules and the exports map, moduleResolution: nodenext:

before after
skipLibCheck: false 4 × TS2834 on dist/types/index.d.ts clean
skipLibCheck: true no error, but const n: number = g.nodes() type-checksGraph is any correctly errors

Fix

1. Add .js to every relative specifier in lib/** and index.ts. Purely mechanical, 54 lines, one shape:

-import {Graph} from '../graph';
+import {Graph} from '../graph.js';

2. Compile tsconfig.build.json with nodenext (2 lines), so this cannot silently come back. That config already scopes itself to exactly the shipped surface (include: ["lib/**/*", "index.ts"], exclude: ["test/**/*"]), so the build now rejects a bad specifier at the source line rather than emitting declarations that break consumers:

lib/alg/dijkstra.ts(1,21): error TS2835: ... Did you mean '../graph.js'?

The emitted declarations are byte-identical to the previous bundler-resolved output — verified with diff -r. tsconfig.json stays on "bundler", so the extensionless imports throughout test/** and src/** are untouched and need no churn.

I looked at enforcing this with ESLint instead, but no installed plugin has such a rule (it would mean adding eslint-plugin-import), and it would be strictly weaker — import/extensions pattern-matches specifiers, whereas tsc actually resolves them.

3. Add test/dist-types.test.ts for the half that compiling this repo cannot cover: that a dependent resolving the package through node_modules and the "exports" map gets working types. That wiring can regress independently, as #233 showed. It builds declarations into a temp package next to a copy of the real package.json, then type-checks a consumer against it under nodenext with skipLibCheck off. The @ts-expect-error on a deliberate misuse is what catches the degrade-to-any case.

The test is hermetic: it builds from current source rather than reading the committed dist/, so it leaves the working tree alone and cannot pass on a stale artifact.

Backward compatibility

The change is additive — extensionless specifiers were valid in only some resolution modes, .js resolves in all of them. Verified with a consumer matrix (skipLibCheck off, plus a @ts-expect-error proving types genuinely resolve rather than falling back to any):

moduleResolution result
node (node10), commonjs and esnext pass
bundler pass
node16, nodenext pass
classic fails — identically before and after, as it cannot resolve node_modules at all

Across TypeScript 4.0.8, 4.5.5, 5.0.4 and 5.9.3 under node10 resolution: all pass.

Runtime is unaffected, since esbuild inlines every relative import — 0 relative specifiers survive into any bundle. The unminified dist/graphlib.js rebuilt from this branch differs from the committed one by only the pending 4.0.54.0.6-pre version string, i.e. no logic change. CJS require(), ESM import and both IIFE bundles smoke-test working.

Validation

  • Reproduced on a clean consumer before fixing; both faces of the failure confirmed gone after.
  • Each guard checked to be load-bearing: reverting a single specifier fails the new test and the nodenext build.
  • make lint, make build, make test all pass — 259 tests, 21 suites.

Note on dist/

dist/ is not included here. The history shows it is regenerated by the maintainer in the release build (prepublishOnly runs npm run build), and contributor fixes such as #235 do not touch it; #237 only did because its fix was a dist rename. The committed dist/types therefore still carries the bug until the next release build regenerates it. Happy to commit the rebuilt dist/ instead if you would prefer it in the PR — note that rebuilding also picks up the pending 4.0.54.0.6-pre version string, since the committed bundles predate that bump.

Closes #238

Since 4.0.5 the package is "type": "module", but the emitted .d.ts
files still used extensionless relative specifiers. Under
moduleResolution node16/nodenext those are invalid, so consumers get
TS2834 on dist/types/index.d.ts -- or, with the common skipLibCheck
default, no error at all while every graphlib export silently degrades
to `any`.

Add the .js extension to every relative specifier in lib/** and
index.ts. Only index.ts's type-only re-export had one already, so the
convention was already half-applied. The change is additive:
extensionless specifiers were valid only in some resolution modes,
whereas .js resolves in all of them. The runtime bundles are
unaffected, as esbuild inlines every relative import.

To keep the convention applied, compile tsconfig.build.json with
nodenext. It already scopes itself to the shipped surface (lib/** and
index.ts, excluding test/**), so the build now rejects a bad specifier
at the source line rather than emitting declarations that break
consumers. Output is byte-identical to the previous bundler-resolved
emit. tsconfig.json stays on "bundler", leaving the extensionless
imports under test/** and src/** untouched.

Add test/dist-types.test.ts for the half a compile of this repo cannot
cover: that a dependent resolving the package through node_modules and
the "exports" map gets working types. It builds declarations into a
temp package alongside the real package.json, then type-checks a
consumer against it under nodenext with skipLibCheck off; a
@ts-expect-error on a deliberate misuse catches the degrade-to-any
case.

Verified:
* Reproduced first: a consumer under nodenext fails with TS2834, and
  with skipLibCheck on loses its types silently. Both clear after.
* The new test and the nodenext build config each fail when a single
  specifier is reverted, so both guards are load-bearing.
* Backward compatibility: a consumer type-checks clean under
  moduleResolution node10 (commonjs and esnext), bundler, node16 and
  nodenext, on TypeScript 4.0, 4.5, 5.0 and 5.9. Only "classic" fails,
  identically before and after, as it cannot resolve node_modules.
* Runtime unchanged: no relative specifier survives into any bundle,
  the unminified bundle differs only by the pending version string,
  and CJS, ESM and both IIFE bundles smoke-test working.
* make lint, make build and make test all pass (259 tests).

dist/ is left to be regenerated by the maintainer's release build.

Closes dagrejs#238
@laazyj

laazyj commented Sep 13, 2026

Copy link
Copy Markdown
Author

Hi - just nudging this for visibility as it still blocks me being able to merge 4.0.5

laazyj added a commit to laazyj/composureCDK that referenced this pull request Sep 13, 2026
PR #458 bundled 13 updates and failed CI for three unrelated reasons.
Two of them are upstream blockers this repo cannot fix, so they get
`ignore` entries.

@dagrejs/graphlib 4.0.5 added `"type": "module"` but still ships .d.ts
files whose relative imports carry no extension. Under NodeNext nothing
in the package resolves, so `json` and `alg` go error-typed and
core/src/compose.ts collects 19 `no-unsafe-*` errors. `tsc` passes --
skipLibCheck hides the TS2834s and the error type degrades to `any`,
which only the type-aware lint rules object to. Reported as
dagrejs/graphlib#238; fix open as dagrejs/graphlib#239.

The pin is the exact release, not a range, so it clears itself: 4.0.6
will be proposed on the next weekly run and CI on that PR is what tells
us whether the fix shipped. A `>=4.0.5` range would turn a temporary
block into a permanent one that nothing ever reminds us to lift.

typescript >= 7 is blocked because typescript-eslint cannot run on it:
the parser's peer range is `>=4.8.4 <6.1.0` and the package throws
"typescript-eslint does not support TS 7.0" on load. A TS 7 bump fails
`npm install` outright in all 12 cdk-floors shards. Unlike the graphlib
pin this one has no automatic expiry, so the comment names
typescript-eslint/typescript-eslint#10940 and the condition for
deleting it.

Splitting the toolchain out of the `npm-dependencies` catch-all stops
either blocker holding back the other 11 updates it was grouped with.
The group is the set coupled by peer ranges -- typescript,
typescript-eslint, the eslint packages and eslint-config-prettier.
`prettier` is deliberately excluded: it declares no peer dependencies,
so it shares none of that coupling, and grouping it would park a
routine formatter patch behind a stuck compiler.

Claude-Session: https://claude.ai/code/session_01UwyS3cfCR11tJwuQbZq3hX
laazyj added a commit to laazyj/composureCDK that referenced this pull request Sep 13, 2026
PR #458 bundled 13 updates and failed CI for three unrelated reasons.
Two of them are upstream blockers this repo cannot fix, so they get
`ignore` entries.

@dagrejs/graphlib 4.0.5 added `"type": "module"` but still ships .d.ts
files whose relative imports carry no extension. Under NodeNext nothing
in the package resolves, so `json` and `alg` go error-typed and
core/src/compose.ts collects 19 `no-unsafe-*` errors. `tsc` passes --
skipLibCheck hides the TS2834s and the error type degrades to `any`,
which only the type-aware lint rules object to. Reported as
dagrejs/graphlib#238; fix open as dagrejs/graphlib#239.

The pin is the exact release, not a range, so it clears itself: 4.0.6
will be proposed on the next weekly run and CI on that PR is what tells
us whether the fix shipped. A `>=4.0.5` range would turn a temporary
block into a permanent one that nothing ever reminds us to lift.

typescript >= 7 is blocked because typescript-eslint cannot run on it:
the parser's peer range is `>=4.8.4 <6.1.0` and the package throws
"typescript-eslint does not support TS 7.0" on load. A TS 7 bump fails
`npm install` outright in all 12 cdk-floors shards. Unlike the graphlib
pin this one has no automatic expiry, so the comment names
typescript-eslint/typescript-eslint#10940 and the condition for
deleting it.

Splitting the toolchain out of the `npm-dependencies` catch-all stops
either blocker holding back the other 11 updates it was grouped with.
The group is the set coupled by peer ranges -- typescript,
typescript-eslint, the eslint packages and eslint-config-prettier.
`prettier` is deliberately excluded: it declares no peer dependencies,
so it shares none of that coupling, and grouping it would park a
routine formatter patch behind a stuck compiler.

Claude-Session: https://claude.ai/code/session_01UwyS3cfCR11tJwuQbZq3hX
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.

4.0.5: "type": "module" makes the shipped .d.ts files unresolvable under NodeNext (TS2834)

1 participant