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

ESM considerations for ts-node and other require.extensions hooks #4274

Closed
Tracked by #2
cspotcode opened this issue May 8, 2020 · 3 comments
Closed
Tracked by #2

ESM considerations for ts-node and other require.extensions hooks #4274

cspotcode opened this issue May 8, 2020 · 3 comments
Labels
stale this has been inactive for a while... type: discussion debates, philosophy, navel-gazing, etc. type: question support question

Comments

@cspotcode
Copy link
Contributor

I want to share a couple things I've hit while adding ESM support to ts-node, because I think it affects design decisions for mocha's ESM support.

require.extensions need to add hacks to classify files as either CJS or ESM

@emma-borhanian hit an issue with babel's require.extensions hook, which also happens to ts-node
#4038 (comment)

I've shared the fix I'm going to add to ts-node; babel could do something similar.
nodejs/modules#351 (comment)

With this fix we will throw the same errors as node when you try to require() an ESM file.

Mocha needs to consult require.extensions to properly load non-standard file extensions

In mocha you have a comment about using import() for all files in the future, letting it classify files as CJS or ESM. Unfortunately, I believe this will fail to resolve non-standard file extensions such as .ts.

In ts-node we have implemented an ESM loader that does understand .ts, .tsx, and .jsx. However, this requires users to invoke node or mocha like this:

node --loader ts-node/esm.mjs ./node_modules/mocha/bin/mocha --extension ts

Users who are not using ESM will not install this loader, so the ESM resolver will not be able to resolve nor classify .ts, .tsx, or .jsx files. For this, mocha must continue using require to locate and attempt loading the file. I believe that even import(require.resolve('./ts-file.ts')) will fail to classify the file as CJS or ESM if our custom ESM hook is not installed.

See also: nodejs/node#33226
It's possible that node's built-in loader will be changed to classify unrecognized extensions based on package.json "type" field, but I'm not sure.


I hope this information is helpful!

@cspotcode cspotcode added the type: question support question label May 8, 2020
@boneskull
Copy link
Contributor

Thanks!

I'll admit I don't have enough experience with TS, ts-node, nor loaders to understand what, if anything, is actionable here.

I think we'll want to add some tests for whatever we do, though. There's precedent for this in our test suites, so we can pull in ts-node or whatever is necessary to test ... whatever it is we need to test, exactly.

@boneskull boneskull added the type: discussion debates, philosophy, navel-gazing, etc. label May 18, 2020
@cspotcode
Copy link
Contributor Author

The only actionable thing is:

  • don't use import() to load all test files. It will fail to discover files with non-standard file extensions. The only action, if any, is to update the code comments to explain why import() fails to consider require.extensions

Also, I mentioned a hack we were going to use to classify files as either CJS or ESM. Turns out that hack won't work reliably. Instead, we've had to copy-paste some of node's source code where it discovers the nearest package.json and checks for a "type" field.
https://github.com/TypeStrong/ts-node/blob/7a1a55bc69bc35b665795c8a49b20131a3cb5fc0/dist-raw/node-cjs-loader-utils.js#L10-L20
Every time our require hook is invoked, we call assertScriptCanLoadAsCJSImpl. If appropriate, it throws a ERR_REQUIRE_ESM lookalike error. We do this so that you can still detect it

if (err.code === 'ERR_REQUIRE_ESM') {

@stale
Copy link

stale bot commented Sep 20, 2020

I am a bot that watches issues for inactivity.
This issue hasn't had any recent activity, and I'm labeling it stale. In 14 days, if there are no further comments or activity, I will close this issue.
Thanks for contributing to Mocha!

@stale stale bot added the stale this has been inactive for a while... label Sep 20, 2020
@stale stale bot closed this as completed Oct 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale this has been inactive for a while... type: discussion debates, philosophy, navel-gazing, etc. type: question support question
Projects
None yet
Development

No branches or pull requests

2 participants