Skip to content

Commit 4667b07

Browse files
JakobJingleheimeraduh95GeoffreyBoothtargos
authored
esm: move hook execution to separate thread
PR-URL: #44710 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com> Co-authored-by: Geoffrey Booth <webadmin@geoffreybooth.com> Co-authored-by: Michaël Zasso <targos@protonmail.com>
1 parent 5273947 commit 4667b07

37 files changed

+997
-457
lines changed

doc/api/esm.md

+25-14
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
<!-- YAML
88
added: v8.5.0
99
changes:
10+
- version: REPLACEME
11+
pr-url: https://github.com/nodejs/node/pull/44710
12+
description: Loader hooks are executed off the main thread.
1013
- version:
1114
- v18.6.0
1215
- v16.17.0
@@ -325,6 +328,9 @@ added:
325328
- v13.9.0
326329
- v12.16.2
327330
changes:
331+
- version: REPLACEME
332+
pr-url: https://github.com/nodejs/node/pull/44710
333+
description: This API now returns a string synchronously instead of a Promise.
328334
- version:
329335
- v16.2.0
330336
- v14.18.0
@@ -340,29 +346,26 @@ command flag enabled.
340346
* `specifier` {string} The module specifier to resolve relative to `parent`.
341347
* `parent` {string|URL} The absolute parent module URL to resolve from. If none
342348
is specified, the value of `import.meta.url` is used as the default.
343-
* Returns: {Promise}
349+
* Returns: {string}
344350
345351
Provides a module-relative resolution function scoped to each module, returning
346-
the URL string.
352+
the URL string. In alignment with browser behavior, this now returns
353+
synchronously.
347354
348-
<!-- eslint-skip -->
355+
> **Caveat** This can result in synchronous file-system operations, which
356+
> can impact performance similarly to `require.resolve`.
349357
350358
```js
351-
const dependencyAsset = await import.meta.resolve('component-lib/asset.css');
359+
const dependencyAsset = import.meta.resolve('component-lib/asset.css');
352360
```
353361
354362
`import.meta.resolve` also accepts a second argument which is the parent module
355-
from which to resolve from:
356-
357-
<!-- eslint-skip -->
363+
from which to resolve:
358364
359365
```js
360-
await import.meta.resolve('./dep', import.meta.url);
366+
import.meta.resolve('./dep', import.meta.url);
361367
```
362368
363-
This function is asynchronous because the ES module resolver in Node.js is
364-
allowed to be asynchronous.
365-
366369
## Interoperability with CommonJS
367370
368371
### `import` statements
@@ -730,6 +733,11 @@ A hook that returns without calling `next<hookName>()` _and_ without returning
730733
`shortCircuit: true` also triggers an exception. These errors are to help
731734
prevent unintentional breaks in the chain.
732735
736+
Hooks are run in a separate thread, isolated from the main. That means it is a
737+
different [realm](https://tc39.es/ecma262/#realm). The hooks thread may be
738+
terminated by the main thread at any time, so do not depend on asynchronous
739+
operations to (like `console.log`) complete.
740+
733741
#### `resolve(specifier, context, nextResolve)`
734742
735743
<!-- YAML
@@ -762,7 +770,7 @@ changes:
762770
Node.js default `resolve` hook after the last user-supplied `resolve` hook
763771
* `specifier` {string}
764772
* `context` {Object}
765-
* Returns: {Object}
773+
* Returns: {Object|Promise}
766774
* `format` {string|null|undefined} A hint to the load hook (it might be
767775
ignored)
768776
`'builtin' | 'commonjs' | 'json' | 'module' | 'wasm'`
@@ -772,6 +780,9 @@ changes:
772780
terminate the chain of `resolve` hooks. **Default:** `false`
773781
* `url` {string} The absolute URL to which this input resolves
774782
783+
> **Caveat** Despite support for returning promises and async functions, calls
784+
> to `resolve` may block the main thread which can impact performance.
785+
775786
The `resolve` hook chain is responsible for telling Node.js where to find and
776787
how to cache a given `import` statement or expression. It can optionally return
777788
its format (such as `'module'`) as a hint to the `load` hook. If a format is
@@ -797,7 +808,7 @@ Node.js module specifier resolution behavior_ when calling `defaultResolve`, the
797808
`context.conditions` array originally passed into the `resolve` hook.
798809
799810
```js
800-
export async function resolve(specifier, context, nextResolve) {
811+
export function resolve(specifier, context, nextResolve) {
801812
const { parentURL = null } = context;
802813

803814
if (Math.random() > 0.5) { // Some condition.
@@ -1100,7 +1111,7 @@ const baseURL = pathToFileURL(`${cwd()}/`).href;
11001111
// CoffeeScript files end in .coffee, .litcoffee, or .coffee.md.
11011112
const extensionsRegex = /\.coffee$|\.litcoffee$|\.coffee\.md$/;
11021113
1103-
export async function resolve(specifier, context, nextResolve) {
1114+
export function resolve(specifier, context, nextResolve) {
11041115
if (extensionsRegex.test(specifier)) {
11051116
const { parentURL = baseURL } = context;
11061117

lib/internal/main/worker_thread.js

+54-35
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ const {
1010
ObjectDefineProperty,
1111
PromisePrototypeThen,
1212
RegExpPrototypeExec,
13+
SafeWeakMap,
1314
globalThis: {
1415
Atomics,
1516
SharedArrayBuffer,
@@ -89,23 +90,25 @@ port.on('message', (message) => {
8990
const {
9091
argv,
9192
cwdCounter,
92-
filename,
9393
doEval,
94-
workerData,
9594
environmentData,
96-
publicPort,
95+
filename,
96+
hasStdin,
9797
manifestSrc,
9898
manifestURL,
99-
hasStdin,
99+
publicPort,
100+
workerData,
100101
} = message;
101102

102-
if (argv !== undefined) {
103-
ArrayPrototypePushApply(process.argv, argv);
104-
}
103+
if (doEval !== 'internal') {
104+
if (argv !== undefined) {
105+
ArrayPrototypePushApply(process.argv, argv);
106+
}
105107

106-
const publicWorker = require('worker_threads');
107-
publicWorker.parentPort = publicPort;
108-
publicWorker.workerData = workerData;
108+
const publicWorker = require('worker_threads');
109+
publicWorker.parentPort = publicPort;
110+
publicWorker.workerData = workerData;
111+
}
109112

110113
require('internal/worker').assignEnvironmentData(environmentData);
111114

@@ -138,31 +141,47 @@ port.on('message', (message) => {
138141
debug(`[${threadId}] starts worker script ${filename} ` +
139142
`(eval = ${doEval}) at cwd = ${process.cwd()}`);
140143
port.postMessage({ type: UP_AND_RUNNING });
141-
if (doEval === 'classic') {
142-
const { evalScript } = require('internal/process/execution');
143-
const name = '[worker eval]';
144-
// This is necessary for CJS module compilation.
145-
// TODO: pass this with something really internal.
146-
ObjectDefineProperty(process, '_eval', {
147-
__proto__: null,
148-
configurable: true,
149-
enumerable: true,
150-
value: filename,
151-
});
152-
ArrayPrototypeSplice(process.argv, 1, 0, name);
153-
evalScript(name, filename);
154-
} else if (doEval === 'module') {
155-
const { evalModule } = require('internal/process/execution');
156-
PromisePrototypeThen(evalModule(filename), undefined, (e) => {
157-
workerOnGlobalUncaughtException(e, true);
158-
});
159-
} else {
160-
// script filename
161-
// runMain here might be monkey-patched by users in --require.
162-
// XXX: the monkey-patchability here should probably be deprecated.
163-
ArrayPrototypeSplice(process.argv, 1, 0, filename);
164-
const CJSLoader = require('internal/modules/cjs/loader');
165-
CJSLoader.Module.runMain(filename);
144+
switch (doEval) {
145+
case 'internal': {
146+
// Create this WeakMap in js-land because V8 has no C++ API for WeakMap.
147+
internalBinding('module_wrap').callbackMap = new SafeWeakMap();
148+
require(filename)(workerData, publicPort);
149+
break;
150+
}
151+
152+
case 'classic': {
153+
const { evalScript } = require('internal/process/execution');
154+
const name = '[worker eval]';
155+
// This is necessary for CJS module compilation.
156+
// TODO: pass this with something really internal.
157+
ObjectDefineProperty(process, '_eval', {
158+
__proto__: null,
159+
configurable: true,
160+
enumerable: true,
161+
value: filename,
162+
});
163+
ArrayPrototypeSplice(process.argv, 1, 0, name);
164+
evalScript(name, filename);
165+
break;
166+
}
167+
168+
case 'module': {
169+
const { evalModule } = require('internal/process/execution');
170+
PromisePrototypeThen(evalModule(filename), undefined, (e) => {
171+
workerOnGlobalUncaughtException(e, true);
172+
});
173+
break;
174+
}
175+
176+
default: {
177+
// script filename
178+
// runMain here might be monkey-patched by users in --require.
179+
// XXX: the monkey-patchability here should probably be deprecated.
180+
ArrayPrototypeSplice(process.argv, 1, 0, filename);
181+
const CJSLoader = require('internal/modules/cjs/loader');
182+
CJSLoader.Module.runMain(filename);
183+
break;
184+
}
166185
}
167186
} else if (message.type === STDIO_PAYLOAD) {
168187
const { stream, chunks } = message;

0 commit comments

Comments
 (0)