Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate separate type declarations for CommonJS. #11

Merged
merged 1 commit into from
Apr 18, 2024

Conversation

djcsdy
Copy link
Contributor

@djcsdy djcsdy commented Nov 25, 2023

Although these are identical to the type declarations for ESM, it is necessary to generate separate type declarations for CommonJS for compatibility with TypeScript's Node16/NodeNext module resolution modes.

See microsoft/TypeScript#56529

See https://arethetypeswrong.github.io/?p=unknown%400.2.5

See https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/FalseESM.md

With this change, TypeScript will resolve the types as follows in Node16/NodeNext modes:

  • When importing using require (CommonJS), match "exports": ".": "require": "./dist/cjs/unknown.js", and resolve types to the accompanying "./dist/cjs/unknown.d.ts".
  • Otherwise, match "exports": ".": "default": "./dist/unknown.js", and resolve types to the accompanying "./dist/unknown.d.ts".

Note that it is not necessary to specify "types" in "exports" because TypeScript will follow the node module resolution algorithm to resolve the JavaScript file that would be imported, and then automatically discover type definitions adjacent to that JavaScript file.

The top-level "types" entry is probably not required for the same reason, but I have left it alone for now for the sake of making a minimal change to solve this problem.

Without this change, when importing unknown from a CommonJS project with Node16/NodeNext module resolution, TypeScript reports:

index.ts:1:27 - error TS1479: The current file is a CommonJS module whose
imports will produce 'require' calls; however, the referenced file is an
ECMAScript module and cannot be imported with 'require'. Consider writing a
dynamic 'import("unknown")' call instead.

Although these are identical to the type declarations
for ESM, it is necessary to generate separate type
declarations for CommonJS for compatibility with
TypeScript's Node16/NodeNext module resolution modes.

See microsoft/TypeScript#56529

See https://arethetypeswrong.github.io/?p=unknown%400.2.5

See https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/FalseESM.md
@djcsdy
Copy link
Contributor Author

djcsdy commented Apr 18, 2024

Hi @BTOdell. Just checking in to see if there is any chance of getting this merged. I realise you are probably very busy :-).

Let me know if there's anything I can do to help.

@BTOdell
Copy link
Owner

BTOdell commented Apr 18, 2024

Sorry that I let this slip by and thanks for bringing it to my attention again.

I'll merge this and spin a new release later today.

@BTOdell BTOdell merged commit 34df450 into BTOdell:master Apr 18, 2024
6 checks passed
@BTOdell
Copy link
Owner

BTOdell commented Apr 18, 2024

https://www.npmjs.com/package/unknown/v/0.2.6

Please confirm whether your issue is fixed with this version.
Thanks!

@djcsdy
Copy link
Contributor Author

djcsdy commented Apr 19, 2024

Confirmed fixed in v0.2.6.

https://arethetypeswrong.github.io/?p=unknown%400.2.6

Thanks!

@djcsdy djcsdy deleted the cjs-types branch April 19, 2024 18:05
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.

2 participants