From 442ad8d787484136116658761738face0f2f7dcf Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Tue, 9 Nov 2021 23:23:47 -0800 Subject: [PATCH] module: import assertions improvements PR-URL: https://github.com/nodejs/node/pull/40785 Backport-PR-URL: https://github.com/nodejs/node/pull/41776 Reviewed-By: James M Snell Reviewed-By: Bradley Farias Reviewed-By: Myles Borins Reviewed-By: Guy Bedford Reviewed-By: Antoine du Hamel --- doc/api/esm.md | 14 +++++-- lib/internal/modules/esm/assert.js | 38 +++++++++++-------- lib/internal/modules/esm/module_map.js | 9 ++--- .../test-esm-import-assertion-errors.js | 2 +- .../test-esm-import-assertion-validation.js | 4 +- test/es-module/test-esm-loader-modulemap.js | 5 +-- 6 files changed, 41 insertions(+), 31 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 71873519eefb02..4c784da560fbaf 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -225,6 +225,8 @@ import fs from 'node:fs/promises'; added: REPLACEME --> +> Stability: 1 - Experimental + The [Import Assertions proposal][] adds an inline syntax for module import statements to pass on more information alongside the module specifier. @@ -235,11 +237,12 @@ const { default: barData } = await import('./bar.json', { assert: { type: 'json' } }); ``` -Node.js supports the following `type` values: +Node.js supports the following `type` values, for which the assertion is +mandatory: -| `type` | Resolves to | -| -------- | ---------------- | -| `'json'` | [JSON modules][] | +| Assertion `type` | Needed for | +| ---------------- | ---------------- | +| `'json'` | [JSON modules][] | ## Builtin modules @@ -548,6 +551,8 @@ node index.mjs # fails node --experimental-json-modules index.mjs # works ``` +The `assert { type: 'json' }` syntax is mandatory; see [Import Assertions][]. + ## Wasm modules @@ -1406,6 +1411,7 @@ success! [Dynamic `import()`]: https://wiki.developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import#Dynamic_Imports [ECMAScript Top-Level `await` proposal]: https://github.com/tc39/proposal-top-level-await/ [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 [JSON modules]: #json-modules [Node.js Module Resolution Algorithm]: #resolver-algorithm-specification diff --git a/lib/internal/modules/esm/assert.js b/lib/internal/modules/esm/assert.js index e7d8cbb519fb79..f0f8387bea1cbf 100644 --- a/lib/internal/modules/esm/assert.js +++ b/lib/internal/modules/esm/assert.js @@ -1,11 +1,11 @@ 'use strict'; const { + ArrayPrototypeFilter, ArrayPrototypeIncludes, ObjectCreate, ObjectValues, ObjectPrototypeHasOwnProperty, - Symbol, } = primordials; const { validateString } = require('internal/validators'); @@ -15,12 +15,13 @@ const { ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED, } = require('internal/errors').codes; -const kImplicitAssertType = Symbol('implicit assert type'); +// The HTML spec has an implied default type of `'javascript'`. +const kImplicitAssertType = 'javascript'; /** - * Define a map of module formats to import assertion types (the value of `type` - * in `assert { type: 'json' }`). - * @type {Map} */ const formatTypeMap = { '__proto__': null, @@ -28,11 +29,18 @@ const formatTypeMap = { 'commonjs': kImplicitAssertType, 'json': 'json', 'module': kImplicitAssertType, - 'wasm': kImplicitAssertType, // Should probably be 'webassembly' per https://github.com/tc39/proposal-import-assertions + '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 }; -/** @type {Array} + */ +const supportedAssertionTypes = ArrayPrototypeFilter( + ObjectValues(formatTypeMap), + (type) => type !== kImplicitAssertType); /** @@ -50,14 +58,10 @@ function validateAssertions(url, format, switch (validType) { case undefined: - // Ignore assertions for module types we don't recognize, to allow new + // Ignore assertions for module formats we don't recognize, to allow new // formats in the future. return true; - case importAssertions.type: - // The asserted type is the valid type for this format. - 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. @@ -66,9 +70,13 @@ function validateAssertions(url, format, } return handleInvalidType(url, importAssertions.type); + case importAssertions.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` was not it. + // `importAssertions.type` might not have been it. if (!ObjectPrototypeHasOwnProperty(importAssertions, 'type')) { // `type` wasn't specified at all. throw new ERR_IMPORT_ASSERTION_TYPE_MISSING(url, validType); @@ -86,7 +94,7 @@ function handleInvalidType(url, type) { // `type` might have not been a string. validateString(type, 'type'); - // `type` was not one of the types we understand. + // `type` might not have been one of the types we understand. if (!ArrayPrototypeIncludes(supportedAssertionTypes, type)) { throw new ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED(type); } diff --git a/lib/internal/modules/esm/module_map.js b/lib/internal/modules/esm/module_map.js index d51986dd700c85..df02ebd708517f 100644 --- a/lib/internal/modules/esm/module_map.js +++ b/lib/internal/modules/esm/module_map.js @@ -12,20 +12,17 @@ let debug = require('internal/util/debuglog').debuglog('esm', (fn) => { const { ERR_INVALID_ARG_TYPE } = require('internal/errors').codes; const { validateString } = require('internal/validators'); -const validateAssertType = (type) => - type === kImplicitAssertType || validateString(type, 'type'); - // Tracks the state of the loader-level module cache class ModuleMap extends SafeMap { constructor(i) { super(i); } // eslint-disable-line no-useless-constructor get(url, type = kImplicitAssertType) { validateString(url, 'url'); - validateAssertType(type); + validateString(type, 'type'); return super.get(url)?.[type]; } set(url, type = kImplicitAssertType, job) { validateString(url, 'url'); - validateAssertType(type); + validateString(type, 'type'); if (job instanceof ModuleJob !== true && typeof job !== 'function') { throw new ERR_INVALID_ARG_TYPE('job', 'ModuleJob', job); @@ -39,7 +36,7 @@ class ModuleMap extends SafeMap { } has(url, type = kImplicitAssertType) { validateString(url, 'url'); - validateAssertType(type); + validateString(type, 'type'); return super.get(url)?.[type] !== undefined; } } diff --git a/test/es-module/test-esm-import-assertion-errors.js b/test/es-module/test-esm-import-assertion-errors.js index 3a55c23fbfbdf2..c7d5abee693979 100644 --- a/test/es-module/test-esm-import-assertion-errors.js +++ b/test/es-module/test-esm-import-assertion-errors.js @@ -25,7 +25,7 @@ async function test() { ); await rejects( - import('data:text/javascript,', { assert: { type: 'unsupported' } }), + import(jsModuleDataUrl, { assert: { type: 'unsupported' } }), { code: 'ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED' } ); diff --git a/test/es-module/test-esm-import-assertion-validation.js b/test/es-module/test-esm-import-assertion-validation.js index 7e64bd47392ab0..3792ad7ff1617c 100644 --- a/test/es-module/test-esm-import-assertion-validation.js +++ b/test/es-module/test-esm-import-assertion-validation.js @@ -22,8 +22,8 @@ assert.throws(() => validateAssertions(url, 'module', { type: 'json' }), { code: 'ERR_IMPORT_ASSERTION_TYPE_FAILED', }); -// This should be allowed according to HTML spec. Let's keep it disabled -// until WASM module import is sorted out. +// 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', }); diff --git a/test/es-module/test-esm-loader-modulemap.js b/test/es-module/test-esm-loader-modulemap.js index dbfda754924372..190676ec725cd2 100644 --- a/test/es-module/test-esm-loader-modulemap.js +++ b/test/es-module/test-esm-loader-modulemap.js @@ -7,7 +7,6 @@ const { strictEqual, throws } = require('assert'); const { ESMLoader } = require('internal/modules/esm/loader'); const ModuleMap = require('internal/modules/esm/module_map'); const ModuleJob = require('internal/modules/esm/module_job'); -const { kImplicitAssertType } = require('internal/modules/esm/assert'); const createDynamicModule = require( 'internal/modules/esm/create_dynamic_module'); @@ -38,14 +37,14 @@ const jsonModuleJob = new ModuleJob(loader, stubJsonModule.module, strictEqual(moduleMap.get(jsonModuleDataUrl, 'json'), jsonModuleJob); strictEqual(moduleMap.has(jsModuleDataUrl), true); - strictEqual(moduleMap.has(jsModuleDataUrl, kImplicitAssertType), true); + strictEqual(moduleMap.has(jsModuleDataUrl, 'javascript'), true); strictEqual(moduleMap.has(jsonModuleDataUrl, 'json'), true); strictEqual(moduleMap.has('unknown'), false); // The types must match strictEqual(moduleMap.has(jsModuleDataUrl, 'json'), false); - strictEqual(moduleMap.has(jsonModuleDataUrl, kImplicitAssertType), false); + strictEqual(moduleMap.has(jsonModuleDataUrl, 'javascript'), false); strictEqual(moduleMap.has(jsonModuleDataUrl), false); strictEqual(moduleMap.has(jsModuleDataUrl, 'unknown'), false); strictEqual(moduleMap.has(jsonModuleDataUrl, 'unknown'), false);