Skip to content

Commit

Permalink
esm: remove return value for Module.register
Browse files Browse the repository at this point in the history
The current API shape si not great because it's too limited and
redundant with the use of `MessagePort`.

PR-URL: nodejs#49529
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>
  • Loading branch information
aduh95 authored and targos committed Nov 11, 2023
1 parent 9a7a8c5 commit fbc7c3e
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 30 deletions.
26 changes: 7 additions & 19 deletions doc/api/module.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ added: REPLACEME
[`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 @@ -346,7 +345,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 @@ -384,20 +383,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 @@ -406,7 +400,6 @@ Module customization code:

export async function initialize({ number, port }) {
port.postMessage(`increment: ${number + 1}`);
return 'ok';
}
```
Expand All @@ -426,13 +419,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 @@ -450,13 +441,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 @@ -1080,7 +1069,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 @@ -139,7 +139,7 @@ class Hooks {
parentURL,
kEmptyObject,
);
return this.addCustomLoader(urlOrSpecifier, keyedExports, data);
await this.addCustomLoader(urlOrSpecifier, keyedExports, data);
}

/**
Expand All @@ -149,7 +149,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 @@ -536,7 +536,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 @@ -562,7 +562,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 @@ -569,7 +569,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 @@ -647,10 +647,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 @@ -660,9 +660,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;
}

0 comments on commit fbc7c3e

Please sign in to comment.