esm: allow resolve to return import assertions

PR-URL: https://github.com/nodejs/node/pull/46153
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>

4 files changed, 71 insertions, 43 deletions

diff --git a/doc/api/esm.md b/doc/api/esm.md
index 9d859401ef..6293779131 100644
--- a/doc/api/esm.md
+++ b/doc/api/esm.md
@@ -754,7 +754,8 @@ changes:
 * `specifier` {string}
 * `context` {Object}
   * `conditions` {string\[]} Export conditions of the relevant `package.json`
-  * `importAssertions` {Object}
+  * `importAssertions` {Object} An object whose key-value pairs represent the
+    assertions for the module to import
   * `parentURL` {string|undefined} The module importing this one, or undefined
     if this is the Node.js entry point
 * `nextResolve` {Function} The subsequent `resolve` hook in the chain, or the
@@ -765,23 +766,24 @@ changes:
   * `format` {string|null|undefined} A hint to the load hook (it might be
     ignored)
     `'builtin' | 'commonjs' | 'json' | 'module' | 'wasm'`
+  * `importAssertions` {Object|undefined} The import assertions to use when
+    caching the module (optional; if excluded the input will be used)
   * `shortCircuit` {undefined|boolean} A signal that this hook intends to
     terminate the chain of `resolve` hooks. **Default:** `false`
   * `url` {string} The absolute URL to which this input resolves
 
-The `resolve` hook chain is responsible for resolving file URL for a given
-module specifier and parent URL, and optionally its format (such as `'module'`)
-as a hint to the `load` hook. If a format is specified, the `load` hook is
-ultimately responsible for providing the final `format` value (and it is free to
-ignore the hint provided by `resolve`); if `resolve` provides a `format`, a
-custom `load` hook is required even if only to pass the value to the Node.js
-default `load` hook.
-
-The module specifier is the string in an `import` statement or
-`import()` expression.
-
-The parent URL is the URL of the module that imported this one, or `undefined`
-if this is the main entry point for the application.
+The `resolve` hook chain is responsible for telling Node.js where to find and
+how to cache a given `import` statement or expression. It can optionally return
+its format (such as `'module'`) as a hint to the `load` hook. If a format is
+specified, the `load` hook is ultimately responsible for providing the final
+`format` value (and it is free to ignore the hint provided by `resolve`); if
+`resolve` provides a `format`, a custom `load` hook is required even if only to
+pass the value to the Node.js default `load` hook.
+
+Import type assertions are part of the cache key for saving loaded modules into
+the internal module cache. The `resolve` hook is responsible for
+returning an `importAssertions` object if the module should be cached with
+different assertions than were present in the source code.
 
 The `conditions` property in `context` is an array of conditions for
 [package exports conditions][Conditional Exports] that apply to this resolution
diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js
index d8a7cb550e..83e5038903 100644
--- a/lib/internal/modules/esm/hooks.js
+++ b/lib/internal/modules/esm/hooks.js
@@ -84,7 +84,7 @@ class Hooks {
   };
 
   // Enable an optimization in ESMLoader.getModuleJob
-  hasCustomLoadHooks = false;
+  hasCustomResolveOrLoadHooks = false;
 
   // Cache URLs we've already validated to avoid repeated validation
   #validatedUrls = new SafeSet();
