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

Fix TypeScript types with Node16/NodeNext module resolution #21270

Merged
merged 2 commits into from
Dec 1, 2023

Conversation

Gerrit0
Copy link
Contributor

@Gerrit0 Gerrit0 commented Nov 18, 2023

Summary

TypeScript with the Node16 or NodeNext setting for "module" in tsconfig.json will read the package exports map. This adds types entries to package.json so that it can be used in those settings. The types key is left in place so that the package types still work for users with "module": "CommonJS".

TypeScript requires different declaration files (even though the same content works for both) so that it can distinguish between import/require. For clarity, the build script now writes to require.d.ts (not .d.cts for compatibility with old TS versions) and import.d.mjs

Test results and supporting details

Tested by adding the build directory as a dependency, and checking:

Config main main types forLegacyNode forLegacyNode types
.mts file with NodeNext
.cjs file with NodeNext ✅ (require is error, can be dynamically imported) ✅ (types can be imported)
.ts file with CommonJS using import bcd from transpiled into require ❌ (no support for exports, would have to rename files, anyone who runs into this should probably be using Node16/NodeNext anyways)
.ts file with CommonJS using import x = require("@mdn/browser-compat-data") ❌ (ts thinks the value is under .default, not many TS users even know this is a feature, don't think this matters)

Related issues

Fixes #17964

@github-actions github-actions bot added the scripts Issues or pull requests regarding the scripts in scripts/. label Nov 18, 2023
@Elchi3 Elchi3 requested a review from queengooborg November 20, 2023 12:58
Copy link
Contributor

@queengooborg queengooborg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, let's get this landed, thank you for your PR! Welcome to BCD!

@queengooborg queengooborg merged commit 9ff1567 into mdn:main Dec 1, 2023
4 checks passed
@Gerrit0 Gerrit0 deleted the fix-ts-types branch June 22, 2024 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scripts Issues or pull requests regarding the scripts in scripts/.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Problem using from TypeScript as module
2 participants