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

ts-node/register not supported in 9.0.0 #4665

Closed
4 tasks done
danielgindi opened this issue Jun 23, 2021 · 14 comments · Fixed by #4668
Closed
4 tasks done

ts-node/register not supported in 9.0.0 #4665

danielgindi opened this issue Jun 23, 2021 · 14 comments · Fixed by #4668
Assignees
Labels
area: node.js command-line-or-Node.js-specific type: bug a defect, confirmed by a maintainer

Comments

@danielgindi
Copy link

Prerequisites

  • Checked that your issue hasn't already been filed by cross-referencing issues with the faq label
  • Checked next-gen ES issues and syntax problems by using the same environment and/or transpiler configuration without Mocha to ensure it isn't just a feature that actually isn't supported in the environment in question or a bug in your code.
  • 'Smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, your usage of Mocha, or Mocha itself
  • Ensured that there is no discrepancy between the locally and globally installed versions of Mocha. You can find them with: node node_modules/.bin/mocha --version(Local) and mocha --version(Global). We recommend that you not install Mocha globally.

Description

mocha configuration contains require: ['ts-node/register'], in order to load the TS loader.
But since node_modules/ts-node/register is a folder, the new ESM loader in mocha fails to load, and throws:
Error [ERR_UNSUPPORTED_DIR_IMPORT]: Directory import '...../node_modules/ts-node/register' is not supported resolving ES modules imported from ...../node_modules/mocha/lib/esm-utils.js

Steps to Reproduce

  1. npm i --save-dev ts-node
  2. Add require: ['ts-node/register'] to the .mocharc.yml

Expected behavior: Should work, and at least automatically resolve some well-known loaders like ts-node.

Actual behavior: Fails miserably.

Reproduces how often: 100%

Versions

  • The output of mocha --version and node node_modules/.bin/mocha --version: 9.0.0
  • The output of node --version: v15.12.0
  • Your operating system
    • name and version: MacOS 11.12.3
    • architecture (32 or 64-bit): M1
  • Your shell (e.g., bash, zsh, PowerShell, cmd): bash
  • Any code transpiler (e.g., TypeScript, CoffeeScript, Babel) being used (and its version): ts-node 9.1.1
@giltayar
Copy link
Contributor

Will look into it tonight. Hopefully, just require-ing when we get ERR_UNSUPPORTED_DIR_IMPORT will solve the problem.

@giltayar
Copy link
Contributor

Oh, wait, I see your fix @swansontec. Looks like it will solve it. Thanks!

@juergba juergba linked a pull request Jun 24, 2021 that will close this issue
@juergba
Copy link
Contributor

juergba commented Jun 24, 2021

I'm surprised that this issue is arising only two weeks after publishing v9.0.0. Is nobody using typescript with ts-node?
You are using node v15.12.0 which is end-of-life already. Is v14/v16 also failing?

@giltayar
Copy link
Contributor

@swantontec, I tried reproducing it, but it works for me:

// x.test.ts
function foo(x: number) {
  console.log('hi')
}

foo(4)
# .mocharc.yaml
require: ['ts-node/register']
$ npm init -y
$ npm i -D ts-node mocha
$ npx mocha x.test.ts
hi


  0 passing (0ms)

I tried both for Node.js v15.12.0, and v16.3.0. Mocha was v9.0.1, and ts-node was 10.0.0 and 9.1.1.

If you could create a repository that reproduces this (including full instructions to reproduce), that would maybe pinpoint the problem?

@juergba
Copy link
Contributor

juergba commented Jun 24, 2021

@danielgindi @swansontec are you requiring the esm package?

@danielgindi
Copy link
Author

@danielgindi @swansontec are you requiring the esm package?

Mot specifically. Simply require: ['ts-node/register']

@swansontec
Copy link

swansontec commented Jun 24, 2021

Aha! This only affects old versions of ts-node. Modern versions, such as the current one, have an exports field in their package.json file, which solves the problem another way.

However, the latest version of Sucrase (3.19.0) still has the issue. Here is a minimal reproduction script:

mkdir mocha-demo; cd mocha-demo
yarn add --dev mocha sucrase
echo "console.log('Hey')" > test.ts
yarn mocha -r sucrase/register test.ts

So, this does open up the possibility of patching Sucrase to use exports, instead of fixing Mocha itself. Perhaps both would be worthwhile.

@danielgindi
Copy link
Author

Aha! This only affects old versions of ts-node. Modern versions, such as the current one, have an exports field in their package.json file, which solves the problem another way.

Wait. What would be the correct wait to register ts-node with mocha?

@swansontec
Copy link

swansontec commented Jun 24, 2021

Aha! This only affects old versions of ts-node. Modern versions, such as the current one, have an exports field in their package.json file, which solves the problem another way.

Wait. What would be the correct wait to register ts-node with mocha?

It works the same as before - you just add -r ts-node/require, or put require: ['ts-node/register'] into a Mocha config file.

The problem is that import() does not know how to search the filesystem the way require() does. The only way for import('ts-node/register') to succeed is if the library provides an explicit subpath export in package.json. Recent versions of ts-node do have the right subpath exports, so import('ts-node/register') succeeds!

However, other packages like sucrase do not provide subpath exports in their package.json file, which means import('sucrase/register') will fail. Falling back onto require('sucrase/register') will work fine, because require will search the filesystem as well. This which is what #4666 achieves - enabling the require fallback logic for this case.

@danielgindi
Copy link
Author

Got it. But for commonjs, if there's no package.json, the default of course is index.js.
And commonjs is the base module system for nodejs and it does not make sense to remove support for it :)
Well, there's a PR for it so all good!

@giltayar
Copy link
Contributor

@swansontec got it. I'll try and reproduce it without specifically using ts-node. Thanks for the detailed explanation.

@giltayar
Copy link
Contributor

@swansontec yup. got it. Fixing...

@giltayar
Copy link
Contributor

Hey, @swansontec, I just saw you have a PR for this. After I made mine... 😂

Unfortunately, your PR doesn't include a test. If you'd rather you add a test to your PR, we can use yours. Otherwise, I'll add mine.

@swansontec
Copy link

@giltayar We should go ahead and use your PR! I will close mine.

@juergba juergba added type: bug a defect, confirmed by a maintainer area: node.js command-line-or-Node.js-specific and removed unconfirmed-bug labels Jun 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: node.js command-line-or-Node.js-specific type: bug a defect, confirmed by a maintainer
Projects
None yet
4 participants