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 types ESM module resolution #170

Merged
merged 2 commits into from
Sep 3, 2023
Merged

Conversation

Methuselah96
Copy link
Contributor

@Methuselah96 Methuselah96 commented Jul 6, 2023

"Are the types wrong?" reveals that the ESM modules in this package are masquerading as CJS from a type perspective (https://arethetypeswrong.github.io/?p=fflate%400.8.0):

image

This PR fixes the situation by creating a separate declaration file per module as recommended in the TypeScript documentation:

It’s important to note that the CommonJS entrypoint and the ES module entrypoint each needs its own declaration file, even if the contents are the same between them. Every declaration file is interpreted either as a CommonJS module or as an ES module, based on its file extension and the "type" field of the package.json, and this detected module kind must match the module kind that Node will detect for the corresponding JavaScript file for type checking to be correct. Attempting to use a single .d.ts file to type both an ES module entrypoint and a CommonJS entrypoint will cause TypeScript to think only one of those entrypoints exists, causing compiler errors for users of the package.

Creating a declaration file per module fixes the type module resolution and fixes the "Are the types wrong?" tests:

image

The resulting built files now all specify whether they are CJS or MJS with the corresponding file extension and each module now has a corresponding declaration file which absolves the need to explicitly list them in package.json#exports:

image

@101arrowz
Copy link
Owner

This PR seems good but I remember avoiding the .mjs file extension for a reason - I'll have to look into why before merging.

@101arrowz
Copy link
Owner

101arrowz commented Jul 23, 2023

I think I made this change to avoid errors when directly importing something like fflate/esm/browser without an extension in some environments. That was probably more necessary a few years ago than today, but I'd still like to support that behavior for backwards compat. Also, .mjs gives a bad MIME type in some static file servers, which could be an issue for people who serve fflate manually.

I think it's possible to fix the types here without switching the file extensions; I'll try to modify this PR to do so.

@101arrowz 101arrowz merged commit c36d658 into 101arrowz:master Sep 3, 2023
@Methuselah96 Methuselah96 deleted the fix-types branch September 3, 2023 18:18
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.

None yet

2 participants