GeezerVison is showing github.com — esm: allow resolve to return import assertions · nodejs/node@45de8d1
File treeExpand file treeCollapse file tree Open diff view settings test/fixtures/es-module-loaders
Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: doc/api/esm.md +16-14Lines changed: 16 additions & 14 deletions - Display the source diff
- Display the rich diff
| Original file line number | Diff line number | Diff line change |
|---|
@@ -751,7 +751,8 @@ changes: | 751 | 751 | * `specifier` {string} | 752 | 752 | * `context` {Object} | 753 | 753 | * `conditions` {string\[]} Export conditions of the relevant `package.json` | 754 | | - * `importAssertions` {Object} | | 754 | + * `importAssertions` {Object} An object whose key-value pairs represent the | | 755 | + assertions for the module to import | 755 | 756 | * `parentURL` {string|undefined} The module importing this one, or undefined | 756 | 757 | if this is the Node.js entry point | 757 | 758 | * `nextResolve` {Function} The subsequent `resolve` hook in the chain, or the | @@ -762,23 +763,24 @@ changes: | 762 | 763 | * `format` {string|null|undefined} A hint to the load hook (it might be | 763 | 764 | ignored) | 764 | 765 | `'builtin' | 'commonjs' | 'json' | 'module' | 'wasm'` | | 766 | + * `importAssertions` {Object|undefined} The import assertions to use when | | 767 | + caching the module (optional; if excluded the input will be used) | 765 | 768 | * `shortCircuit` {undefined|boolean} A signal that this hook intends to | 766 | 769 | terminate the chain of `resolve` hooks. **Default:** `false` | 767 | 770 | * `url` {string} The absolute URL to which this input resolves | 768 | 771 |
| 769 | | -The `resolve` hook chain is responsible for resolving file URL for a given | 770 | | -module specifier and parent URL, and optionally its format (such as `'module'`) | 771 | | -as a hint to the `load` hook. If a format is specified, the `load` hook is | 772 | | -ultimately responsible for providing the final `format` value (and it is free to | 773 | | -ignore the hint provided by `resolve`); if `resolve` provides a `format`, a | 774 | | -custom `load` hook is required even if only to pass the value to the Node.js | 775 | | -default `load` hook. | 776 | | -
| 777 | | -The module specifier is the string in an `import` statement or | 778 | | -`import()` expression. | 779 | | -
| 780 | | -The parent URL is the URL of the module that imported this one, or `undefined` | 781 | | -if this is the main entry point for the application. | | 772 | +The `resolve` hook chain is responsible for telling Node.js where to find and | | 773 | +how to cache a given `import` statement or expression. It can optionally return | | 774 | +its format (such as `'module'`) as a hint to the `load` hook. If a format is | | 775 | +specified, the `load` hook is ultimately responsible for providing the final | | 776 | +`format` value (and it is free to ignore the hint provided by `resolve`); if | | 777 | +`resolve` provides a `format`, a custom `load` hook is required even if only to | | 778 | +pass the value to the Node.js default `load` hook. | | 779 | +
| | 780 | +Import type assertions are part of the cache key for saving loaded modules into | | 781 | +the internal module cache. The `resolve` hook is responsible for | | 782 | +returning an `importAssertions` object if the module should be cached with | | 783 | +different assertions than were present in the source code. | 782 | 784 |
| 783 | 785 | The `conditions` property in `context` is an array of conditions for | 784 | 786 | [package exports conditions][Conditional Exports] that apply to this resolution | |
Collapse file Copy file name to clipboardExpand all lines: lib/internal/modules/esm/hooks.js +29-14Lines changed: 29 additions & 14 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -84,7 +84,7 @@ class Hooks { | 84 | 84 | }; | 85 | 85 |
| 86 | 86 | // Enable an optimization in ESMLoader.getModuleJob | 87 | | - hasCustomLoadHooks = false; | | 87 | + hasCustomResolveOrLoadHooks = false; | 88 | 88 |
| 89 | 89 | // Cache URLs we've already validated to avoid repeated validation | 90 | 90 | #validatedUrls = new SafeSet(); | @@ -125,6 +125,7 @@ class Hooks { | 125 | 125 | ); | 126 | 126 | } | 127 | 127 | if (resolve) { | | 128 | + this.hasCustomResolveOrLoadHooks = true; | 128 | 129 | ArrayPrototypePush( | 129 | 130 | this.#hooks.resolve, | 130 | 131 | { | @@ -134,7 +135,7 @@ class Hooks { | 134 | 135 | ); | 135 | 136 | } | 136 | 137 | if (load) { | 137 | | - this.hasCustomLoadHooks = true; | | 138 | + this.hasCustomResolveOrLoadHooks = true; | 138 | 139 | ArrayPrototypePush( | 139 | 140 | this.#hooks.load, | 140 | 141 | { | @@ -317,21 +318,10 @@ class Hooks { | 317 | 318 |
| 318 | 319 | const { | 319 | 320 | format, | | 321 | + importAssertions: resolvedImportAssertions, | 320 | 322 | url, | 321 | 323 | } = resolution; | 322 | 324 |
| 323 | | - if ( | 324 | | - format != null && | 325 | | - typeof format !== 'string' // [2] | 326 | | - ) { | 327 | | - throw new ERR_INVALID_RETURN_PROPERTY_VALUE( | 328 | | - 'a string', | 329 | | - hookErrIdentifier, | 330 | | - 'format', | 331 | | - format, | 332 | | - ); | 333 | | - } | 334 | | - | 335 | 325 | if (typeof url !== 'string') { | 336 | 326 | // non-strings can be coerced to a URL string | 337 | 327 | // validateString() throws a less-specific error | @@ -358,9 +348,34 @@ class Hooks { | 358 | 348 | } | 359 | 349 | } | 360 | 350 |
| | 351 | + if ( | | 352 | + resolvedImportAssertions != null && | | 353 | + typeof resolvedImportAssertions !== 'object' | | 354 | + ) { | | 355 | + throw new ERR_INVALID_RETURN_PROPERTY_VALUE( | | 356 | + 'an object', | | 357 | + hookErrIdentifier, | | 358 | + 'importAssertions', | | 359 | + resolvedImportAssertions, | | 360 | + ); | | 361 | + } | | 362 | + | | 363 | + if ( | | 364 | + format != null && | | 365 | + typeof format !== 'string' // [2] | | 366 | + ) { | | 367 | + throw new ERR_INVALID_RETURN_PROPERTY_VALUE( | | 368 | + 'a string', | | 369 | + hookErrIdentifier, | | 370 | + 'format', | | 371 | + format, | | 372 | + ); | | 373 | + } | | 374 | + | 361 | 375 | return { | 362 | 376 | __proto__: null, | 363 | 377 | format, | | 378 | + importAssertions: resolvedImportAssertions, | 364 | 379 | url, | 365 | 380 | }; | 366 | 381 | } | |
Collapse file Copy file name to clipboardExpand all lines: lib/internal/modules/esm/loader.js +9-5Lines changed: 9 additions & 5 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -171,25 +171,28 @@ class ESMLoader { | 171 | 171 |
| 172 | 172 | // We can skip cloning if there are no user-provided loaders because | 173 | 173 | // the Node.js default resolve hook does not use import assertions. | 174 | | - if (this.#hooks?.hasCustomLoadHooks) { | | 174 | + if (this.#hooks?.hasCustomResolveOrLoadHooks) { | | 175 | + // This method of cloning only works so long as import assertions cannot contain objects as values, | | 176 | + // which they currently cannot per spec. | 175 | 177 | importAssertionsForResolve = { | 176 | 178 | __proto__: null, | 177 | 179 | ...importAssertions, | 178 | 180 | }; | 179 | 181 | } | 180 | 182 |
| 181 | | - const { format, url } = | 182 | | - await this.resolve(specifier, parentURL, importAssertionsForResolve); | | 183 | + const resolveResult = await this.resolve(specifier, parentURL, importAssertionsForResolve); | | 184 | + const { url, format } = resolveResult; | | 185 | + const resolvedImportAssertions = resolveResult.importAssertions ?? importAssertions; | 183 | 186 |
| 184 | | - let job = this.moduleMap.get(url, importAssertions.type); | | 187 | + let job = this.moduleMap.get(url, resolvedImportAssertions.type); | 185 | 188 |
| 186 | 189 | // CommonJS will set functions for lazy job evaluation. | 187 | 190 | if (typeof job === 'function') { | 188 | 191 | this.moduleMap.set(url, undefined, job = job()); | 189 | 192 | } | 190 | 193 |
| 191 | 194 | if (job === undefined) { | 192 | | - job = this.#createModuleJob(url, importAssertions, parentURL, format); | | 195 | + job = this.#createModuleJob(url, resolvedImportAssertions, parentURL, format); | 193 | 196 | } | 194 | 197 |
| 195 | 198 | return job; | @@ -234,6 +237,7 @@ class ESMLoader { | 234 | 237 | if (process.env.WATCH_REPORT_DEPENDENCIES && process.send) { | 235 | 238 | process.send({ 'watch:import': [url] }); | 236 | 239 | } | | 240 | + | 237 | 241 | const ModuleJob = require('internal/modules/esm/module_job'); | 238 | 242 | const job = new ModuleJob( | 239 | 243 | this, | |
Collapse file Copy file name to clipboard +17-10Lines changed: 17 additions & 10 deletions | Original file line number | Diff line number | Diff line change |
|---|
| 1 | 1 | const DATA_URL_PATTERN = /^data:application\/json(?:[^,]*?)(;base64)?,([\s\S]*)$/; | 2 | | -const JSON_URL_PATTERN = /\.json(\?[^#]*)?(#.*)?$/; | | 2 | +const JSON_URL_PATTERN = /^[^?]+\.json(\?[^#]*)?(#.*)?$/; | | 3 | + | | 4 | +export async function resolve(specifier, context, next) { | | 5 | + const noAssertionSpecified = context.importAssertions.type == null; | 3 | 6 |
| 4 | | -export function resolve(url, context, next) { | 5 | 7 | // Mutation from resolve hook should be discarded. | 6 | 8 | context.importAssertions.type = 'whatever'; | 7 | | - return next(url); | 8 | | -} | 9 | 9 |
| 10 | | -export function load(url, context, next) { | 11 | | - if (context.importAssertions.type == null && | 12 | | - (DATA_URL_PATTERN.test(url) || JSON_URL_PATTERN.test(url))) { | 13 | | - const { importAssertions } = context; | 14 | | - importAssertions.type = 'json'; | | 10 | + // This fixture assumes that no other resolve hooks in the chain will error on invalid import assertions | | 11 | + // (as defaultResolve doesn't). | | 12 | + const result = await next(specifier, context); | | 13 | + | | 14 | + if (noAssertionSpecified && | | 15 | + (DATA_URL_PATTERN.test(result.url) || JSON_URL_PATTERN.test(result.url))) { | | 16 | + // Clean new import assertions object to ensure that this test isn't passing due to mutation. | | 17 | + result.importAssertions = { | | 18 | + ...(result.importAssertions ?? context.importAssertions), | | 19 | + type: 'json', | | 20 | + }; | 15 | 21 | } | 16 | | - return next(url); | | 22 | + | | 23 | + return result; | 17 | 24 | } |
|