From 7ae7c89d479b69c2b217e11e752945388a9f9312 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel <duhamelantoine1995@gmail.com> Date: Sat, 14 Oct 2023 05:52:38 +0200 Subject: [PATCH] esm: use import attributes instead of import assertions The old import assertions proposal has been renamed to "import attributes" with the follwing major changes: 1. The keyword is now `with` instead of `assert`. 2. Unknown assertions cause an error rather than being ignored, This commit updates the documentation to encourage folks to use the new syntax, and add aliases for module customization hooks. PR-URL: https://github.com/nodejs/node/pull/50140 Fixes: https://github.com/nodejs/node/issues/50134 Refs: https://github.com/v8/v8/commit/159c82c5e6392e78b9bba7161b1bed6e23758984 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> --- doc/api/errors.md | 17 +++- doc/api/esm.md | 25 +++--- doc/api/module.md | 19 +++-- lib/internal/errors.js | 9 ++- lib/internal/modules/cjs/loader.js | 8 +- lib/internal/modules/esm/assert.js | 53 ++++++------- lib/internal/modules/esm/hooks.js | 54 ++++++++++--- lib/internal/modules/esm/load.js | 22 ++++-- lib/internal/modules/esm/loader.js | 77 ++++++++++--------- lib/internal/modules/esm/module_job.js | 10 +-- lib/internal/modules/esm/module_map.js | 16 ++-- lib/internal/modules/esm/translators.js | 16 ++-- lib/internal/process/execution.js | 4 +- lib/repl.js | 8 +- src/module_wrap.cc | 44 +++++------ src/module_wrap.h | 2 +- ...s => test-esm-dynamic-import-attribute.js} | 0 ... => test-esm-dynamic-import-attribute.mjs} | 0 .../es-module/test-esm-import-assertion-2.mjs | 6 -- .../test-esm-import-assertion-validation.js | 45 ----------- .../test-esm-import-assertion-warning.mjs | 43 +++++++++-- ...1.mjs => test-esm-import-attributes-1.mjs} | 0 ...4.mjs => test-esm-import-attributes-2.mjs} | 0 ...3.mjs => test-esm-import-attributes-3.mjs} | 0 ...s => test-esm-import-attributes-errors.js} | 12 ++- ... => test-esm-import-attributes-errors.mjs} | 10 +-- .../test-esm-import-attributes-validation.js | 45 +++++++++++ test/es-module/test-esm-json.mjs | 1 - test/es-module/test-esm-virtual-json.mjs | 2 +- .../assertionless-json-import.mjs | 14 ++-- .../builtin-named-exports-loader.mjs | 2 +- .../es-module-loaders/hooks-custom.mjs | 4 +- .../es-module-loaders/hooks-input.mjs | 12 +-- .../loader-invalid-format.mjs | 2 +- .../es-module-loaders/loader-invalid-url.mjs | 4 +- .../es-module-loaders/loader-with-dep.mjs | 4 +- .../not-found-assert-loader.mjs | 4 +- .../es-module-loaders/string-sources.mjs | 2 +- .../parallel/test-vm-module-dynamic-import.js | 4 +- 39 files changed, 338 insertions(+), 262 deletions(-) rename test/es-module/{test-esm-dynamic-import-assertion.js => test-esm-dynamic-import-attribute.js} (100%) rename test/es-module/{test-esm-dynamic-import-assertion.mjs => test-esm-dynamic-import-attribute.mjs} (100%) delete mode 100644 test/es-module/test-esm-import-assertion-2.mjs delete mode 100644 test/es-module/test-esm-import-assertion-validation.js rename test/es-module/{test-esm-import-assertion-1.mjs => test-esm-import-attributes-1.mjs} (100%) rename test/es-module/{test-esm-import-assertion-4.mjs => test-esm-import-attributes-2.mjs} (100%) rename test/es-module/{test-esm-import-assertion-3.mjs => test-esm-import-attributes-3.mjs} (100%) rename test/es-module/{test-esm-import-assertion-errors.js => test-esm-import-attributes-errors.js} (84%) rename test/es-module/{test-esm-import-assertion-errors.mjs => test-esm-import-attributes-errors.mjs} (81%) create mode 100644 test/es-module/test-esm-import-attributes-validation.js diff --git a/doc/api/errors.md b/doc/api/errors.md index 95ad3c9c671954..89048c6485df0f 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1759,7 +1759,8 @@ added: - v16.14.0 --> -An import assertion has failed, preventing the specified module to be imported. +An import `type` attribute was provided, but the specified module is of a +different type. <a id="ERR_IMPORT_ASSERTION_TYPE_MISSING"></a> @@ -1771,7 +1772,7 @@ added: - v16.14.0 --> -An import assertion is missing, preventing the specified module to be imported. +An import attribute is missing, preventing the specified module to be imported. <a id="ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED"></a> @@ -1783,7 +1784,17 @@ added: - v16.14.0 --> -An import assertion is not supported by this version of Node.js. +An import attribute is not supported by this version of Node.js. + +<a id="ERR_IMPORT_ATTRIBUTE_UNSUPPORTED"></a> + +### `ERR_IMPORT_ATTRIBUTE_UNSUPPORTED` + +<!-- YAML +added: REPLACEME +--> + +An import attribute is not supported by this version of Node.js. <a id="ERR_INCOMPATIBLE_OPTION_PAIR"></a> diff --git a/doc/api/esm.md b/doc/api/esm.md index b40b4ccb6f0865..35eea1b6413e21 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -19,7 +19,7 @@ changes: - v17.1.0 - v16.14.0 pr-url: https://github.com/nodejs/node/pull/40250 - description: Add support for import assertions. + description: Add experimental support for import assertions. - version: - v17.0.0 - v16.12.0 @@ -234,7 +234,9 @@ absolute URL strings. import fs from 'node:fs/promises'; ``` -## Import assertions +<a id="import-assertions"></a> + +## Import attributes <!-- YAML added: @@ -242,9 +244,14 @@ added: - v16.14.0 --> -> Stability: 1 - Experimental +> Stability: 1.1 - Active development + +> This feature was previously named "Import assertions", and using the `assert` +> keyword instead of `with`. Because the version of V8 on this release line does +> not support the `with` keyword, you need to keep using `assert` to support +> this version of Node.js. -The [Import Assertions proposal][] adds an inline syntax for module import +The [Import Attributes proposal][] adds an inline syntax for module import statements to pass on more information alongside the module specifier. ```js @@ -254,10 +261,10 @@ const { default: barData } = await import('./bar.json', { assert: { type: 'json' } }); ``` -Node.js supports the following `type` values, for which the assertion is +Node.js supports the following `type` values, for which the attribute is mandatory: -| Assertion `type` | Needed for | +| Attribute `type` | Needed for | | ---------------- | ---------------- | | `'json'` | [JSON modules][] | @@ -547,7 +554,7 @@ JSON files can be referenced by `import`: import packageConfig from './package.json' assert { type: 'json' }; ``` -The `assert { type: 'json' }` syntax is mandatory; see [Import Assertions][]. +The `assert { type: 'json' }` syntax is mandatory; see [Import Attributes][]. The imported JSON only exposes a `default` export. There is no support for named exports. A cache entry is created in the CommonJS cache to avoid duplication. @@ -1050,8 +1057,8 @@ resolution for ESM specifiers is [commonjs-extension-resolution-loader][]. [Determining module system]: packages.md#determining-module-system [Dynamic `import()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import [ES Module Integration Proposal for WebAssembly]: https://github.com/webassembly/esm-integration -[Import Assertions]: #import-assertions -[Import Assertions proposal]: https://github.com/tc39/proposal-import-assertions +[Import Attributes]: #import-attributes +[Import Attributes proposal]: https://github.com/tc39/proposal-import-attributes [JSON modules]: #json-modules [Module customization hooks]: module.md#customization-hooks [Node.js Module Resolution And Loading Algorithm]: #resolution-algorithm-specification diff --git a/doc/api/module.md b/doc/api/module.md index 8bf5c09041a41a..857c02142b199f 100644 --- a/doc/api/module.md +++ b/doc/api/module.md @@ -458,6 +458,11 @@ register('./path-to-my-hooks.js', { <!-- YAML changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/50140 + description: The property `context.importAssertions` is replaced with + `context.importAttributes`. Using the old name is still + supported and will emit an experimental warning. - version: - v18.6.0 - v16.17.0 @@ -477,8 +482,8 @@ changes: * `specifier` {string} * `context` {Object} * `conditions` {string\[]} Export conditions of the relevant `package.json` - * `importAssertions` {Object} An object whose key-value pairs represent the - assertions for the module to import + * `importAttributes` {Object} An object whose key-value pairs represent the + attributes 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 @@ -489,7 +494,7 @@ 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 + * `importAttributes` {Object|undefined} The import attributes 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` @@ -506,10 +511,10 @@ 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 +Import type attributes 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. +`importAttributes` object if the module should be cached with different +attributes 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 @@ -575,7 +580,7 @@ changes: * `conditions` {string\[]} Export conditions of the relevant `package.json` * `format` {string|null|undefined} The format optionally supplied by the `resolve` hook chain - * `importAssertions` {Object} + * `importAttributes` {Object} * `nextLoad` {Function} The subsequent `load` hook in the chain, or the Node.js default `load` hook after the last user-supplied `load` hook * `specifier` {string} diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 4e332e1ce18d16..f5fee095665cd0 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1178,12 +1178,17 @@ E('ERR_HTTP_SOCKET_ENCODING', E('ERR_HTTP_TRAILER_INVALID', 'Trailers are invalid with this transfer encoding', Error); E('ERR_ILLEGAL_CONSTRUCTOR', 'Illegal constructor', TypeError); +// TODO(aduh95): change the error to mention import attributes instead of import assertions. E('ERR_IMPORT_ASSERTION_TYPE_FAILED', 'Module "%s" is not of type "%s"', TypeError); +// TODO(aduh95): change the error to mention import attributes instead of import assertions. E('ERR_IMPORT_ASSERTION_TYPE_MISSING', - 'Module "%s" needs an import assertion of type "%s"', TypeError); + 'Module "%s" needs an import attribute of type "%s"', TypeError); +// TODO(aduh95): change the error to mention import attributes instead of import assertions. E('ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED', - 'Import assertion type "%s" is unsupported', TypeError); + 'Import attribute type "%s" is unsupported', TypeError); +E('ERR_IMPORT_ATTRIBUTE_UNSUPPORTED', + 'Import attribute "%s" with value "%s" is not supported', TypeError); E('ERR_INCOMPATIBLE_OPTION_PAIR', 'Option "%s" cannot be used in combination with option "%s"', TypeError); E('ERR_INPUT_TYPE_NOT_ALLOWED', '--input-type can only be used with string ' + diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 5fe9e874c7ad9f..042172290def25 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -1295,10 +1295,10 @@ function wrapSafe(filename, content, cjsModuleInstance, codeCache) { const script = new Script(wrapper, { filename, lineOffset: 0, - importModuleDynamically: async (specifier, _, importAssertions) => { + importModuleDynamically: async (specifier, _, importAttributes) => { const cascadedLoader = getCascadedLoader(); return cascadedLoader.import(specifier, normalizeReferrerURL(filename), - importAssertions); + importAttributes); }, }); @@ -1322,10 +1322,10 @@ function wrapSafe(filename, content, cjsModuleInstance, codeCache) { ], { filename, cachedData: codeCache, - importModuleDynamically(specifier, _, importAssertions) { + importModuleDynamically(specifier, _, importAttributes) { const cascadedLoader = getCascadedLoader(); return cascadedLoader.import(specifier, normalizeReferrerURL(filename), - importAssertions); + importAttributes); }, }); diff --git a/lib/internal/modules/esm/assert.js b/lib/internal/modules/esm/assert.js index 45df6781d49497..ce3280de84bf4d 100644 --- a/lib/internal/modules/esm/assert.js +++ b/lib/internal/modules/esm/assert.js @@ -13,16 +13,15 @@ const { ERR_IMPORT_ASSERTION_TYPE_FAILED, ERR_IMPORT_ASSERTION_TYPE_MISSING, ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED, + ERR_IMPORT_ATTRIBUTE_UNSUPPORTED, } = require('internal/errors').codes; // The HTML spec has an implied default type of `'javascript'`. const kImplicitAssertType = 'javascript'; -let alreadyWarned = false; - /** - * Define a map of module formats to import assertion types (the value of - * `type` in `assert { type: 'json' }`). + * Define a map of module formats to import attributes types (the value of + * `type` in `with { type: 'json' }`). * @type {Map<string, string>} */ const formatTypeMap = { @@ -31,13 +30,13 @@ const formatTypeMap = { 'commonjs': kImplicitAssertType, 'json': 'json', 'module': kImplicitAssertType, - 'wasm': kImplicitAssertType, // It's unclear whether the HTML spec will require an assertion type or not for Wasm; see https://github.com/WebAssembly/esm-integration/issues/42 + 'wasm': kImplicitAssertType, // It's unclear whether the HTML spec will require an attribute type or not for Wasm; see https://github.com/WebAssembly/esm-integration/issues/42 }; /** * The HTML spec disallows the default type to be explicitly specified * (for now); so `import './file.js'` is okay but - * `import './file.js' assert { type: 'javascript' }` throws. + * `import './file.js' with { type: 'javascript' }` throws. * @type {Array<string, string>} */ const supportedAssertionTypes = ArrayPrototypeFilter( @@ -46,54 +45,50 @@ const supportedAssertionTypes = ArrayPrototypeFilter( /** - * Test a module's import assertions. + * Test a module's import attributes. * @param {string} url The URL of the imported module, for error reporting. * @param {string} format One of Node's supported translators - * @param {Record<string, string>} importAssertions Validations for the + * @param {Record<string, string>} importAttributes Validations for the * module import. * @returns {true} * @throws {TypeError} If the format and assertion type are incompatible. */ -function validateAssertions(url, format, - importAssertions = { __proto__: null }) { - const validType = formatTypeMap[format]; - - if (!alreadyWarned && ObjectKeys(importAssertions).length !== 0) { - alreadyWarned = true; - process.emitWarning( - 'Import assertions are not a stable feature of the JavaScript language. ' + - 'Avoid relying on their current behavior and syntax as those might change ' + - 'in a future version of Node.js.', - 'ExperimentalWarning', - ); +function validateAttributes(url, format, + importAttributes = { __proto__: null }) { + const keys = ObjectKeys(importAttributes); + for (let i = 0; i < keys.length; i++) { + if (keys[i] !== 'type') { + throw new ERR_IMPORT_ATTRIBUTE_UNSUPPORTED(keys[i], importAttributes[keys[i]]); + } } + const validType = formatTypeMap[format]; switch (validType) { case undefined: - // Ignore assertions for module formats we don't recognize, to allow new + // Ignore attributes for module formats we don't recognize, to allow new // formats in the future. return true; case kImplicitAssertType: // This format doesn't allow an import assertion type, so the property - // must not be set on the import assertions object. - if (!ObjectPrototypeHasOwnProperty(importAssertions, 'type')) { + // must not be set on the import attributes object. + if (!ObjectPrototypeHasOwnProperty(importAttributes, 'type')) { return true; } - return handleInvalidType(url, importAssertions.type); + return handleInvalidType(url, importAttributes.type); - case importAssertions.type: + case importAttributes.type: // The asserted type is the valid type for this format. return true; default: // There is an expected type for this format, but the value of - // `importAssertions.type` might not have been it. - if (!ObjectPrototypeHasOwnProperty(importAssertions, 'type')) { + // `importAttributes.type` might not have been it. + if (!ObjectPrototypeHasOwnProperty(importAttributes, 'type')) { // `type` wasn't specified at all. throw new ERR_IMPORT_ASSERTION_TYPE_MISSING(url, validType); } - return handleInvalidType(url, importAssertions.type); + return handleInvalidType(url, importAttributes.type); } } @@ -118,5 +113,5 @@ function handleInvalidType(url, type) { module.exports = { kImplicitAssertType, - validateAssertions, + validateAttributes, }; diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js index 05885050b82fc9..12b0366eade9ec 100644 --- a/lib/internal/modules/esm/hooks.js +++ b/lib/internal/modules/esm/hooks.js @@ -9,6 +9,7 @@ const { ObjectDefineProperty, ObjectSetPrototypeOf, Promise, + ReflectSet, SafeSet, StringPrototypeSlice, StringPrototypeStartsWith, @@ -71,6 +72,30 @@ let debug = require('internal/util/debuglog').debuglog('esm', (fn) => { }); let importMetaInitializer; +let importAssertionAlreadyWarned = false; + +function emitImportAssertionWarning() { + if (!importAssertionAlreadyWarned) { + importAssertionAlreadyWarned = true; + process.emitWarning('Use `importAttributes` instead of `importAssertions`', 'ExperimentalWarning'); + } +} + +function defineImportAssertionAlias(context) { + return ObjectDefineProperty(context, 'importAssertions', { + __proto__: null, + configurable: true, + get() { + emitImportAssertionWarning(); + return this.importAttributes; + }, + set(value) { + emitImportAssertionWarning(); + return ReflectSet(this, 'importAttributes', value); + }, + }); +} + /** * @typedef {object} ExportedHooks * @property {Function} initialize Customizations setup hook. @@ -230,21 +255,21 @@ class Hooks { * @param {string} originalSpecifier The specified URL path of the module to * be resolved. * @param {string} [parentURL] The URL path of the module's parent. - * @param {ImportAssertions} [importAssertions] Assertions from the import + * @param {ImportAttributes} [importAttributes] Attributes from the import * statement or expression. * @returns {Promise<{ format: string, url: URL['href'] }>} */ async resolve( originalSpecifier, parentURL, - importAssertions = { __proto__: null }, + importAttributes = { __proto__: null }, ) { throwIfInvalidParentURL(parentURL); const chain = this.#chains.resolve; const context = { conditions: getDefaultConditions(), - importAssertions, + importAttributes, parentURL, }; const meta = { @@ -286,9 +311,9 @@ class Hooks { throw new ERR_LOADER_CHAIN_INCOMPLETE(hookErrIdentifier); } + let resolvedImportAttributes; const { format, - importAssertions: resolvedImportAssertions, url, } = resolution; @@ -318,15 +343,22 @@ class Hooks { this.#validatedUrls.add(url); } + if (!('importAttributes' in resolution) && ('importAssertions' in resolution)) { + emitImportAssertionWarning(); + resolvedImportAttributes = resolution.importAssertions; + } else { + resolvedImportAttributes = resolution.importAttributes; + } + if ( - resolvedImportAssertions != null && - typeof resolvedImportAssertions !== 'object' + resolvedImportAttributes != null && + typeof resolvedImportAttributes !== 'object' ) { throw new ERR_INVALID_RETURN_PROPERTY_VALUE( 'an object', hookErrIdentifier, - 'importAssertions', - resolvedImportAssertions, + 'importAttributes', + resolvedImportAttributes, ); } @@ -345,12 +377,12 @@ class Hooks { return { __proto__: null, format, - importAssertions: resolvedImportAssertions, + importAttributes: resolvedImportAttributes, url, }; } - resolveSync(_originalSpecifier, _parentURL, _importAssertions) { + resolveSync(_originalSpecifier, _parentURL, _importAttributes) { throw new ERR_METHOD_NOT_IMPLEMENTED('resolveSync()'); } @@ -413,7 +445,7 @@ class Hooks { const nextLoad = nextHookFactory(chain[chain.length - 1], meta, { validateArgs, validateOutput }); - const loaded = await nextLoad(url, context); + const loaded = await nextLoad(url, defineImportAssertionAlias(context)); const { hookErrIdentifier } = meta; // Retrieve the value after all settled validateOutput(hookErrIdentifier, loaded); diff --git a/lib/internal/modules/esm/load.js b/lib/internal/modules/esm/load.js index d064296d11c463..1881745a6d3134 100644 --- a/lib/internal/modules/esm/load.js +++ b/lib/internal/modules/esm/load.js @@ -8,7 +8,7 @@ const { const { kEmptyObject } = require('internal/util'); const { defaultGetFormat } = require('internal/modules/esm/get_format'); -const { validateAssertions } = require('internal/modules/esm/assert'); +const { validateAttributes, emitImportAssertionWarning } = require('internal/modules/esm/assert'); const { getOptionValue } = require('internal/options'); const { readFileSync } = require('fs'); @@ -107,19 +107,29 @@ function getSourceSync(url, context) { */ async function defaultLoad(url, context = kEmptyObject) { let responseURL = url; - const { importAssertions } = context; let { + importAttributes, format, source, } = context; + if (importAttributes == null && !('importAttributes' in context) && 'importAssertions' in context) { + emitImportAssertionWarning(); + importAttributes = context.importAssertions; + // Alias `importAssertions` to `importAttributes` + context = { + ...context, + importAttributes, + }; + } + const urlInstance = new URL(url); throwIfUnsupportedURLScheme(urlInstance, experimentalNetworkImports); format ??= await defaultGetFormat(urlInstance, context); - validateAssertions(url, format, importAssertions); + validateAttributes(url, format, importAttributes); if ( format === 'builtin' || @@ -141,7 +151,7 @@ async function defaultLoad(url, context = kEmptyObject) { * @typedef LoadContext * @property {string} [format] A hint (possibly returned from `resolve`) * @property {string | Buffer | ArrayBuffer} [source] source - * @property {Record<string, string>} [importAssertions] import attributes + * @property {Record<string, string>} [importAttributes] import attributes */ /** @@ -158,7 +168,7 @@ async function defaultLoad(url, context = kEmptyObject) { */ function defaultLoadSync(url, context = kEmptyObject) { let responseURL = url; - const { importAssertions } = context; + const { importAttributes } = context; let { format, source, @@ -170,7 +180,7 @@ function defaultLoadSync(url, context = kEmptyObject) { format ??= defaultGetFormat(urlInstance, context); - validateAssertions(url, format, importAssertions); + validateAttributes(url, format, importAttributes); if (format === 'builtin') { source = null; diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 0b08db7ffef7ec..6044765c3709f5 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -165,12 +165,12 @@ class ModuleLoader { * resolve( * originalSpecifier: * string, parentURL: string, - * importAssertions: Record<string, string> + * importAttributes: Record<string, string> * ): Promise<ResolveResult> * resolveSync( * originalSpecifier: * string, parentURL: string, - * importAssertions: Record<string, string> + * importAttributes: Record<string, string> * ) ResolveResult; * register(specifier: string, parentURL: string): any; * forceLoadHooks(): void; @@ -204,8 +204,8 @@ class ModuleLoader { registerModule(module, { __proto__: null, initializeImportMeta: (meta, wrap) => this.importMetaInitialize(meta, { url }), - importModuleDynamically: (specifier, { url }, importAssertions) => { - return this.import(specifier, url, importAssertions); + importModuleDynamically: (specifier, { url }, importAttributes) => { + return this.import(specifier, url, importAttributes); }, }); @@ -232,24 +232,24 @@ class ModuleLoader { * @param {string | undefined} parentURL The URL of the module importing this * one, unless this is the Node.js entry * point. - * @param {Record<string, string>} importAssertions Validations for the + * @param {Record<string, string>} importAttributes Validations for the * module import. * @returns {Promise<ModuleJob>} The (possibly pending) module job */ - async getModuleJob(specifier, parentURL, importAssertions) { - const resolveResult = await this.resolve(specifier, parentURL, importAssertions); - return this.getJobFromResolveResult(resolveResult, parentURL, importAssertions); + async getModuleJob(specifier, parentURL, importAttributes) { + const resolveResult = await this.resolve(specifier, parentURL, importAttributes); + return this.getJobFromResolveResult(resolveResult, parentURL, importAttributes); } - getModuleJobSync(specifier, parentURL, importAssertions) { - const resolveResult = this.resolveSync(specifier, parentURL, importAssertions); - return this.getJobFromResolveResult(resolveResult, parentURL, importAssertions, true); + getModuleJobSync(specifier, parentURL, importAttributes) { + const resolveResult = this.resolveSync(specifier, parentURL, importAttributes); + return this.getJobFromResolveResult(resolveResult, parentURL, importAttributes, true); } - getJobFromResolveResult(resolveResult, parentURL, importAssertions, sync) { + getJobFromResolveResult(resolveResult, parentURL, importAttributes, sync) { const { url, format } = resolveResult; - const resolvedImportAssertions = resolveResult.importAssertions ?? importAssertions; - let job = this.loadCache.get(url, resolvedImportAssertions.type); + const resolvedImportAttributes = resolveResult.importAttributes ?? importAttributes; + let job = this.loadCache.get(url, resolvedImportAttributes.type); // CommonJS will set functions for lazy job evaluation. if (typeof job === 'function') { @@ -257,7 +257,7 @@ class ModuleLoader { } if (job === undefined) { - job = this.#createModuleJob(url, resolvedImportAssertions, parentURL, format, sync); + job = this.#createModuleJob(url, resolvedImportAttributes, parentURL, format, sync); } return job; @@ -266,7 +266,7 @@ class ModuleLoader { /** * Create and cache an object representing a loaded module. * @param {string} url The absolute URL that was resolved for this module - * @param {Record<string, string>} importAssertions Validations for the + * @param {Record<string, string>} importAttributes Validations for the * module import. * @param {string} [parentURL] The absolute URL of the module importing this * one, unless this is the Node.js entry point @@ -274,7 +274,7 @@ class ModuleLoader { * `resolve` hook * @returns {Promise<ModuleJob>} The (possibly pending) module job */ - #createModuleJob(url, importAssertions, parentURL, format, sync) { + #createModuleJob(url, importAttributes, parentURL, format, sync) { const callTranslator = ({ format: finalFormat, responseURL, source }, isMain) => { const translator = getTranslators().get(finalFormat); @@ -284,7 +284,8 @@ class ModuleLoader { return FunctionPrototypeCall(translator, this, responseURL, source, isMain); }; - const context = { format, importAssertions }; + const context = { format, importAttributes }; + const moduleProvider = sync ? (url, isMain) => callTranslator(this.loadSync(url, context), isMain) : async (url, isMain) => callTranslator(await this.load(url, context), isMain); @@ -302,14 +303,14 @@ class ModuleLoader { const job = new ModuleJob( this, url, - importAssertions, + importAttributes, moduleProvider, parentURL === undefined, inspectBrk, sync, ); - this.loadCache.set(url, importAssertions.type, job); + this.loadCache.set(url, importAttributes.type, job); return job; } @@ -319,12 +320,12 @@ class ModuleLoader { * Use directly with caution. * @param {string} specifier The first parameter of an `import()` expression. * @param {string} parentURL Path of the parent importing the module. - * @param {Record<string, string>} importAssertions Validations for the + * @param {Record<string, string>} importAttributes Validations for the * module import. * @returns {Promise<ModuleExports>} */ - async import(specifier, parentURL, importAssertions) { - const moduleJob = await this.getModuleJob(specifier, parentURL, importAssertions); + async import(specifier, parentURL, importAttributes) { + const moduleJob = await this.getModuleJob(specifier, parentURL, importAttributes); const { module } = await moduleJob.run(); return module.getNamespace(); } @@ -348,20 +349,20 @@ class ModuleLoader { * @param {string} originalSpecifier The specified URL path of the module to * be resolved. * @param {string} [parentURL] The URL path of the module's parent. - * @param {ImportAssertions} importAssertions Assertions from the import + * @param {ImportAttributes} importAttributes Attributes from the import * statement or expression. * @returns {{ format: string, url: URL['href'] }} */ - resolve(originalSpecifier, parentURL, importAssertions) { + resolve(originalSpecifier, parentURL, importAttributes) { if (this.#customizations) { - return this.#customizations.resolve(originalSpecifier, parentURL, importAssertions); + return this.#customizations.resolve(originalSpecifier, parentURL, importAttributes); } - const requestKey = this.#resolveCache.serializeKey(originalSpecifier, importAssertions); + const requestKey = this.#resolveCache.serializeKey(originalSpecifier, importAttributes); const cachedResult = this.#resolveCache.get(requestKey, parentURL); if (cachedResult != null) { return cachedResult; } - const result = this.defaultResolve(originalSpecifier, parentURL, importAssertions); + const result = this.defaultResolve(originalSpecifier, parentURL, importAttributes); this.#resolveCache.set(requestKey, parentURL, result); return result; } @@ -370,11 +371,11 @@ class ModuleLoader { * Just like `resolve` except synchronous. This is here specifically to support * `import.meta.resolve` which must happen synchronously. */ - resolveSync(originalSpecifier, parentURL, importAssertions) { + resolveSync(originalSpecifier, parentURL, importAttributes) { if (this.#customizations) { - return this.#customizations.resolveSync(originalSpecifier, parentURL, importAssertions); + return this.#customizations.resolveSync(originalSpecifier, parentURL, importAttributes); } - return this.defaultResolve(originalSpecifier, parentURL, importAssertions); + return this.defaultResolve(originalSpecifier, parentURL, importAttributes); } /** @@ -382,13 +383,13 @@ class ModuleLoader { * `resolve` and `resolveSync`. This function is here just to avoid * repeating the same code block twice in those functions. */ - defaultResolve(originalSpecifier, parentURL, importAssertions) { + defaultResolve(originalSpecifier, parentURL, importAttributes) { defaultResolve ??= require('internal/modules/esm/resolve').defaultResolve; const context = { __proto__: null, conditions: this.#defaultConditions, - importAssertions, + importAttributes, parentURL, }; @@ -483,17 +484,17 @@ class CustomizedModuleLoader { * @param {string} originalSpecifier The specified URL path of the module to * be resolved. * @param {string} [parentURL] The URL path of the module's parent. - * @param {ImportAssertions} importAssertions Assertions from the import + * @param {ImportAttributes} importAttributes Attributes from the import * statement or expression. * @returns {{ format: string, url: URL['href'] }} */ - resolve(originalSpecifier, parentURL, importAssertions) { - return hooksProxy.makeAsyncRequest('resolve', undefined, originalSpecifier, parentURL, importAssertions); + resolve(originalSpecifier, parentURL, importAttributes) { + return hooksProxy.makeAsyncRequest('resolve', undefined, originalSpecifier, parentURL, importAttributes); } - resolveSync(originalSpecifier, parentURL, importAssertions) { + resolveSync(originalSpecifier, parentURL, importAttributes) { // This happens only as a result of `import.meta.resolve` calls, which must be sync per spec. - return hooksProxy.makeSyncRequest('resolve', undefined, originalSpecifier, parentURL, importAssertions); + return hooksProxy.makeSyncRequest('resolve', undefined, originalSpecifier, parentURL, importAttributes); } /** diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 8ac5ff240bdb37..77f2909a262f4c 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -50,10 +50,10 @@ const isCommonJSGlobalLikeNotDefinedError = (errorMessage) => class ModuleJob { // `loader` is the Loader instance used for loading dependencies. // `moduleProvider` is a function - constructor(loader, url, importAssertions = { __proto__: null }, + constructor(loader, url, importAttributes = { __proto__: null }, moduleProvider, isMain, inspectBrk, sync = false) { this.loader = loader; - this.importAssertions = importAssertions; + this.importAttributes = importAttributes; this.isMain = isMain; this.inspectBrk = inspectBrk; @@ -81,8 +81,8 @@ class ModuleJob { // so that circular dependencies can't cause a deadlock by two of // these `link` callbacks depending on each other. const dependencyJobs = []; - const promises = this.module.link(async (specifier, assertions) => { - const job = await this.loader.getModuleJob(specifier, url, assertions); + const promises = this.module.link(async (specifier, attributes) => { + const job = await this.loader.getModuleJob(specifier, url, attributes); ArrayPrototypePush(dependencyJobs, job); return job.modulePromise; }); @@ -156,7 +156,7 @@ class ModuleJob { let format; try { // This might throw for non-CommonJS modules because we aren't passing - // in the import assertions and some formats require them; but we only + // in the import attributes and some formats require them; but we only // care about CommonJS for the purposes of this error message. ({ format } = await this.loader.load(childFileURL)); diff --git a/lib/internal/modules/esm/module_map.js b/lib/internal/modules/esm/module_map.js index 12a1a526178a7e..eab00386c413a5 100644 --- a/lib/internal/modules/esm/module_map.js +++ b/lib/internal/modules/esm/module_map.js @@ -17,7 +17,7 @@ const { validateString } = require('internal/validators'); /** * Cache the results of the `resolve` step of the module resolution and loading process. - * Future resolutions of the same input (specifier, parent URL and import assertions) + * Future resolutions of the same input (specifier, parent URL and import attributes) * must return the same result if the first attempt was successful, per * https://tc39.es/ecma262/#sec-HostLoadImportedModule. * This cache is *not* used when custom loaders are registered. @@ -30,15 +30,15 @@ class ResolveCache extends SafeMap { * * It is exposed to allow more efficient read and overwrite a cache entry. * @param {string} specifier - * @param {Record<string,string>} importAssertions + * @param {Record<string,string>} importAttributes * @returns {string} */ - serializeKey(specifier, importAssertions) { - // To serialize the ModuleRequest (specifier + list of import assertions), - // we need to sort the assertions by key, then stringifying, - // so that different import statements with the same assertions are always treated + serializeKey(specifier, importAttributes) { + // To serialize the ModuleRequest (specifier + list of import attributes), + // we need to sort the attributes by key, then stringifying, + // so that different import statements with the same attributes are always treated // as identical. - const keys = ObjectKeys(importAssertions); + const keys = ObjectKeys(importAttributes); if (keys.length === 0) { return specifier + '::'; @@ -47,7 +47,7 @@ class ResolveCache extends SafeMap { return specifier + '::' + ArrayPrototypeJoin( ArrayPrototypeMap( ArrayPrototypeSort(keys), - (key) => JSONStringify(key) + JSONStringify(importAssertions[key])), + (key) => JSONStringify(key) + JSONStringify(importAttributes[key])), ','); } diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index c9f7845a664cb7..6ae85326fc705c 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -138,11 +138,11 @@ function errPath(url) { * @param {string} specifier - The module specifier to import. * @param {object} options - An object containing options for the import. * @param {string} options.url - The URL of the module requesting the import. - * @param {Record<string, string>} [assertions] - An object containing assertions for the import. + * @param {Record<string, string>} [attributes] - An object containing attributes for the import. * @returns {Promise<import('internal/modules/esm/loader.js').ModuleExports>} The imported module. */ -async function importModuleDynamically(specifier, { url }, assertions) { - return asyncESM.esmLoader.import(specifier, url, assertions); +async function importModuleDynamically(specifier, { url }, attributes) { + return asyncESM.esmLoader.import(specifier, url, attributes); } // Strategy for loading a standard JavaScript module. @@ -201,8 +201,8 @@ function loadCJSModule(module, source, url, filename) { '__dirname', ], { filename, - importModuleDynamically(specifier, _, importAssertions) { - return asyncESM.esmLoader.import(specifier, url, importAssertions); + importModuleDynamically(specifier, _, importAttributes) { + return asyncESM.esmLoader.import(specifier, url, importAttributes); }, }).function; } catch (err) { @@ -213,14 +213,14 @@ function loadCJSModule(module, source, url, filename) { const __dirname = dirname(filename); // eslint-disable-next-line func-name-matching,func-style const requireFn = function require(specifier) { - let importAssertions = kEmptyObject; + let importAttributes = kEmptyObject; if (!StringPrototypeStartsWith(specifier, 'node:')) { // TODO: do not depend on the monkey-patchable CJS loader here. const path = CJSModule._resolveFilename(specifier, module); if (specifier !== path) { switch (extname(path)) { case '.json': - importAssertions = { __proto__: null, type: 'json' }; + importAttributes = { __proto__: null, type: 'json' }; break; case '.node': return CJSModule._load(specifier, module); @@ -230,7 +230,7 @@ function loadCJSModule(module, source, url, filename) { specifier = `${pathToFileURL(path)}`; } } - const job = asyncESM.esmLoader.getModuleJobSync(specifier, url, importAssertions); + const job = asyncESM.esmLoader.getModuleJobSync(specifier, url, importAttributes); job.runSync(); return cjsCache.get(job.url).exports; }; diff --git a/lib/internal/process/execution.js b/lib/internal/process/execution.js index 4b77aa47c2cb35..8ae6a1678af1b5 100644 --- a/lib/internal/process/execution.js +++ b/lib/internal/process/execution.js @@ -84,9 +84,9 @@ function evalScript(name, body, breakFirstLine, print, shouldLoadESM = false) { filename: name, displayErrors: true, [kVmBreakFirstLineSymbol]: !!breakFirstLine, - importModuleDynamically(specifier, _, importAssertions) { + importModuleDynamically(specifier, _, importAttributes) { const loader = asyncESM.esmLoader; - return loader.import(specifier, baseUrl, importAssertions); + return loader.import(specifier, baseUrl, importAttributes); }, })); if (print) { diff --git a/lib/repl.js b/lib/repl.js index 931ae5087c8fb3..52f3026414d72d 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -483,9 +483,9 @@ function REPLServer(prompt, vm.createScript(fallbackCode, { filename: file, displayErrors: true, - importModuleDynamically: (specifier, _, importAssertions) => { + importModuleDynamically: (specifier, _, importAttributes) => { return asyncESM.esmLoader.import(specifier, parentURL, - importAssertions); + importAttributes); }, }); } catch (fallbackError) { @@ -527,9 +527,9 @@ function REPLServer(prompt, script = vm.createScript(code, { filename: file, displayErrors: true, - importModuleDynamically: (specifier, _, importAssertions) => { + importModuleDynamically: (specifier, _, importAttributes) => { return asyncESM.esmLoader.import(specifier, parentURL, - importAssertions); + importAttributes); }, }); } catch (e) { diff --git a/src/module_wrap.cc b/src/module_wrap.cc index a1b0f812391486..a52b939fdd596c 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -249,19 +249,19 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) { args.GetReturnValue().Set(that); } -static Local<Object> createImportAssertionContainer(Environment* env, - Isolate* isolate, Local<FixedArray> raw_assertions) { - Local<Object> assertions = - Object::New(isolate, v8::Null(env->isolate()), nullptr, nullptr, 0); - for (int i = 0; i < raw_assertions->Length(); i += 3) { - assertions - ->Set(env->context(), - raw_assertions->Get(env->context(), i).As<String>(), - raw_assertions->Get(env->context(), i + 1).As<Value>()) - .ToChecked(); - } - - return assertions; +static Local<Object> createImportAttributesContainer( + Environment* env, Isolate* isolate, Local<FixedArray> raw_attributes) { + Local<Object> attributes = + Object::New(isolate, v8::Null(env->isolate()), nullptr, nullptr, 0); + for (int i = 0; i < raw_attributes->Length(); i += 3) { + attributes + ->Set(env->context(), + raw_attributes->Get(env->context(), i).As<String>(), + raw_attributes->Get(env->context(), i + 1).As<Value>()) + .ToChecked(); + } + + return attributes; } void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) { @@ -297,13 +297,13 @@ void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) { Utf8Value specifier_utf8(env->isolate(), specifier); std::string specifier_std(*specifier_utf8, specifier_utf8.length()); - Local<FixedArray> raw_assertions = module_request->GetImportAssertions(); - Local<Object> assertions = - createImportAssertionContainer(env, isolate, raw_assertions); + Local<FixedArray> raw_attributes = module_request->GetImportAssertions(); + Local<Object> attributes = + createImportAttributesContainer(env, isolate, raw_attributes); Local<Value> argv[] = { specifier, - assertions, + attributes, }; MaybeLocal<Value> maybe_resolve_return_value = @@ -499,7 +499,7 @@ void ModuleWrap::GetError(const FunctionCallbackInfo<Value>& args) { MaybeLocal<Module> ModuleWrap::ResolveModuleCallback( Local<Context> context, Local<String> specifier, - Local<FixedArray> import_assertions, + Local<FixedArray> import_attributes, Local<Module> referrer) { Environment* env = Environment::GetCurrent(context); if (env == nullptr) { @@ -552,7 +552,7 @@ static MaybeLocal<Promise> ImportModuleDynamically( Local<v8::Data> host_defined_options, Local<Value> resource_name, Local<String> specifier, - Local<FixedArray> import_assertions) { + Local<FixedArray> import_attributes) { Isolate* isolate = context->GetIsolate(); Environment* env = Environment::GetCurrent(context); if (env == nullptr) { @@ -582,13 +582,13 @@ static MaybeLocal<Promise> ImportModuleDynamically( Local<Symbol> id = options->Get(context, HostDefinedOptions::kID).As<Symbol>(); - Local<Object> assertions = - createImportAssertionContainer(env, isolate, import_assertions); + Local<Object> attributes = + createImportAttributesContainer(env, isolate, import_attributes); Local<Value> import_args[] = { id, Local<Value>(specifier), - assertions, + attributes, }; Local<Value> result; diff --git a/src/module_wrap.h b/src/module_wrap.h index 6435bad40936fe..1fc801edced9c5 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -97,7 +97,7 @@ class ModuleWrap : public BaseObject { static v8::MaybeLocal<v8::Module> ResolveModuleCallback( v8::Local<v8::Context> context, v8::Local<v8::String> specifier, - v8::Local<v8::FixedArray> import_assertions, + v8::Local<v8::FixedArray> import_attributes, v8::Local<v8::Module> referrer); static ModuleWrap* GetFromModule(node::Environment*, v8::Local<v8::Module>); diff --git a/test/es-module/test-esm-dynamic-import-assertion.js b/test/es-module/test-esm-dynamic-import-attribute.js similarity index 100% rename from test/es-module/test-esm-dynamic-import-assertion.js rename to test/es-module/test-esm-dynamic-import-attribute.js diff --git a/test/es-module/test-esm-dynamic-import-assertion.mjs b/test/es-module/test-esm-dynamic-import-attribute.mjs similarity index 100% rename from test/es-module/test-esm-dynamic-import-assertion.mjs rename to test/es-module/test-esm-dynamic-import-attribute.mjs diff --git a/test/es-module/test-esm-import-assertion-2.mjs b/test/es-module/test-esm-import-assertion-2.mjs deleted file mode 100644 index 8001c29772b1f0..00000000000000 --- a/test/es-module/test-esm-import-assertion-2.mjs +++ /dev/null @@ -1,6 +0,0 @@ -import '../common/index.mjs'; -import { strictEqual } from 'assert'; - -import secret from '../fixtures/experimental.json' assert { type: 'json', unsupportedAssertion: 'should ignore' }; - -strictEqual(secret.ofLife, 42); diff --git a/test/es-module/test-esm-import-assertion-validation.js b/test/es-module/test-esm-import-assertion-validation.js deleted file mode 100644 index ec2d2a2c08f7b5..00000000000000 --- a/test/es-module/test-esm-import-assertion-validation.js +++ /dev/null @@ -1,45 +0,0 @@ -// Flags: --expose-internals -'use strict'; -const common = require('../common'); - -const assert = require('assert'); - -const { validateAssertions } = require('internal/modules/esm/assert'); - -common.expectWarning( - 'ExperimentalWarning', - 'Import assertions are not a stable feature of the JavaScript language. ' + - 'Avoid relying on their current behavior and syntax as those might change ' + - 'in a future version of Node.js.' -); - - -const url = 'test://'; - -assert.ok(validateAssertions(url, 'builtin', {})); -assert.ok(validateAssertions(url, 'commonjs', {})); -assert.ok(validateAssertions(url, 'json', { type: 'json' })); -assert.ok(validateAssertions(url, 'module', {})); -assert.ok(validateAssertions(url, 'wasm', {})); - -assert.throws(() => validateAssertions(url, 'json', {}), { - code: 'ERR_IMPORT_ASSERTION_TYPE_MISSING', -}); - -assert.throws(() => validateAssertions(url, 'module', { type: 'json' }), { - code: 'ERR_IMPORT_ASSERTION_TYPE_FAILED', -}); - -// The HTML spec specifically disallows this for now, while Wasm module import -// and whether it will require a type assertion is still an open question. -assert.throws(() => validateAssertions(url, 'module', { type: 'javascript' }), { - code: 'ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED', -}); - -assert.throws(() => validateAssertions(url, 'module', { type: 'css' }), { - code: 'ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED', -}); - -assert.throws(() => validateAssertions(url, 'module', { type: false }), { - code: 'ERR_INVALID_ARG_TYPE', -}); diff --git a/test/es-module/test-esm-import-assertion-warning.mjs b/test/es-module/test-esm-import-assertion-warning.mjs index 0b18d8ff9eaf62..a11b5164cebffc 100644 --- a/test/es-module/test-esm-import-assertion-warning.mjs +++ b/test/es-module/test-esm-import-assertion-warning.mjs @@ -1,10 +1,37 @@ -import { expectWarning } from '../common/index.mjs'; +import { spawnPromisified } from '../common/index.mjs'; +import assert from 'node:assert'; +import { execPath } from 'node:process'; -expectWarning( - 'ExperimentalWarning', - 'Import assertions are not a stable feature of the JavaScript language. ' + - 'Avoid relying on their current behavior and syntax as those might change ' + - 'in a future version of Node.js.' -); +await Promise.all([ + // Using importAssertions in the resolve hook should warn but still work. + `data:text/javascript,export ${encodeURIComponent(function resolve() { + return { shortCircuit: true, url: 'data:application/json,1', importAssertions: { type: 'json' } }; + })}`, + // Setting importAssertions on the context object of the load hook should warn but still work. + `data:text/javascript,export ${encodeURIComponent(function load(u, c, n) { + c.importAssertions = { type: 'json' }; + return n('data:application/json,1', c); + })}`, + // Creating a new context object with importAssertions in the load hook should warn but still work. + `data:text/javascript,export ${encodeURIComponent(function load(u, c, n) { + return n('data:application/json,1', { importAssertions: { type: 'json' } }); + })}`, +].map(async (loaderURL) => { + const { stdout, stderr, code } = await spawnPromisified(execPath, [ + '--input-type=module', + '--eval', ` + import assert from 'node:assert'; + import { register } from 'node:module'; + + register(${JSON.stringify(loaderURL)}); + + assert.deepStrictEqual( + { ...await import('data:') }, + { default: 1 } + );`, + ]); -await import('data:text/javascript,', { assert: { someUnsupportedKey: 'value' } }); + assert.match(stderr, /Use `importAttributes` instead of `importAssertions`/); + assert.strictEqual(stdout, ''); + assert.strictEqual(code, 0); +})); diff --git a/test/es-module/test-esm-import-assertion-1.mjs b/test/es-module/test-esm-import-attributes-1.mjs similarity index 100% rename from test/es-module/test-esm-import-assertion-1.mjs rename to test/es-module/test-esm-import-attributes-1.mjs diff --git a/test/es-module/test-esm-import-assertion-4.mjs b/test/es-module/test-esm-import-attributes-2.mjs similarity index 100% rename from test/es-module/test-esm-import-assertion-4.mjs rename to test/es-module/test-esm-import-attributes-2.mjs diff --git a/test/es-module/test-esm-import-assertion-3.mjs b/test/es-module/test-esm-import-attributes-3.mjs similarity index 100% rename from test/es-module/test-esm-import-assertion-3.mjs rename to test/es-module/test-esm-import-attributes-3.mjs diff --git a/test/es-module/test-esm-import-assertion-errors.js b/test/es-module/test-esm-import-attributes-errors.js similarity index 84% rename from test/es-module/test-esm-import-assertion-errors.js rename to test/es-module/test-esm-import-attributes-errors.js index e2abd3fb43976d..9cf5ce33693ce5 100644 --- a/test/es-module/test-esm-import-assertion-errors.js +++ b/test/es-module/test-esm-import-attributes-errors.js @@ -5,19 +5,17 @@ const { rejects } = require('assert'); const jsModuleDataUrl = 'data:text/javascript,export{}'; const jsonModuleDataUrl = 'data:application/json,""'; -common.expectWarning( - 'ExperimentalWarning', - 'Import assertions are not a stable feature of the JavaScript language. ' + - 'Avoid relying on their current behavior and syntax as those might change ' + - 'in a future version of Node.js.' -); - async function test() { await rejects( import('data:text/css,', { assert: { type: 'css' } }), { code: 'ERR_UNKNOWN_MODULE_FORMAT' } ); + await rejects( + import('data:text/css,', { assert: { unsupportedAttribute: 'value' } }), + { code: 'ERR_IMPORT_ATTRIBUTE_UNSUPPORTED' } + ); + await rejects( import(`data:text/javascript,import${JSON.stringify(jsModuleDataUrl)}assert{type:"json"}`), { code: 'ERR_IMPORT_ASSERTION_TYPE_FAILED' } diff --git a/test/es-module/test-esm-import-assertion-errors.mjs b/test/es-module/test-esm-import-attributes-errors.mjs similarity index 81% rename from test/es-module/test-esm-import-assertion-errors.mjs rename to test/es-module/test-esm-import-attributes-errors.mjs index 9cc08c06528fc6..acaeef50626508 100644 --- a/test/es-module/test-esm-import-assertion-errors.mjs +++ b/test/es-module/test-esm-import-attributes-errors.mjs @@ -1,17 +1,9 @@ -import { expectWarning } from '../common/index.mjs'; +import '../common/index.mjs'; import { rejects } from 'assert'; const jsModuleDataUrl = 'data:text/javascript,export{}'; const jsonModuleDataUrl = 'data:application/json,""'; -expectWarning( - 'ExperimentalWarning', - 'Import assertions are not a stable feature of the JavaScript language. ' + - 'Avoid relying on their current behavior and syntax as those might change ' + - 'in a future version of Node.js.' -); - - await rejects( // This rejects because of the unsupported MIME type, not because of the // unsupported assertion. diff --git a/test/es-module/test-esm-import-attributes-validation.js b/test/es-module/test-esm-import-attributes-validation.js new file mode 100644 index 00000000000000..f436f7073126d7 --- /dev/null +++ b/test/es-module/test-esm-import-attributes-validation.js @@ -0,0 +1,45 @@ +// Flags: --expose-internals +'use strict'; +require('../common'); + +const assert = require('assert'); + +const { validateAttributes } = require('internal/modules/esm/assert'); + +const url = 'test://'; + +assert.ok(validateAttributes(url, 'builtin', {})); +assert.ok(validateAttributes(url, 'commonjs', {})); +assert.ok(validateAttributes(url, 'json', { type: 'json' })); +assert.ok(validateAttributes(url, 'module', {})); +assert.ok(validateAttributes(url, 'wasm', {})); + +assert.throws(() => validateAttributes(url, 'json', {}), { + code: 'ERR_IMPORT_ASSERTION_TYPE_MISSING', +}); + +assert.throws(() => validateAttributes(url, 'json', { type: 'json', unsupportedAttribute: 'value' }), { + code: 'ERR_IMPORT_ATTRIBUTE_UNSUPPORTED', +}); + +assert.throws(() => validateAttributes(url, 'module', { unsupportedAttribute: 'value' }), { + code: 'ERR_IMPORT_ATTRIBUTE_UNSUPPORTED', +}); + +assert.throws(() => validateAttributes(url, 'module', { type: 'json' }), { + code: 'ERR_IMPORT_ASSERTION_TYPE_FAILED', +}); + +// The HTML spec specifically disallows this for now, while Wasm module import +// and whether it will require a type assertion is still an open question. +assert.throws(() => validateAttributes(url, 'module', { type: 'javascript' }), { + code: 'ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED', +}); + +assert.throws(() => validateAttributes(url, 'module', { type: 'css' }), { + code: 'ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED', +}); + +assert.throws(() => validateAttributes(url, 'module', { type: false }), { + code: 'ERR_INVALID_ARG_TYPE', +}); diff --git a/test/es-module/test-esm-json.mjs b/test/es-module/test-esm-json.mjs index 82232838b79150..61d58e94d95e93 100644 --- a/test/es-module/test-esm-json.mjs +++ b/test/es-module/test-esm-json.mjs @@ -20,7 +20,6 @@ describe('ESM: importing JSON', () => { ]); assert.match(stderr, /ExperimentalWarning: Importing JSON modules/); - assert.match(stderr, /ExperimentalWarning: Import assertions/); assert.strictEqual(code, 0); assert.strictEqual(signal, null); }); diff --git a/test/es-module/test-esm-virtual-json.mjs b/test/es-module/test-esm-virtual-json.mjs index 8ff185a428ef01..8876eea013ced8 100644 --- a/test/es-module/test-esm-virtual-json.mjs +++ b/test/es-module/test-esm-virtual-json.mjs @@ -12,7 +12,7 @@ async function resolve(referrer, context, next) { } function load(url, context, next) { - if (context.importAssertions.type === 'json') { + if (context.importAttributes.type === 'json') { return { shortCircuit: true, format: 'json', diff --git a/test/fixtures/es-module-loaders/assertionless-json-import.mjs b/test/fixtures/es-module-loaders/assertionless-json-import.mjs index e7e7b20fd26798..3ffc4c1148fe69 100644 --- a/test/fixtures/es-module-loaders/assertionless-json-import.mjs +++ b/test/fixtures/es-module-loaders/assertionless-json-import.mjs @@ -2,20 +2,20 @@ const DATA_URL_PATTERN = /^data:application\/json(?:[^,]*?)(;base64)?,([\s\S]*)$ const JSON_URL_PATTERN = /^[^?]+\.json(\?[^#]*)?(#.*)?$/; export async function resolve(specifier, context, next) { - const noAssertionSpecified = context.importAssertions.type == null; + const noAttributesSpecified = context.importAttributes.type == null; // Mutation from resolve hook should be discarded. - context.importAssertions.type = 'whatever'; + context.importAttributes.type = 'whatever'; - // This fixture assumes that no other resolve hooks in the chain will error on invalid import assertions + // This fixture assumes that no other resolve hooks in the chain will error on invalid import attributes // (as defaultResolve doesn't). const result = await next(specifier, context); - if (noAssertionSpecified && + if (noAttributesSpecified && (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), + // Clean new import attributes object to ensure that this test isn't passing due to mutation. + result.importAttributes = { + ...(result.importAttributes ?? context.importAttributes), type: 'json', }; } diff --git a/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs b/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs index 38fa0b3a7488aa..fca31c585a6ea9 100644 --- a/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs +++ b/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs @@ -14,7 +14,7 @@ export async function resolve(specifier, context, next) { return { shortCircuit: true, url: `custom-${def.url}`, - importAssertions: context.importAssertions, + importAttributes: context.importAttributes, }; } return def; diff --git a/test/fixtures/es-module-loaders/hooks-custom.mjs b/test/fixtures/es-module-loaders/hooks-custom.mjs index ea2ffaf7e97070..5656f95232b856 100644 --- a/test/fixtures/es-module-loaders/hooks-custom.mjs +++ b/test/fixtures/es-module-loaders/hooks-custom.mjs @@ -6,7 +6,7 @@ import count from '../es-modules/stateful.mjs'; // used to assert node-land and user-land have different contexts count(); -export function resolve(specifier, { importAssertions }, next) { +export function resolve(specifier, { importAttributes }, next) { let format = ''; if (specifier === 'esmHook/format.false') { @@ -24,7 +24,7 @@ export function resolve(specifier, { importAssertions }, next) { format, shortCircuit: true, url: pathToFileURL(specifier).href, - importAssertions, + importAttributes, }; } diff --git a/test/fixtures/es-module-loaders/hooks-input.mjs b/test/fixtures/es-module-loaders/hooks-input.mjs index ee0a468cc0e8d1..1d3759f458224e 100644 --- a/test/fixtures/es-module-loaders/hooks-input.mjs +++ b/test/fixtures/es-module-loaders/hooks-input.mjs @@ -22,12 +22,12 @@ export async function resolve(specifier, context, next) { assert.strictEqual(context.parentURL, undefined); } - assert.deepStrictEqual(context.importAssertions, {}); + assert.deepStrictEqual(context.importAttributes, {}); } else if (resolveCalls === 2) { url = new URL(specifier, context.parentURL).href; assert.match(specifier, /experimental\.json$/); assert.match(context.parentURL, /json-modules\.mjs$/); - assert.deepStrictEqual(context.importAssertions, { + assert.deepStrictEqual(context.importAttributes, { type: 'json', }); } @@ -35,7 +35,7 @@ export async function resolve(specifier, context, next) { // Ensure `context` has all and only the properties it's supposed to assert.deepStrictEqual(Reflect.ownKeys(context), [ 'conditions', - 'importAssertions', + 'importAttributes', 'parentURL', ]); assert.ok(Array.isArray(context.conditions)); @@ -59,11 +59,11 @@ export async function load(url, context, next) { if (loadCalls === 1) { assert.match(url, /json-modules\.mjs$/); - assert.deepStrictEqual(context.importAssertions, {}); + assert.deepStrictEqual(context.importAttributes, {}); format = 'module'; } else if (loadCalls === 2) { assert.match(url, /experimental\.json$/); - assert.deepStrictEqual(context.importAssertions, { + assert.deepStrictEqual(context.importAttributes, { type: 'json', }); format = 'json'; @@ -73,7 +73,7 @@ export async function load(url, context, next) { // Ensure `context` has all and only the properties it's supposed to assert.deepStrictEqual(Object.keys(context), [ 'format', - 'importAssertions', + 'importAttributes', ]); assert.strictEqual(context.format, 'test'); assert.strictEqual(typeof next, 'function'); diff --git a/test/fixtures/es-module-loaders/loader-invalid-format.mjs b/test/fixtures/es-module-loaders/loader-invalid-format.mjs index e7dd06c108ba1d..dc61a792b2be8b 100644 --- a/test/fixtures/es-module-loaders/loader-invalid-format.mjs +++ b/test/fixtures/es-module-loaders/loader-invalid-format.mjs @@ -1,4 +1,4 @@ -export async function resolve(specifier, { parentURL, importAssertions }, next) { +export async function resolve(specifier, { parentURL, importAttributes }, next) { if (parentURL && specifier === '../fixtures/es-modules/test-esm-ok.mjs') { return { shortCircuit: true, diff --git a/test/fixtures/es-module-loaders/loader-invalid-url.mjs b/test/fixtures/es-module-loaders/loader-invalid-url.mjs index a54f39521f29ac..aac2b16b6f58fe 100644 --- a/test/fixtures/es-module-loaders/loader-invalid-url.mjs +++ b/test/fixtures/es-module-loaders/loader-invalid-url.mjs @@ -1,9 +1,9 @@ -export async function resolve(specifier, { parentURL, importAssertions }, next) { +export async function resolve(specifier, { parentURL, importAttributes }, next) { if (parentURL && specifier === '../fixtures/es-modules/test-esm-ok.mjs') { return { shortCircuit: true, url: specifier, - importAssertions, + importAttributes, }; } return next(specifier); diff --git a/test/fixtures/es-module-loaders/loader-with-dep.mjs b/test/fixtures/es-module-loaders/loader-with-dep.mjs index 1b5fd6c3c1642a..625341eaed7eb2 100644 --- a/test/fixtures/es-module-loaders/loader-with-dep.mjs +++ b/test/fixtures/es-module-loaders/loader-with-dep.mjs @@ -3,9 +3,9 @@ import {createRequire} from '../../common/index.mjs'; const require = createRequire(import.meta.url); const dep = require('./loader-dep.js'); -export async function resolve(specifier, { parentURL, importAssertions }, defaultResolve) { +export async function resolve(specifier, { parentURL, importAttributes }, defaultResolve) { return { - url: (await defaultResolve(specifier, { parentURL, importAssertions }, defaultResolve)).url, + url: (await defaultResolve(specifier, { parentURL, importAttributes }, defaultResolve)).url, format: dep.format }; } diff --git a/test/fixtures/es-module-loaders/not-found-assert-loader.mjs b/test/fixtures/es-module-loaders/not-found-assert-loader.mjs index 2486899ea3753f..bf66efbd0810e5 100644 --- a/test/fixtures/es-module-loaders/not-found-assert-loader.mjs +++ b/test/fixtures/es-module-loaders/not-found-assert-loader.mjs @@ -3,7 +3,7 @@ import assert from 'node:assert'; // A loader that asserts that the defaultResolve will throw "not found" // (skipping the top-level main of course, and the built-in ones needed for run-worker). let mainLoad = true; -export async function resolve(specifier, { importAssertions }, next) { +export async function resolve(specifier, { importAttributes }, next) { if (mainLoad || specifier === 'path' || specifier === 'worker_threads') { mainLoad = false; return next(specifier); @@ -11,6 +11,6 @@ export async function resolve(specifier, { importAssertions }, next) { await assert.rejects(next(specifier), { code: 'ERR_MODULE_NOT_FOUND' }); return { url: 'node:fs', - importAssertions, + importAttributes, }; } diff --git a/test/fixtures/es-module-loaders/string-sources.mjs b/test/fixtures/es-module-loaders/string-sources.mjs index 396d17cb17a75c..39ad32c465fa31 100644 --- a/test/fixtures/es-module-loaders/string-sources.mjs +++ b/test/fixtures/es-module-loaders/string-sources.mjs @@ -23,7 +23,7 @@ const SOURCES = { export function resolve(specifier, context, next) { if (specifier.startsWith('test:')) { return { - importAssertions: context.importAssertions, + importAttributes: context.importAttributes, shortCircuit: true, url: specifier, }; diff --git a/test/parallel/test-vm-module-dynamic-import.js b/test/parallel/test-vm-module-dynamic-import.js index cd318511401412..5bca08b8c9c3bb 100644 --- a/test/parallel/test-vm-module-dynamic-import.js +++ b/test/parallel/test-vm-module-dynamic-import.js @@ -59,10 +59,10 @@ async function test() { { const s = new Script('import("foo", { assert: { key: "value" } })', { - importModuleDynamically: common.mustCall((specifier, wrap, assertion) => { + importModuleDynamically: common.mustCall((specifier, wrap, attributes) => { assert.strictEqual(specifier, 'foo'); assert.strictEqual(wrap, s); - assert.deepStrictEqual(assertion, { __proto__: null, key: 'value' }); + assert.deepStrictEqual(attributes, { __proto__: null, key: 'value' }); return foo; }), });