From 19cbaf13bf6dd83dc8e6d4b3e3eaac9636688fa9 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Thu, 7 Sep 2023 14:32:45 +0200 Subject: [PATCH 1/3] esm: remove return value for `Module.register` The current API shape si not great because it's too limited and redundant with the use of `MessagePort`. --- doc/api/module.md | 25 ++++++------------- lib/internal/modules/esm/hooks.js | 4 +-- lib/internal/modules/esm/loader.js | 4 +-- test/es-module/test-esm-loader-hooks.mjs | 10 ++++---- .../hooks-initialize-port.mjs | 1 - .../es-module-loaders/hooks-initialize.mjs | 1 - 6 files changed, 16 insertions(+), 29 deletions(-) diff --git a/doc/api/module.md b/doc/api/module.md index e7b94d34a63b18..28e2bc7d56e5ca 100644 --- a/doc/api/module.md +++ b/doc/api/module.md @@ -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][]. @@ -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`. + // Receive data from `register` } export async function resolve(specifier, context, nextResolve) { @@ -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: @@ -408,7 +402,6 @@ Module customization code: export async function initialize({ number, port }) { port.postMessage(`increment: ${number + 1}`); - return 'ok'; } ``` @@ -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 @@ -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)` diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js index b7e1afac31060d..08040a25b14c89 100644 --- a/lib/internal/modules/esm/hooks.js +++ b/lib/internal/modules/esm/hooks.js @@ -140,7 +140,7 @@ class Hooks { parentURL, kEmptyObject, ); - return this.addCustomLoader(urlOrSpecifier, keyedExports, data); + await this.addCustomLoader(urlOrSpecifier, keyedExports, data); } /** @@ -150,7 +150,7 @@ class Hooks { * @param {Record} 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} User data, ignored unless it's a promise, in which case it will be awaited. */ addCustomLoader(url, exports, data) { const { diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 5305c1eb8f37d9..14b78f660f1782 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -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'); @@ -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, diff --git a/test/es-module/test-esm-loader-hooks.mjs b/test/es-module/test-esm-loader-hooks.mjs index 2ea0128596e25b..b799b112fee4b0 100644 --- a/test/es-module/test-esm-loader-hooks.mjs +++ b/test/es-module/test-esm-loader-hooks.mjs @@ -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', '' ]); @@ -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'))} )); @@ -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); diff --git a/test/fixtures/es-module-loaders/hooks-initialize-port.mjs b/test/fixtures/es-module-loaders/hooks-initialize-port.mjs index c522e3fa8bfd98..cefe8854297c50 100644 --- a/test/fixtures/es-module-loaders/hooks-initialize-port.mjs +++ b/test/fixtures/es-module-loaders/hooks-initialize-port.mjs @@ -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) { diff --git a/test/fixtures/es-module-loaders/hooks-initialize.mjs b/test/fixtures/es-module-loaders/hooks-initialize.mjs index ab6f2c50d146e3..7622d982a9d7c5 100644 --- a/test/fixtures/es-module-loaders/hooks-initialize.mjs +++ b/test/fixtures/es-module-loaders/hooks-initialize.mjs @@ -4,5 +4,4 @@ let counter = 0; export async function initialize() { writeFileSync(1, `hooks initialize ${++counter}\n`); - return counter; } From 22f57944ed1ec1c5104228e78edf3e0dc2d29345 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Thu, 7 Sep 2023 14:55:00 +0200 Subject: [PATCH 2/3] fixup! esm: remove return value for `Module.register` --- doc/api/module.md | 1 - 1 file changed, 1 deletion(-) diff --git a/doc/api/module.md b/doc/api/module.md index 28e2bc7d56e5ca..e07e71c2520c6d 100644 --- a/doc/api/module.md +++ b/doc/api/module.md @@ -1105,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 From 3e8188bc7a8de278ecfc9b8aff0af24a0e7ecb54 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Thu, 7 Sep 2023 14:55:57 +0200 Subject: [PATCH 3/3] Update doc/api/module.md --- doc/api/module.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/module.md b/doc/api/module.md index e07e71c2520c6d..8f426297043652 100644 --- a/doc/api/module.md +++ b/doc/api/module.md @@ -347,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` + // Receives data from `register`. } export async function resolve(specifier, context, nextResolve) {