From 4be5612bae9d59d29ac37f0f570baad8f778fbee Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sat, 9 Sep 2023 14:44:28 +0200 Subject: [PATCH] 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`. PR-URL: https://github.com/nodejs/node/pull/49529 Backport-PR-URL: https://github.com/nodejs/node/pull/50669 Reviewed-By: Geoffrey Booth Reviewed-By: Jacob Smith --- doc/api/module.md | 26 +++++-------------- 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(+), 30 deletions(-) diff --git a/doc/api/module.md b/doc/api/module.md index 9f6f447ab62bd6..cc8f7d0f2da3ae 100644 --- a/doc/api/module.md +++ b/doc/api/module.md @@ -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][]. @@ -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) { @@ -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: @@ -406,7 +400,6 @@ Module customization code: export async function initialize({ number, port }) { port.postMessage(`increment: ${number + 1}`); - return 'ok'; } ``` @@ -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 @@ -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)` @@ -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 diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js index abba6b51a05939..da714761e3859c 100644 --- a/lib/internal/modules/esm/hooks.js +++ b/lib/internal/modules/esm/hooks.js @@ -139,7 +139,7 @@ class Hooks { parentURL, kEmptyObject, ); - return this.addCustomLoader(urlOrSpecifier, keyedExports, data); + await this.addCustomLoader(urlOrSpecifier, keyedExports, data); } /** @@ -149,7 +149,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 608aef352d7018..73112f411b53db 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -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'); @@ -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, diff --git a/test/es-module/test-esm-loader-hooks.mjs b/test/es-module/test-esm-loader-hooks.mjs index 046e3d2e663fb1..1db6903ed368cf 100644 --- a/test/es-module/test-esm-loader-hooks.mjs +++ b/test/es-module/test-esm-loader-hooks.mjs @@ -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', '' ]); @@ -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'))} )); @@ -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); 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; }