esm: allow resolve to return import assertions

doc/api/esm.md

@@ -754,8 +754,9 @@ 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
     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
@@ -765,23 +766,26 @@ 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 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
@@ -857,7 +861,8 @@ changes:
   * `format` {string}
   * `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

@@ -116,6 +116,7 @@ class Hooks {
       } = pluckHooks(exports);
 
       if (globalPreload) {
+        this.hasCustomLoadHooks = true;
         ArrayPrototypePush(
           this.#hooks.globalPreload,
           {
@@ -125,6 +126,7 @@ class Hooks {
         );
       }
       if (resolve) {
+        this.hasCustomLoadHooks = true;
         ArrayPrototypePush(
           this.#hooks.resolve,
           {
@@ -318,21 +320,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 +350,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,
     };
   }

lib/internal/modules/esm/loader.js

@@ -155,7 +155,7 @@ class ESMLoader {
    *                                                  module import.
    * @returns {Promise<ModuleJob>} The (possibly pending) module job
    */
-  async getModuleJob(specifier, parentURL, importAssertions) {
+  async getModuleJob(specifier, parentURL, importAssertions = { __proto__: null }) {
     let importAssertionsForResolve;
 
     // We can skip cloning if there are no user-provided loaders because
@@ -167,18 +167,19 @@ class ESMLoader {
       };
     }
 
-    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') {
       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 +224,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,

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

@@ -1,17 +1,21 @@
 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);
-}
+  context.importAssertions.type ??= 'whatever';
 
-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 = { type: 'json' };
   }
-  return next(url);
+
+  return result;
 }