Skip to content

fix: nest types under import/require in exports maps - #360

Open
OsamaAnsar wants to merge 1 commit into
tursodatabase:mainfrom
OsamaAnsar:fix/cjs-typescript-module-resolution
Open

OsamaAnsar wants to merge 1 commit into
tursodatabase:mainfrom
OsamaAnsar:fix/cjs-typescript-module-resolution

Conversation

@OsamaAnsar

Copy link
Copy Markdown

Fixes #182.

The bug

TypeScript's Node16/NodeNext resolver matches exports conditions by JSON key order against the currently-active condition set. Both @libsql/client and @libsql/core had types as a sibling key listed before import/require:

"./api": {
    "types": "./lib-esm/api.d.ts",
    "import": "./lib-esm/api.js",
    "require": "./lib-cjs/api.js"
}

Since types itself is always one of the active conditions (require, types, node or import, types, node), and it's the first matching key in the object, TypeScript picks it regardless of whether resolution is actually happening in CJS or ESM mode. So a CJS consumer's require resolution still lands on the ESM .d.ts, which then recursively resolves its own imports in ESM mode — producing exactly the TS1479 error from #182 ("this file is CommonJS ... but the referenced file is an ECMAScript module").

This affects @libsql/core too, which matters because @libsql/client's own entry point imports from it — fixing only the client package wasn't enough to fully resolve the repro.

The fix

  • Nest types inside each import/require block instead of as a shared sibling, in both packages' exports maps (preserving every existing conditional target — workerd/deno/edge-light/etc. mappings are unchanged, just relocated).
  • Flip declaration: false → true in both packages' tsconfig.build-cjs.json, since the new require.types entries point at lib-cjs/*.d.ts, which didn't previously exist.

Verification

Reproduced the issue exactly as described: a Node16-resolution CJS+TS consumer importing @libsql/client fails tsc --noEmit with TS1479 on current main, and passes cleanly after this fix. Checked a parallel ESM consumer too, to confirm no regression on that side (still resolves and typechecks correctly).

Ran the full jest suite before and after: identical result both times, 183 passing / 14 failing (all 14 are a pre-existing, unrelated Windows-specific EPERM issue in the test harness's temp-directory cleanup — reproduced identically on unmodified main, confirmed via git stash).

…base#182)

TypeScript's Node16/NodeNext resolver matches exports conditions by
JSON key order against the active condition set. With a top-level
`types` key sitting as a sibling of `import`/`require`, `types`
always won regardless of which module system was actually being
resolved - so a CJS consumer's `require` would still get pointed at
the ESM `.d.ts`, which then recursively resolves its own imports in
ESM mode and produces TS1479 ("this file is CommonJS ... but the
referenced file is an ECMAScript module").

Fixes it in both @libsql/client and @libsql/core (the bug affects
@libsql/core's own exports map too, and @libsql/client's CJS entry
imports from it) by nesting `types` inside each `import`/`require`
block instead, and flipping `declaration: false` -> `true` in each
package's tsconfig.build-cjs.json so lib-cjs actually has .d.ts files
for the `require` condition to point at - the previous shape assumed
CJS types could just reuse the ESM ones, which is exactly what broke
CJS resolution.

Verified against the issue's own reproduction (a Node16-resolution
CJS+TS consumer importing `@libsql/client`): `tsc --noEmit` failed
with TS1479 before this change and passes after it. A parallel ESM
consumer was checked too, to confirm no regression there. The full
jest suite is unaffected: same 183 passing / 14 pre-existing failures
before and after (those 14 are unrelated Windows temp-dir EPERM
issues in the test harness, reproduced identically on unmodified
main).

This branch has not been deployed

No deployments
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.

@libsql/client can't be used from CommonJS modules using TypeScript

1 participant