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 loader hooks in Workers no longer working since Node.js 22.2 #53097

Closed
alan-agius4 opened this issue May 22, 2024 · 7 comments
Closed

ESM loader hooks in Workers no longer working since Node.js 22.2 #53097

alan-agius4 opened this issue May 22, 2024 · 7 comments
Labels
loaders Issues and PRs related to ES module loaders

Comments

@alan-agius4
Copy link
Contributor

alan-agius4 commented May 22, 2024

Version

v22.2.0

Platform

Linux 6.6.15-2rodete2-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.6.15-2rodete2 (2024-03-19) x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

Since Node.js version 22.2, ESM loader hooks no longer function inside of a worker and the Node.js application becomes unresponsive.

app.js

const { Worker, isMainThread } = require("node:worker_threads");

if (!isMainThread) {
  import("./test.mjs").then(console.log).catch(console.error);
} else {
  new Worker("./app.js", {
    execArgv: [
      '--import=data:text/javascript,import { register } from "node:module"; import { pathToFileURL } from "node:url"; register("./hooks.mjs", pathToFileURL("./"), { data: {} });',
    ],
  });
}

hooks.mjs

export async function initialize() {
  console.log('init')
}

export async function resolve(specifier, context, nextResolve) {
  console.log('resolve: ' + specifier)
  return nextResolve(specifier, context)
}
$ node app.js

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

Always

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

init
Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/test-loader-esm/test.mjs' imported from ///test-loader-esm/app.js
    at finalizeResolution (node:internal/modules/esm/resolve:264:11)
    at moduleResolve (node:internal/modules/esm/resolve:924:10)
    at defaultResolve (node:internal/modules/esm/resolve:1148:11)
    at nextResolve (node:internal/modules/esm/hooks:750:28)
    at resolve (/test-loader-esm/hooks.mjs:8:10)
    at nextResolve (node:internal/modules/esm/hooks:750:28)
    at Hooks.resolve (node:internal/modules/esm/hooks:238:30)
    at handleMessage (node:internal/modules/esm/worker:199:24)
    at Immediate.checkForMessages (node:internal/modules/esm/worker:141:28)
    at process.processImmediate (node:internal/timers:478:21) {
  code: 'ERR_MODULE_NOT_FOUND',
  url: '/test-loader-esm/test.mjs'
}
resolve: ./test.mjs

What do you see instead?

Application becomes unresponsive.

Additional information

I suspect that this is caused by #52706

@alan-agius4 alan-agius4 changed the title ESM loader hooks no longer working since Node.js 22.2 in Workers ESM loader hooks in Workers no longer working since Node.js 22.2 May 22, 2024
@joshrules007

This comment was marked as off-topic.

@aduh95
Copy link
Contributor

aduh95 commented May 22, 2024

/cc @nodejs/loaders @dygabo

@aduh95 aduh95 added the loaders Issues and PRs related to ES module loaders label May 22, 2024
@dygabo
Copy link
Member

dygabo commented May 22, 2024

this behaviour changed indeed with #52706. Passing execArgv to new Worker is not supported. Please see this PR as a trial to document the behaviour.

Please add the customization hook to the main thread and this will affect all created workers as described here.

app.js will become:

const { Worker, isMainThread } = require("node:worker_threads");

if (!isMainThread) {
  import("./test.mjs").then(console.log).catch(console.error);
} else {
  new Worker("./app.js");
}

hooks.mjs will stay the same.

reg.mjs will be:

import { register } from "node:module";
import { pathToFileURL } from "node:url";
register("./hooks.mjs", pathToFileURL("./"));

and the application start will be:
node --import=./reg.mjs app.js

the customization hooks chain will be as mentioned the same on all threads and the output will be something like:

