Node 20 ESM loaders cannot hook into the CommonJS loader

[TomasHubelbauer]

Version

20.1.0

Platform

Darwin Tomass-MBP-2.netis.cc 22.4.0 Darwin Kernel Version 22.4.0: Mon Mar 6 21:00:17 PST 2023; root:xnu-8796.101.5~3/RELEASE_X86_64 x86_64

Subsystem

node --loader

What steps will reproduce the bug?

Follow the steps in my repro here: https://github.com/TomasHubelbauer/node-esm-loader-repro

Copied here for posterity.

Node 20:

1. nvm install 20 to install Node 20
2. node --version to ensure Node version (I get 20.1.0)
3. npm install to install dependencies
4. npm run test to run the health.test.ts script

Notice the test fails and Fastify's autoload is seemingly not aware of the
--loader option and attempts to load routes/health.ts without TypeScript to
JavaScript conversion via ts-node/esm.

npm run test

> repro@0.0.0 test
> node --loader=ts-node/esm --experimental-specifier-resolution=node --test health.test.ts

ℹ (node:95105) ExperimentalWarning: Custom ESM Loaders is an experimental feature and might change at any time
ℹ (Use `node --trace-warnings ...` to show where the warning was created)
✖ should be alive (32.750836ms)
  Error: "@fastify/autoload cannot import plugin at '/routes/health.ts'. To fix this error compile TypeScript to JavaScript or use 'ts-node' to run your app."
      at findPlugins (/node_modules/@fastify/autoload/index.js:224:15)
      at async autoload (/node-esm-loader-repro/node_modules/@fastify/autoload/index.js:35:22)

ℹ tests 1
ℹ suites 0
ℹ pass 0
ℹ fail 1
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 2208.335483

✖ failing tests:

✖ should be alive (32.750836ms)
  Error: "@fastify/autoload cannot import plugin at 'routes/health.ts'. To fix this error compile TypeScript to JavaScript or use 'ts-node' to run your app."
      at findPlugins (/node_modules/@fastify/autoload/index.js:224:15)
      at async autoload (/node_modules/@fastify/autoload/index.js:35:22)

Node 19:

1. nvm install 19 to install Node 20
2. node --version to ensure Node version (I get 19.9.0)
3. npm install to install dependencies
4. npm run test to run the health.test.ts script

Notice the test passes and Fastify's autoload is inherit the --loader option
and uses the ts-node/esm loader successfully to auto-load routes/health.ts.

npm run test

> repro@0.0.0 test
> node --loader=ts-node/esm --experimental-specifier-resolution=node --test health.test.ts

ℹ (node:95453) ExperimentalWarning: Custom ESM Loaders is an experimental feature and might change at any time
ℹ (Use `node --trace-warnings ...` to show where the warning was created)
✔ should be alive (472.711395ms)
ℹ tests 1
ℹ suites 0
ℹ pass 1
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 2679.742161

How often does it reproduce? Is there a required condition?

Node 20: every time
Node 10: never

What is the expected behavior? Why is that the expected behavior?

Follow Node 19's behavior of using --loader to transpile TypeScript to JavaScript on the fly using ts-node/esm.

What do you see instead?

An error that is the result of the loader not being applied.

Additional information

No response

[MoLow]

@TomasHubelbauer does it make a difference if you run with or without --test?

[TomasHubelbauer]

No difference, see this branch: https://github.com/TomasHubelbauer/node-esm-loader-repro/tree/without-test
The readme there contains the error messages I get when dropping --test and changing the script to be top-level code instead of an it.

[targos]

This was reported last week by @isaacs on Slack.

The problem is that ts-node/esm also hooks into the CommonJS loader (require.extensions), but since #44710, it has no effect because it doesn't happen on the main user thread.

[targos]

/cc @cspotcode @nodejs/loaders

[isaacs]

This may be related to #47747. It is clear that the "off-thread loaders" feature seems to have introduced some significant instability, and in my opinion should probably be rolled back and put off until the next major, or at least until there is a straightforward way to allow loaders to either (a) modify require.extensions in such a way that the loaded modules see the modifications, or (b) return transpiled source for "type": "commonjs" modules. If the approach is (b), then that is still a significant breaking change that should get the full cycle of play testing in an unstable v21 release cycle before going live. If the approach is (a), then I think that would solve at least the ts-node/esm problems, which is definitely the most acute community impact.

My preference would be to roll back off-thread loaders in v20, and asap in v21, provide the capability for loaders to return transpiled source for commonjs modules. That would give existing transpilers plenty of time to migrate to the new approach, and also unify the esm/cjs story considerably, and provide a path to finally fully deprecating require.extensions.

[JakobJingleheimer]

I don't foresee a rollback to accommodate a small subset of use-cases that happens to affect you. I do think enabling loaders to supply source for commonjs is a near-future priority (which I intend to champion). Enabling loaders to supply source for commonjs is most definitely not a breaking change: It was very explicitly not supported—if a user happened to erroneously try and node didn't blow up in their face for doing what was specifically detailed as not supported, congratulations to whoever for foot-gunning themselves.

This change was 7 months in the making, and was well publicised.

I am very content to facilitate and support transition. If an author needs time, inform me/us. To my knowledge, we received no requests for such, and during the time, several packages were updated.

I believe the issue here is a known bug which has a simple workaround. The bug as I understand it should indeed be addressed, and we plan to do.

As a side note, I am personally very offended by this cavalier and callous suggestion. I made every effort to forewarn and collaborate with package authors (I specifically tagged major packages), and in several cases, made the compatibility updates for them.

We had discussed this "privately", and I suggested the resolution of enabling loaders supplying commonjs source, so I do not appreciate this public sucker-punch.

[GeoffreyBooth]

This may be related to #47747. It is clear that the “off-thread loaders” feature seems to have introduced some significant instability, and in my opinion should probably be rolled back and put off until the next major

No. The Loaders API is experimental. Breaking changes are expected.

The intention is to add support for the load hook to return source for type: "commonjs". That’s listed here: https://github.com/nodejs/loaders#milestone-2-stability. That’s one of the few remaining items before we consider the API to be stable.

The team working on this part of the codebase has very limited resources. If you’d like to help, your contribution would be most welcome.

[GeoffreyBooth]

The problem is that ts-node/esm also hooks into the CommonJS loader (require.extensions), but since #44710, it has no effect because it doesn’t happen on the main user thread.

An immediate solution is to run the require.extensions registration on the main thread in addition to registering the loader. Something like --import ts-node/register --loader ts-node/esm where ts-node/register includes the require.extensions logic that’s currently bundled into the loader.