RubenVerborgh / AsyncIterator

An asynchronous iterator library for advanced object pipelines in JavaScript
https://rubenverborgh.github.io/AsyncIterator/docs/
Other
49 stars 7 forks source link

Fix: explicitly add ESM types #105

Closed termontwouter closed 9 months ago

termontwouter commented 11 months ago

Stumbled upon some unexpected behavior when having AsyncIterator as a (sub)depenency of a TypeScript project with configured to use the Node16, NodeNext or Bundler module resolution mechanisms. This probably did not turn up earlier because most packages in the ecosystem still compile to CommonJS modules, for which TypeScript defaults to the old Node module resolution mechanism.

The issue

TS7016: Could not find a declaration file for module 'asynciterator'. 
'[...]/node_modules/asynciterator/dist/asynciterator.mjs' implicitly has an 'any' type.
There are types at '[...]/node_modules/asynciterator/dist/asynciterator.d.ts', but this result could not be resolved when respecting package.json "exports". The 'asynciterator' library may need to update its package.json or typings.

When checked with AreTheTypesWrong, we get the following report.

❯ npx -p @arethetypeswrong/cli attw --from-npm asynciterator

asynciterator v3.8.1

Build tools:
- typescript@^3.9.5

❌ Import resolved to JavaScript files, but no type declarations were found. https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/UntypedResolution.md

┌───────────────────┬──────────────────────────────┬─────────────────┐
│                   │ "asynciterator/package.json" │ "asynciterator" │
├───────────────────┼──────────────────────────────┼─────────────────┤
│ node10            │ 🟢 (JSON)                    │ 🟢              │
├───────────────────┼──────────────────────────────┼─────────────────┤
│ node16 (from CJS) │ 🟢 (JSON)                    │ 🟢 (CJS)        │
├───────────────────┼──────────────────────────────┼─────────────────┤
│ node16 (from ESM) │ 🟢 (JSON)                    │ ❌ No types     │
├───────────────────┼──────────────────────────────┼─────────────────┤
│ bundler           │ 🟢 (JSON)                    │ ❌ No types     │
└───────────────────┴──────────────────────────────┴─────────────────┘

The problem is explained in the linked page: "In newer TypeScript resolution modes, however, TypeScript understands that an import in Node will resolve to index.mjs, and so it looks for a declaration file named index.d.mts, which doesn’t exist." (For more info, also see the TypeScript Handbook.)

The solution

My first attempt at solving this (993c734b3854c04254df593fa28c6d561aabce27) was to simply mention the same type declarations twice in the exports object of the manifest. While this solved my concrete compilation errors, it still left the following "warning" when running AreTheTypesWrong.

❯ npx -p @arethetypeswrong/cli attw --pack .                

asynciterator v3.8.1

Build tools:
- typescript@^3.9.5

🎭 Import resolved to a CommonJS type declaration file, but an ESM JavaScript file. https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/FalseCJS.md

┌───────────────────┬──────────────────────────────┬────────────────────────┐
│                   │ "asynciterator/package.json" │ "asynciterator"        │
├───────────────────┼──────────────────────────────┼────────────────────────┤
│ node10            │ 🟢 (JSON)                    │ 🟢                     │
├───────────────────┼──────────────────────────────┼────────────────────────┤
│ node16 (from CJS) │ 🟢 (JSON)                    │ 🟢 (CJS)               │
├───────────────────┼──────────────────────────────┼────────────────────────┤
│ node16 (from ESM) │ 🟢 (JSON)                    │ 🎭 Masquerading as CJS │
├───────────────────┼──────────────────────────────┼────────────────────────┤
│ bundler           │ 🟢 (JSON)                    │ 🟢                     │
└───────────────────┴──────────────────────────────┴────────────────────────┘

Again, this new issue is explained in the linked page: "Import resolved to an ESM type declaration file, but a CommonJS JavaScript file."

That page also lists more consequences than the error I encountered: "TypeScript will not allow consumers in CommonJS files to import/require the module without using await import("pkg"), even though that won’t actually be needed at runtime. In Node, an ES module cannot be accessed with require, so TypeScript will report this as an error at compile time. Note that in CommonJS TypeScript files (.cts, .ts with no package.json "type": "module"), top-level import declarations are emitted as require variable declarations (since CommonJS files cannot use import/export in Node), so this problem can occur even when the consumer is using import syntax."

So, as a second attempt (a5a7bc6e6eab65a44a65826c0b232de7a3fed9e4) I actually compiled the type declarations files for both module types separately. This gives the desired result. Tests also still run fine, and on @rubensworks' request I tested actual imports in CommonJS and ECMAScript modules from Node.js v12.22.12 onward.

❯ npx -p @arethetypeswrong/cli attw --pack .                                                 

asynciterator v3.8.1

Build tools:
- typescript@^3.9.5

 No problems found 🌟

┌───────────────────┬──────────────────────────────┬─────────────────┐
│                   │ "asynciterator/package.json" │ "asynciterator" │
├───────────────────┼──────────────────────────────┼─────────────────┤
│ node10            │ 🟢 (JSON)                    │ 🟢              │
├───────────────────┼──────────────────────────────┼─────────────────┤
│ node16 (from CJS) │ 🟢 (JSON)                    │ 🟢 (CJS)        │
├───────────────────┼──────────────────────────────┼─────────────────┤
│ node16 (from ESM) │ 🟢 (JSON)                    │ 🟢 (ESM)        │
├───────────────────┼──────────────────────────────┼─────────────────┤
│ bundler           │ 🟢 (JSON)                    │ 🟢              │
└───────────────────┴──────────────────────────────┴─────────────────┘
RubenVerborgh commented 11 months ago

Can't believe we're 2024 and we still need to jump through all of these hoops just to get a CJS and ESM version. Thanks for taking this on!

I just notice a CI error, could you have a look?

termontwouter commented 11 months ago

That's better :)

RubenVerborgh commented 9 months ago

This looks so painful, @termontwouter. Thanks for looking after this!