@@ -125,6 +125,7 @@ class Hooks {
         );
       }
       if (resolve) {
+        this.hasCustomResolveOrLoadHooks = true;
         ArrayPrototypePush(
           this.#hooks.resolve,
           {
@@ -134,7 +135,7 @@ class Hooks {
         );
       }
       if (load) {
-        this.hasCustomLoadHooks = true;
+        this.hasCustomResolveOrLoadHooks = true;
         ArrayPrototypePush(
           this.#hooks.load,
           {
@@ -318,21 +319,10 @@ class Hooks {
 
     const {
       format,
+      importAssertions: resolvedImportAssertions,
       url,
     } = resolution;
 
-    if (
-      format != null &&
-      typeof format !== 'string' // [2]
-    ) {
-      throw new ERR_INVALID_RETURN_PROPERTY_VALUE(
-        'a string',
-        hookErrIdentifier,
-        'format',
-        format,
-      );
-    }
-
     if (typeof url !== 'string') {
       // non-strings can be coerced to a URL string
       // validateString() throws a less-specific error
@@ -359,9 +349,34 @@ class Hooks {
       }
     }
 
+    if (
+      resolvedImportAssertions != null &&
+      typeof resolvedImportAssertions !== 'object'
+    ) {
+      throw new ERR_INVALID_RETURN_PROPERTY_VALUE(
+        'an object',
+        hookErrIdentifier,
+        'importAssertions',
+        resolvedImportAssertions,
+      );
+    }
+
+    if (
+      format != null &&
+      typeof format !== 'string' // [2]
+    ) {
+      throw new ERR_INVALID_RETURN_PROPERTY_VALUE(
+        'a string',
+        hookErrIdentifier,
+        'format',
+        format,
+      );
+    }
+
     return {
       __proto__: null,
       format,
+      importAssertions: resolvedImportAssertions,
       url,
     };
   }
diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js
index 9c51434264..a834dc59dc 100644
--- a/lib/internal/modules/esm/loader.js
+++ b/lib/internal/modules/esm/loader.js
@@ -160,17 +160,20 @@ class ESMLoader {
 
     // We can skip cloning if there are no user-provided loaders because
     // the Node.js default resolve hook does not use import assertions.
-    if (this.#hooks?.hasCustomLoadHooks) {
+    if (this.#hooks?.hasCustomResolveOrLoadHooks) {
+      // This method of cloning only works so long as import assertions cannot contain objects as values,
+      // which they currently cannot per spec.
       importAssertionsForResolve = {
         __proto__: null,
         ...importAssertions,
       };
     }
 
-    const { format, url } =
-      await this.resolve(specifier, parentURL, importAssertionsForResolve);
+    const resolveResult = await this.resolve(specifier, parentURL, importAssertionsForResolve);
+    const { url, format } = resolveResult;
+    const resolvedImportAssertions = resolveResult.importAssertions ?? importAssertions;
 
-    let job = this.moduleMap.get(url, importAssertions.type);
+    let job = this.moduleMap.get(url, resolvedImportAssertions.type);
 
     // CommonJS will set functions for lazy job evaluation.
     if (typeof job === 'function') {
@@ -178,7 +181,7 @@ class ESMLoader {
     }
 
     if (job === undefined) {
-      job = this.#createModuleJob(url, importAssertions, parentURL, format);
+      job = this.#createModuleJob(url, resolvedImportAssertions, parentURL, format);
     }
 
     return job;
@@ -223,6 +226,7 @@ class ESMLoader {
     if (process.env.WATCH_REPORT_DEPENDENCIES && process.send) {
       process.send({ 'watch:import': [url] });
     }
+
     const ModuleJob = require('internal/modules/esm/module_job');
     const job = new ModuleJob(
       this,
diff --git a/test/fixtures/es-module-loaders/assertionless-json-import.mjs b/test/fixtures/es-module-loaders/assertionless-json-import.mjs
index 07656d4ec4..e7e7b20fd2 100644
--- a/test/fixtures/es-module-loaders/assertionless-json-import.mjs
+++ b/test/fixtures/es-module-loaders/assertionless-json-import.mjs
@@ -1,17 +1,24 @@
 const DATA_URL_PATTERN = /^data:application\/json(?:[^,]*?)(;base64)?,([\s\S]*)$/;
-const JSON_URL_PATTERN = /\.json(\?[^#]*)?(#.*)?$/;
+const JSON_URL_PATTERN = /^[^?]+\.json(\?[^#]*)?(#.*)?$/;
+
+export async function resolve(specifier, context, next) {
+  const noAssertionSpecified = context.importAssertions.type == null;
 
-export function resolve(url, context, next) {
   // Mutation from resolve hook should be discarded.
   context.importAssertions.type = 'whatever';
-  return next(url);
-}
 
-export function load(url, context, next) {
-  if (context.importAssertions.type == null &&
-      (DATA_URL_PATTERN.test(url) || JSON_URL_PATTERN.test(url))) {
-    const { importAssertions } = context;
-    importAssertions.type = 'json';
+  // This fixture assumes that no other resolve hooks in the chain will error on invalid import assertions
+  // (as defaultResolve doesn't).
+  const result = await next(specifier, context);
+
+  if (noAssertionSpecified &&
+      (DATA_URL_PATTERN.test(result.url) || JSON_URL_PATTERN.test(result.url))) {
+    // Clean new import assertions object to ensure that this test isn't passing due to mutation.
+    result.importAssertions = {
+      ...(result.importAssertions ?? context.importAssertions),
+      type: 'json',
+    };
   }
-  return next(url);
+
+  return result;
 }