ESM loaders cannot be defined via `Worker` option `execArgv` in v20

[cjihrig]

Version

20.0.0 (tested on main at 36e4e3d too)

Platform

macOS but probably all platforms

Subsystem

esm

What steps will reproduce the bug?

The following works in Node 18 and 19, but not 20:

// main.mjs
import { Worker } from 'node:worker_threads';

new Worker('./worker.js', {
  execArgv: ['--experimental-loader', './loader.mjs'],
});

// worker.js
'use strict';

async function main() {
  await import('node:fs');
}

main();

// loader.mjs
export function resolve (specifier, context, nextResolve) {
  throw new Error('boom');
}

Run: node main.mjs. In Node 18 and 19, the exception in the loader is thrown. In Node 20 it is not. The problem is not unique to throwing. I haven't been able to see console.log() or any other indication that the loader is being called.

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

100% of the time for me.

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

I expect it to work as it did in earlier versions of Node. This is the expected behavior because right now it doesn't seem to work at all.

What do you see instead?

See the description of the bug above.

Additional information

Just a guess, but I'm assuming this is related to #44710.

[isaacs]

Another side effect of ES loaders to a separate thread: updating require.extensions in an es loader no longer has any effect, meaning that node --loader=ts-node/esm no longer functions properly to load both cjs and mjs typescript.

This makes it significantly more challenging to have loaders that work for both esm and cjs, effectively breaking the esm on-ramp for those of us supporting both modes, since now you need to know ahead of time what the module type is (or write the transpiled source to another path and return a short circuit url to that instead), rather than being able to have a single loader capable of handling both.

[isaacs]

Verified that node --loader=ts-node/esm foo.ts stopped working on 4667b07

[cjihrig]

cc: @nodejs/loaders

[JakobJingleheimer]

As stated in the ESM doc, async activity like console.log (which is specifically cited in the caveat) is not guaranteed to run.

Please try other methods, such as fs.writeSync to confirm.

I believe this is not a dupe of #47615 because it specifically provides execArgv, thus avoiding the fork-bomb.

[cjihrig]

@JakobJingleheimer I've tried fs.writeSync() and process._rawDebug() with no luck.

[mcollina]

I’m not sure how would it work in the current design. There is (by design) one loader process for the whole Node.js process. In the example above, we are asking Node.js to start another one just for that specific thread.

We need a new API to handle this case, something that:

1. allows us to create a "loaders worker", potentially exposing what we are already doing in core
2. pass that loader down to new Worker(path, { loader } ), so that the loader thread could be reused.

[cjihrig]

We need a new API to handle this case

What you are proposing seems like an explicit API. It seems like --experimental-loader passed to the Worker constructor should implicitly do that (without needing to expose more public APIs).

[targos]

It's interesting because it doesn't work --loader in the worker's execArgv, but it works fine if --loader is passed to the main script (the worker inherits the flag).

[GeoffreyBooth]

It’s interesting because it doesn’t work --loader in the worker’s execArgv, but it works fine if --loader is passed to the main script (the worker inherits the flag).

I don’t think of worker threads like child processes that can have their own Node flags. I feel like they should inherit whatever the parent process has, which is what @targos is describing here.

[cjihrig]

I don’t think of worker threads like child processes that can have their own Node flags.

That is the exact purpose of the execArgv option to the Worker constructor. Some flags are not supported there, but, for example, --require does appear to be. And the loaders docs do claim that they follow the pattern of --require

[GeoffreyBooth]

the loaders docs do claim that they follow the pattern of --require

Only in the sense that the way that chaining works for multiple invocations of --require is the same way that chaining works for multiple invocations of --loader. There’s not much else that loaders share in common with require.

Unless there’s some argument for why this should be considered a bug, I think we should just update the docs for https://nodejs.org/api/worker_threads.html#new-workerfilename-options to clarify that --loader is one of the flags not supported by that option.