From 1a9e7d68a14724f7310c2788af4132e0c87ce061 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Mon, 9 Jan 2023 17:09:40 -0800 Subject: [PATCH] esm: resolve optionally returns import assertions --- doc/api/esm.md | 46 +++++++++++-------- lib/internal/modules/esm/hooks.js | 44 +++++++++++------- lib/internal/modules/esm/loader.js | 21 ++++----- lib/internal/modules/esm/resolve.js | 8 +++- .../assertionless-json-import.mjs | 19 ++++---- 5 files changed, 79 insertions(+), 59 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 9a4273940a7ce0..76d2000b92b541 100644 --- a/doc/api/esm.md +++ b/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 diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js index a642b7b111e0df..4195fe5d8f8338 100644 --- a/lib/internal/modules/esm/hooks.js +++ b/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, }; } diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 04982ca94a9221..1fc785546349d5 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -158,19 +158,21 @@ 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') { @@ -178,7 +180,7 @@ class ESMLoader { } 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); } diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index edb74a0395d26b..6955962eb7f942 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/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), }; } diff --git a/test/fixtures/es-module-loaders/assertionless-json-import.mjs b/test/fixtures/es-module-loaders/assertionless-json-import.mjs index 07656d4ec40fa3..5b354c1f68f0df 100644 --- a/test/fixtures/es-module-loaders/assertionless-json-import.mjs +++ b/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 }); }