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: remove return value for Module.register #49529

Merged
merged 3 commits into from
Sep 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 7 additions & 19 deletions doc/api/module.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ added: v20.6.0
[`initialize`][] hook.
* `transferList` {Object\[]} [transferrable objects][] to be passed into the
`initialize` hook.
* Returns: {any} returns whatever was returned by the `initialize` hook.

Register a module that exports [hooks][] that customize Node.js module
resolution and loading behavior. See [Customization hooks][].
Expand Down Expand Up @@ -348,7 +347,7 @@ names and signatures, and they must be exported as named exports.

```mjs
export async function initialize({ number, port }) {
// Receive data from `register`, return data to `register`.
// Receives data from `register`.
}

export async function resolve(specifier, context, nextResolve) {
Expand Down Expand Up @@ -386,20 +385,15 @@ added: REPLACEME
> Stability: 1.1 - Active development

* `data` {any} The data from `register(loader, import.meta.url, { data })`.
* Returns: {any} The data to be returned to the caller of `register`.

The `initialize` hook provides a way to define a custom function that runs in
the hooks thread when the hooks module is initialized. Initialization happens
when the hooks module is registered via [`register`][].

This hook can send and receive data from a [`register`][] invocation, including
ports and other transferrable objects. The return value of `initialize` must be
either:

* `undefined`,
* something that can be posted as a message between threads (e.g. the input to
[`port.postMessage`][]),
* a `Promise` resolving to one of the aforementioned values.
This hook can receive data from a [`register`][] invocation, including
ports and other transferrable objects. The return value of `initialize` can be a
{Promise}, in which case it will be awaited before the main application thread
execution resumes.

Module customization code:

Expand All @@ -408,7 +402,6 @@ Module customization code:

export async function initialize({ number, port }) {
port.postMessage(`increment: ${number + 1}`);
return 'ok';
}
```

Expand All @@ -428,13 +421,11 @@ port1.on('message', (msg) => {
assert.strictEqual(msg, 'increment: 2');
});

const result = register('./path-to-my-hooks.js', {
register('./path-to-my-hooks.js', {
parentURL: import.meta.url,
data: { number: 1, port: port2 },
transferList: [port2],
});

assert.strictEqual(result, 'ok');
```

```cjs
Expand All @@ -452,13 +443,11 @@ port1.on('message', (msg) => {
assert.strictEqual(msg, 'increment: 2');
});

const result = register('./path-to-my-hooks.js', {
register('./path-to-my-hooks.js', {
parentURL: pathToFileURL(__filename),
data: { number: 1, port: port2 },
transferList: [port2],
});

assert.strictEqual(result, 'ok');
```

#### `resolve(specifier, context, nextResolve)`
Expand Down Expand Up @@ -1116,7 +1105,6 @@ returned object contains the following keys:
[`Uint8Array`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Uint8Array
[`initialize`]: #initialize
[`module`]: modules.md#the-module-object
[`port.postMessage`]: worker_threads.md#portpostmessagevalue-transferlist
[`port.ref()`]: worker_threads.md#portref
[`port.unref()`]: worker_threads.md#portunref
[`register`]: #moduleregisterspecifier-parenturl-options
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/modules/esm/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ class Hooks {
parentURL,
kEmptyObject,
);
return this.addCustomLoader(urlOrSpecifier, keyedExports, data);
await this.addCustomLoader(urlOrSpecifier, keyedExports, data);
}

/**
Expand All @@ -150,7 +150,7 @@ class Hooks {
* @param {Record<string, unknown>} exports
* @param {any} [data] Arbitrary data to be passed from the custom loader (user-land)
* to the worker.
* @returns {any} The result of the loader's `initialize` hook, if provided.
* @returns {any | Promise<any>} User data, ignored unless it's a promise, in which case it will be awaited.
*/
addCustomLoader(url, exports, data) {
const {
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/modules/esm/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ function getHooksProxy() {
* @param {string} [options.parentURL] Base to use when resolving `specifier`
* @param {any} [options.data] Arbitrary data passed to the loader's `initialize` hook
* @param {any[]} [options.transferList] Objects in `data` that are changing ownership
* @returns {any} The result of the loader's initialize hook, if any
* @returns {void} We want to reserve the return value for potential future extension of the API.
* @example
* ```js
* register('./myLoader.js');
Expand All @@ -574,7 +574,7 @@ function register(specifier, parentURL = undefined, options) {
options = parentURL;
parentURL = options.parentURL;
}
return moduleLoader.register(
moduleLoader.register(
`${specifier}`,
parentURL ?? 'data:',
options?.data,
Expand Down
10 changes: 5 additions & 5 deletions test/es-module/test-esm-loader-hooks.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,7 @@ describe('Loader hooks', { concurrency: true }, () => {
]);

assert.strictEqual(stderr, '');
assert.deepStrictEqual(stdout.split('\n'), [ 'register ok',
assert.deepStrictEqual(stdout.split('\n'), [ 'register undefined',
'message initialize',
'message resolve node:os',
'' ]);
Expand Down Expand Up @@ -699,10 +699,10 @@ describe('Loader hooks', { concurrency: true }, () => {
'--eval',
`
import {register} from 'node:module';
console.log('result', register(
console.log('result 1', register(
${JSON.stringify(fixtures.fileURL('es-module-loaders/hooks-initialize.mjs'))}
));
console.log('result', register(
console.log('result 2', register(
${JSON.stringify(fixtures.fileURL('es-module-loaders/hooks-initialize.mjs'))}
));

Expand All @@ -712,9 +712,9 @@ describe('Loader hooks', { concurrency: true }, () => {

assert.strictEqual(stderr, '');
assert.deepStrictEqual(stdout.split('\n'), [ 'hooks initialize 1',
'result 1',
'result 1 undefined',
'hooks initialize 2',
'result 2',
'result 2 undefined',
'' ]);
assert.strictEqual(code, 0);
assert.strictEqual(signal, null);
Expand Down
1 change: 0 additions & 1 deletion test/fixtures/es-module-loaders/hooks-initialize-port.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ let thePort = null;
export async function initialize(port) {
port.postMessage('initialize');
thePort = port;
return 'ok';
}

export async function resolve(specifier, context, next) {
Expand Down
1 change: 0 additions & 1 deletion test/fixtures/es-module-loaders/hooks-initialize.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,4 @@ let counter = 0;

export async function initialize() {
writeFileSync(1, `hooks initialize ${++counter}\n`);
return counter;
}