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

Add .js extensions to ESM imports #2

Merged
merged 1 commit into from
Jul 1, 2021
Merged

Add .js extensions to ESM imports #2

merged 1 commit into from
Jul 1, 2021

Conversation

amacneil
Copy link
Contributor

@amacneil amacneil commented Jul 1, 2021

ESM in node.js currently requires explicit file extensions: https://nodejs.org/api/esm.html#esm_mandatory_file_extensions

This results in the following webpack error when you try to consume this package as a module:

BREAKING CHANGE: The request './XmlRpcTypes' failed to resolve only because it was resolved as fully specified
(probably because the origin is a '*.mjs' file or a '*.js' file where the package.json contains '"type": "module"').
The extension in the request is mandatory for it to be fully specified.
Add the extension to the request.

Typescript, for better or worse have solved this by allowing TS files to import *.js and silently treating this as .ts instead. Massive thread if you want to get into it: microsoft/TypeScript#16577 (comment)

Jest of course doesn't understand any of this, and gets confused by the missing .js files. ts-jest doesn't solve this automatically, and neither does esbuild-jest. The best solution is a small custom jest resolver which attempts .ts as a fallback for .js imports. See this ts-jest thread for details. Someone published this as a package (jest-ts-webcompat-resolver).

@jtbandes
Copy link
Member

jtbandes commented Jul 1, 2021

Is there any solution that allows us to keep "sane" (intuitive) import statements in our code?

@amacneil
Copy link
Contributor Author

amacneil commented Jul 1, 2021

Explicit file extensions appear to be part of the ESM spec, so not really. It seems to me like typescript should rewrite them (from .ts to .js), but they have taken a stance against doing that.

So, our options are:

  1. Don't use ESM
  2. Run node with --experimental-specifier-resolution=node (I assume webpack, jest, etc would need equivalent configurations): https://nodejs.org/api/esm.html#esm_customizing_esm_specifier_resolution_algorithm

@amacneil
Copy link
Contributor Author

amacneil commented Jul 1, 2021

I'm going to merge, but feel free to continue researching and let me know if I'm missing anything obvious.

Another alternative I didn't mention is to use rollup or similar, and ship a single bundle file rather than a folder full of files. That way consumers don't need to worry about what our import statements look like.

@amacneil amacneil merged commit 628ec47 into main Jul 1, 2021
@amacneil amacneil deleted the adrian/esm branch July 1, 2021 20:28
@jtbandes
Copy link
Member

jtbandes commented Jul 1, 2021

Why do we care about ESM?

@amacneil
Copy link
Contributor Author

amacneil commented Jul 1, 2021

If we ship commonjs then I don't think the package can be easily used in a browser? Also seems like ecosystem is moving that way? Some packages ship both commonjs + esm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants