diff --git a/doc/api/esm.md b/doc/api/esm.md index 5e4ec1f9816bde..0dd17d1950dd94 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -751,7 +751,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 @@ -762,23 +763,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 8d826f40420445..64bfbe7fef9342 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, { @@ -317,21 +318,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 @@ -358,9 +348,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 54fff8831e5838..125cebd19866d1 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -171,17 +171,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') { @@ -189,7 +192,7 @@ class ESMLoader { } if (job === undefined) { - job = this.#createModuleJob(url, importAssertions, parentURL, format); + job = this.#createModuleJob(url, resolvedImportAssertions, parentURL, format); } return job; @@ -234,6 +237,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 07656d4ec40fa3..e7e7b20fd26798 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; }