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: Fix importing index.js files by folder name #4666

Closed
wants to merge 1 commit into from
Closed

ESM: Fix importing index.js files by folder name #4666

wants to merge 1 commit into from

Conversation

swansontec
Copy link

@swansontec swansontec commented Jun 23, 2021

Description of the Change

This is a fix for #4665, where running with -r ts-node/register or similar flags doesn't work.

The reason this fails is because Node cannot resolve node_modules/ts-node/register/index.js as an ESM module. The returned error has a code of 'ERR_UNSUPPORTED_DIR_IMPORT', which should trigger the require fall-back logic, but it doesn't because that error code is not on the list.

Alternate Designs

Mocha v8 used a much simpler try / catch approach that worked reliably. Mocha v9 tries to be more picky, which is good, but we need to include all the edge cases for this to work smoothly.

Why should this be in core?

It's a bug-fix for a pretty serious v9 regression.

Benefits

mocha -r ts-node/register works again! Or mocha -r sucrase/register, if you prefer that one.

Possible Drawbacks

I can't think of any - it's a pretty obvious fix.

Applicable issues

#4665

This is a minor patch fix for a pretty common issue, so it would be good to release as v9.0.1 -> v9.0.2.

As pointed out in #4665, running mocha with an argument such as `-r ts-node/register` fails because Node cannot resolve `node_modules/ts-node/register/index.js` as an ESM module. The returned error has a `code` of `'ERR_UNSUPPORTED_DIR_IMPORT'`, which should trigger the `require` fall-back logic.
@juergba juergba linked an issue Jun 24, 2021 that may be closed by this pull request
4 tasks
@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.369% when pulling b22921f on swansontec:patch-1 into f033ff1 on mochajs:master.

@swansontec
Copy link
Author

#4668 makes the same fix, but includes unit-tests as well, so that one is definitely better than mine.

@swansontec swansontec closed this Jun 25, 2021
@mochajs mochajs deleted a comment from jdaykin454 Jul 4, 2021
@swansontec swansontec deleted the patch-1 branch July 5, 2021 17:02
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.

ts-node/register not supported in 9.0.0
2 participants