esm: resolve optionally returns import assertions

Author: GeoffreyBooth

doc/api/esm.md

@@ -754,34 +754,39 @@ changes:
 * `specifier` {string}
 * `context` {Object}
   * `conditions` {string\[]} Export conditions of the relevant `package.json`
-  * `importAssertions` {Object}
-  * `parentURL` {string|undefined} The module importing this one, or undefined
+  * `importAssertions` {Object} The object after the `assert` in an `import`
+    statement, or the value of the `assert` property in the second argument of
+    an `import()` expression; or an empty object
+  * `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
   Node.js default `resolve` hook after the last user-supplied `resolve` hook
   * `specifier` {string}
   * `context` {Object}
 * Returns: {Object}
-  * `format` {string|null|undefined} A hint to the load hook (it might be
+  * `format` {string | null | undefined} A hint to the load hook (it might be
     ignored)
     `'builtin' | 'commonjs' | 'json' | 'module' | 'wasm'`
-  * `shortCircuit` {undefined|boolean} A signal that this hook intends to
+  * `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 assertions are part of the cache key for saving loaded modules into the
+Node.js 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 (for example, if no assertions
+were present but the module should be cached with assertions
+`{ type: 'json' }`).
 
 The `conditions` property in `context` is an array of conditions for
 [package exports conditions][Conditional Exports] that apply to this resolution
@@ -846,7 +851,7 @@ changes:
 * `url` {string} The URL returned by the `resolve` chain
 * `context` {Object}
   * `conditions` {string\[]} Export conditions of the relevant `package.json`
-  * `format` {string|null|undefined} The format optionally supplied by the
+  * `format` {string | null | undefined} The format optionally supplied by the
     `resolve` hook chain
   * `importAssertions` {Object}
 * `nextLoad` {Function} The subsequent `load` hook in the chain, or the
@@ -855,9 +860,10 @@ changes:
   * `context` {Object}
 * Returns: {Object}
   * `format` {string}
-  * `shortCircuit` {undefined|boolean} A signal that this hook intends to
+  * `shortCircuit` {undefined | boolean} A signal that this hook intends to
     terminate the chain of `resolve` hooks. **Default:** `false`
-  * `source` {string|ArrayBuffer|TypedArray} The source for Node.js to evaluate
+  * `source` {string | ArrayBuffer | TypedArray} The source for Node.js to
+    evaluate
 
 The `load` hook provides a way to define a custom method of determining how
 a URL should be interpreted, retrieved, and parsed. It is also in charge of

lib/internal/modules/esm/hooks.js

@@ -316,22 +316,7 @@ class Hooks {
       throw new ERR_LOADER_CHAIN_INCOMPLETE(hookErrIdentifier);
     }
 
-    const {
-      format,
-      url,
-    } = resolution;
-
-    if (
-      format != null &&
-      typeof format !== 'string' // [2]
-    ) {
-      throw new ERR_INVALID_RETURN_PROPERTY_VALUE(
-        'a string',
-        hookErrIdentifier,
-        'format',
-        format,
-      );
-    }
+    const { url, importAssertions: resolvedImportAssertions, format } = resolution;
 
     if (typeof url !== 'string') {
       // non-strings can be coerced to a URL string
@@ -359,10 +344,35 @@ 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,
       url,
+      importAssertions: resolvedImportAssertions,
+      format,
     };
   }
 

lib/internal/modules/esm/loader.js

@@ -158,27 +158,29 @@ class ESMLoader {
   async getModuleJob(specifier, parentURL, importAssertions) {
     let importAssertionsForResolve;
 
-    // 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) {
       importAssertionsForResolve = {
         __proto__: null,
         ...importAssertions,
       };
+    } else {
+      // We can skip cloning if there are no user-provided loaders.
+      importAssertionsForResolve = 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 ?? importAssertionsForResolve;
 
-    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') {
       this.moduleMap.set(url, undefined, job = job());
     }
 
     if (job === undefined) {
-      job = this.#createModuleJob(url, importAssertions, parentURL, format);
+      job = this.#createModuleJob(url, resolvedImportAssertions, parentURL, format);
     }
 
     return job;
@@ -223,6 +225,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,
@@ -299,11 +302,7 @@ class ESMLoader {
    *                                              statement or expression.
    * @returns {Promise<{ format: string, url: URL['href'] }>}
    */
-  async resolve(
-    originalSpecifier,
-    parentURL,
-    importAssertions = { __proto__: null },
-  ) {
+  async resolve(originalSpecifier, parentURL, importAssertions = { __proto__: null }) {
     if (this.#hooks) {
       return this.#hooks.resolve(originalSpecifier, parentURL, importAssertions);
     }

lib/internal/modules/esm/resolve.js

@@ -966,6 +966,7 @@ function throwIfUnsupportedURLScheme(parsed, experimentalNetworkImports) {
 }
 
 async function defaultResolve(specifier, context = {}) {
+  const { importAssertions } = context;
   let { parentURL, conditions } = context;
   if (parentURL && policy?.manifest) {
     const redirects = policy.manifest.getDependencyMapper(parentURL);
@@ -1017,7 +1018,7 @@ async function defaultResolve(specifier, context = {}) {
         )
       )
     ) {
-      return { __proto__: null, url: parsed.href };
+      return { __proto__: null, url: parsed.href, importAssertions };
     }
   } catch {
     // Ignore exception
@@ -1035,7 +1036,9 @@ async function defaultResolve(specifier, context = {}) {
   if (maybeReturn) return maybeReturn;
 
   // This must come after checkIfDisallowedImport
-  if (parsed && parsed.protocol === 'node:') return { __proto__: null, url: specifier };
+  if (parsed && parsed.protocol === 'node:') {
+    return { __proto__: null, url: specifier, importAssertions };
+  }
 
   throwIfUnsupportedURLScheme(parsed, experimentalNetworkImports);
 
@@ -1091,6 +1094,7 @@ async function defaultResolve(specifier, context = {}) {
     // Do NOT cast `url` to a string: that will work even when there are real
     // problems, silencing them
     url: url.href,
+    importAssertions,
     format: defaultGetFormatWithoutErrors(url, context),
   };
 }

test/fixtures/es-module-loaders/assertionless-json-import.mjs

@@ -2,16 +2,17 @@ const DATA_URL_PATTERN = /^data:application\/json(?:[^,]*?)(;base64)?,([\s\S]*)$
 const JSON_URL_PATTERN = /\.json(\?[^#]*)?(#.*)?$/;
 
 export function resolve(url, context, next) {
+  const resolvedImportAssertions = {}
+  if (context.importAssertions.type) {
+    resolvedImportAssertions.type = context.importAssertions.type;
+  }
+
+  if (resolvedImportAssertions.type == null && (DATA_URL_PATTERN.test(url) || JSON_URL_PATTERN.test(url))) {
+    resolvedImportAssertions.type = 'json';
+  }
+
   // 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';
-  }
-  return next(url);
+  return next(url, { ...context, importAssertions: resolvedImportAssertions });
 }