esm: fix support for URL instances in register

PR-URL: #49655
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Author: aduh95

doc/api/module.md

@@ -84,14 +84,18 @@ isBuiltin('wss'); // false
 
 <!-- YAML
 added: v20.6.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/49655
+    description: Add support for WHATWG URL instances.
 -->
 
 > Stability: 1.2 - Release candidate
 
-* `specifier` {string} Customization hooks to be registered; this should be the
-  same string that would be passed to `import()`, except that if it is relative,
-  it is resolved relative to `parentURL`.
-* `parentURL` {string} If you want to resolve `specifier` relative to a base
+* `specifier` {string|URL} Customization hooks to be registered; this should be
+  the same string that would be passed to `import()`, except that if it is
+  relative, it is resolved relative to `parentURL`.
+* `parentURL` {string|URL} If you want to resolve `specifier` relative to a base
   URL, such as `import.meta.url`, you can pass that URL here. **Default:**
   `'data:'`
 * `options` {Object}

lib/internal/modules/esm/loader.js

@@ -14,7 +14,7 @@ const {
   ERR_UNKNOWN_MODULE_FORMAT,
 } = require('internal/errors').codes;
 const { getOptionValue } = require('internal/options');
-const { pathToFileURL } = require('internal/url');
+const { pathToFileURL, isURL } = require('internal/url');
 const { emitExperimentalWarning } = require('internal/util');
 const {
   getDefaultConditions,
@@ -320,7 +320,7 @@ class ModuleLoader {
       // eslint-disable-next-line no-use-before-define
       this.setCustomizations(new CustomizedModuleLoader());
     }
-    return this.#customizations.register(specifier, parentURL, data, transferList);
+    return this.#customizations.register(`${specifier}`, `${parentURL}`, data, transferList);
   }
 
   /**
@@ -541,11 +541,11 @@ function getHooksProxy() {
 
 /**
  * Register a single loader programmatically.
- * @param {string} specifier
- * @param {string} [parentURL] Base to use when resolving `specifier`; optional if
+ * @param {string|import('url').URL} specifier
+ * @param {string|import('url').URL} [parentURL] Base to use when resolving `specifier`; optional if
  * `specifier` is absolute. Same as `options.parentUrl`, just inline
  * @param {object} [options] Additional options to apply, described below.
- * @param {string} [options.parentURL] Base to use when resolving `specifier`
+ * @param {string|import('url').URL} [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 {void} We want to reserve the return value for potential future extension of the API.
@@ -570,12 +570,12 @@ function getHooksProxy() {
  */
 function register(specifier, parentURL = undefined, options) {
   const moduleLoader = require('internal/process/esm_loader').esmLoader;
-  if (parentURL != null && typeof parentURL === 'object') {
+  if (parentURL != null && typeof parentURL === 'object' && !isURL(parentURL)) {
     options = parentURL;
     parentURL = options.parentURL;
   }
   moduleLoader.register(
-    `${specifier}`,
+    specifier,
     parentURL ?? 'data:',
     options?.data,
     options?.transferList,

test/es-module/test-esm-loader-hooks.mjs

@@ -507,6 +507,37 @@ describe('Loader hooks', { concurrency: true }, () => {
       assert.strictEqual(signal, null);
     });
 
+    it('should have `register` accept URL objects as `parentURL`', async () => {
+      const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
+        '--no-warnings',
+        '--import',
+        `data:text/javascript,${encodeURIComponent(
+          'import{ register } from "node:module";' +
+          'import { pathToFileURL } from "node:url";' +
+          'register("./hooks-initialize.mjs", pathToFileURL("./"));'
+        )}`,
+        '--input-type=module',
+        '--eval',
+        `
+        import {register} from 'node:module';
+        register(
+          ${JSON.stringify(fixtures.fileURL('es-module-loaders/loader-load-foo-or-42.mjs'))},
+          new URL('data:'),
+        );
+
+        import('node:os').then((result) => {
+          console.log(JSON.stringify(result));
+        });
+        `,
+      ], { cwd: fixtures.fileURL('es-module-loaders/') });
+
+      assert.strictEqual(stderr, '');
+      assert.deepStrictEqual(stdout.split('\n').sort(), ['hooks initialize 1', '{"default":"foo"}', ''].sort());
+
+      assert.strictEqual(code, 0);
+      assert.strictEqual(signal, null);
+    });
+
     it('should have `register` work with cjs', async () => {
       const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
         '--no-warnings',