init
resolve: file:///test-loader-esm/app.js
resolve: file:///test-loader-esm/app.js
resolve: ./test.mjs
Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/test-loader-esm/test.mjs' imported from /test-loader-esm/app.js
    at finalizeResolution (node:internal/modules/esm/resolve:260:11)
    at moduleResolve (node:internal/modules/esm/resolve:920:10)
    at defaultResolve (node:internal/modules/esm/resolve:1119:11)
    at nextResolve (node:internal/modules/esm/hooks:791:28)
    at resolve (file:///test-loader-esm/hooks.mjs:7:10)
    at nextResolve (node:internal/modules/esm/hooks:791:28)
    at Hooks.resolve (node:internal/modules/esm/hooks:238:30)
    at MessagePort.handleMessage (node:internal/modules/esm/worker:255:24)
    at [nodejs.internal.kHybridDispatch] (node:internal/event_target:816:20)
    at MessagePort.<anonymous> (node:internal/per_context/messageport:23:28) {
  code: 'ERR_MODULE_NOT_FOUND',
  url: 'file:///test-loader-esm/test.mjs'
}

@Flarna

This comment was marked as outdated.

@alan-agius4
Copy link
Contributor Author

alan-agius4 commented May 22, 2024

In all fairness, this seems like a breaking change, especially since this feature is in the Release Candidate stage. I wouldn't expect such a drastic change in behavior, particularly in a minor release.

For us, the Angular CLI, this is quite a breaking change because in some of our workers, we want different resolutions, which differs from the main thread and that of other workers.

We could surely do some workarounds, but it was very convenient that workers supported custom ESM hooks separate from the main thread.

If this is the intended and desired behavior, IMHO, it should be implemented in a major release to comply with semantic versioning.

@GeoffreyBooth
Copy link
Member

#52706 fixed a longstanding bug: #50752. We knew that this fix might surprise or break people, which is why we were waiting to fix this before making the API stable (and we still haven’t, in case any further issues arise). This is the point of the Release Candidate stage: to identify and fix issues like this.

I’m sorry that this change has inconvenienced you, but this is why we have experimental features and why they need to be allowed to change. If we couldn’t fix this bug until October with Node 23, and then we couldn’t backport the fix to the LTS lines, it would be April 2025 before a stable hooks API would be available on an LTS line. Many users are eager for this API to become stable so that they can rely on it, and stretching it out over years inconveniences them for the dubious benefit of protecting early adopters who knew what they were getting into by relying on an experimental API. We prioritize the larger group of users seeking stability over the early adopters.

alan-agius4 added a commit to alan-agius4/angular-cli that referenced this issue May 27, 2024
… Node.js 22.2.0 and later

Node.js 22.2.0 introduced a breaking change affecting custom ESM resolution.

For more context, see: [Node.js issue #53097](nodejs/node#53097)

Closes: #53097
alan-agius4 added a commit to alan-agius4/angular-cli that referenced this issue May 27, 2024
This update ensures that we can test prerendering with the ESM loader breaking change. For more details, see: nodejs/node#53097
alan-agius4 added a commit to alan-agius4/angular-cli that referenced this issue May 27, 2024
… Node.js 22.2.0 and later

Node.js 22.2.0 introduced a breaking change affecting custom ESM resolution.

For more context, see: [Node.js issue #53097](nodejs/node#53097)

Closes: #53097
alan-agius4 added a commit to alan-agius4/angular-cli that referenced this issue May 27, 2024
This update ensures that we can test prerendering with the ESM loader breaking change. For more details, see: nodejs/node#53097
alan-agius4 added a commit to alan-agius4/angular-cli that referenced this issue May 27, 2024
… Node.js 22.2.0 and later

Node.js 22.2.0 introduced a breaking change affecting custom ESM resolution.

For more context, see: [Node.js issue #53097](nodejs/node#53097)

Closes: #53097
alan-agius4 added a commit to alan-agius4/angular-cli that referenced this issue May 27, 2024
This update ensures that we can test prerendering with the ESM loader breaking change. For more details, see: nodejs/node#53097
alan-agius4 added a commit to alan-agius4/angular-cli that referenced this issue May 27, 2024
… Node.js 22.2.0 and later

Node.js 22.2.0 introduced a breaking change affecting custom ESM resolution.

For more context, see: [Node.js issue #53097](nodejs/node#53097)

Closes: #53097
alan-agius4 added a commit to alan-agius4/angular-cli that referenced this issue May 27, 2024
This update ensures that we can test prerendering with the ESM loader breaking change. For more details, see: nodejs/node#53097
alan-agius4 added a commit to alan-agius4/angular-cli that referenced this issue May 27, 2024
… Node.js 22.2.0 and later

Node.js 22.2.0 introduced a breaking change affecting custom ESM resolution.

For more context, see: [Node.js issue #53097](nodejs/node#53097)

Closes: #53097
alan-agius4 added a commit to alan-agius4/angular-cli that referenced this issue May 27, 2024
This update ensures that we can test prerendering with the ESM loader breaking change. For more details, see: nodejs/node#53097
alan-agius4 added a commit to alan-agius4/angular-cli that referenced this issue May 27, 2024
… Node.js 22.2.0 and later

Node.js 22.2.0 introduced a breaking change affecting custom ESM resolution.

For more context, see: [Node.js issue #53097](nodejs/node#53097)

Closes: #53097
alan-agius4 added a commit to alan-agius4/angular-cli that referenced this issue May 27, 2024
This update ensures that we can test prerendering with the ESM loader breaking change. For more details, see: nodejs/node#53097
alan-agius4 added a commit to alan-agius4/angular-cli that referenced this issue May 27, 2024
… Node.js 22.2.0 and later

Node.js 22.2.0 introduced a breaking change affecting custom ESM resolution.

For more context, see: [Node.js issue #53097](nodejs/node#53097)

Closes: #53097
alan-agius4 added a commit to alan-agius4/angular-cli that referenced this issue May 27, 2024
This update ensures that we can test prerendering with the ESM loader breaking change. For more details, see: nodejs/node#53097
alan-agius4 added a commit to alan-agius4/angular-cli that referenced this issue May 27, 2024
… Node.js 22.2.0 and later

Node.js 22.2.0 introduced a breaking change affecting custom ESM resolution.

For more context, see: [Node.js issue #53097](nodejs/node#53097)

Closes: #53097
alan-agius4 added a commit to alan-agius4/angular-cli that referenced this issue May 27, 2024
This update ensures that we can test prerendering with the ESM loader breaking change. For more details, see: nodejs/node#53097
@aduh95
Copy link
Contributor

aduh95 commented May 28, 2024

FYI there are discussions for reverting that change. The discussion about that can continue in the other issue, marking this one as duplciate.

Duplicate of #53182

@aduh95 aduh95 closed this as not planned Won't fix, can't repro, duplicate, stale May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
loaders Issues and PRs related to ES module loaders
Projects
None yet
Development

No branches or pull requests

6 participants