diff --git a/doc/api/esm.md b/doc/api/esm.md index 7a5c053f317d5b..5bf4229858a725 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -262,6 +262,9 @@ added: - v17.1.0 - v16.14.0 changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/52699 + description: JSONC can now be used as a valid type attribute. - version: - v21.0.0 - v20.10.0 @@ -289,9 +292,10 @@ const { default: barData } = Node.js supports the following `type` values, for which the attribute is mandatory: -| Attribute `type` | Needed for | -| ---------------- | ---------------- | -| `'json'` | [JSON modules][] | +| Attribute `type` | Used with | +| ------------------------------------------ | ---------------- | +| `'json'` | [JSON modules][] | +| `'jsonc'` (Non-Standard) | [JSON modules][] | ## Builtin modules diff --git a/lib/internal/modules/esm/assert.js b/lib/internal/modules/esm/assert.js index 66086536270816..2f11890b0e6af1 100644 --- a/lib/internal/modules/esm/assert.js +++ b/lib/internal/modules/esm/assert.js @@ -2,6 +2,7 @@ const { ArrayPrototypeFilter, + ArrayPrototypeFlat, ArrayPrototypeIncludes, ObjectKeys, ObjectPrototypeHasOwnProperty, @@ -25,11 +26,11 @@ const kImplicitAssertType = 'javascript'; */ const formatTypeMap = { '__proto__': null, - 'builtin': kImplicitAssertType, - 'commonjs': kImplicitAssertType, - 'json': 'json', - 'module': kImplicitAssertType, - '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 + 'builtin': [kImplicitAssertType], + 'commonjs': [kImplicitAssertType], + 'json': ['json', 'jsonc'], + 'module': [kImplicitAssertType], + '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 }; /** @@ -39,7 +40,7 @@ const formatTypeMap = { * @type {Array} */ const supportedAssertionTypes = ArrayPrototypeFilter( - ObjectValues(formatTypeMap), + ArrayPrototypeFlat(ObjectValues(formatTypeMap)), (type) => type !== kImplicitAssertType); @@ -62,33 +63,30 @@ function validateAttributes(url, format, } const validType = formatTypeMap[format]; - switch (validType) { - case undefined: - // 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 attributes object. - if (!ObjectPrototypeHasOwnProperty(importAttributes, 'type')) { - return true; - } - return handleInvalidType(url, importAttributes.type); + if (validType === undefined) { + return true; + } - case importAttributes.type: - // The asserted type is the valid type for this format. + if (validType[0] === kImplicitAssertType && validType.length === 1) { + // This format doesn't allow an import assertion type, so the property + // must not be set on the import attributes object. + if (!ObjectPrototypeHasOwnProperty(importAttributes, 'type')) { return true; + } + return handleInvalidType(url, importAttributes.type); + } + + if (ArrayPrototypeIncludes(validType, 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 - // `importAttributes.type` might not have been it. - if (!ObjectPrototypeHasOwnProperty(importAttributes, 'type')) { - // `type` wasn't specified at all. - throw new ERR_IMPORT_ATTRIBUTE_MISSING(url, 'type', validType); - } - return handleInvalidType(url, importAttributes.type); + if (!ObjectPrototypeHasOwnProperty(importAttributes, 'type')) { + // `type` wasn't specified at all. + throw new ERR_IMPORT_ATTRIBUTE_MISSING(url, 'type', validType); } + + return handleInvalidType(url, importAttributes.type); } /** diff --git a/lib/internal/modules/esm/formats.js b/lib/internal/modules/esm/formats.js index 470f679b92ec2a..298d083052bd5c 100644 --- a/lib/internal/modules/esm/formats.js +++ b/lib/internal/modules/esm/formats.js @@ -17,6 +17,7 @@ const extensionFormatMap = { '.cjs': 'commonjs', '.js': 'module', '.json': 'json', + '.jsonc': 'jsonc', '.mjs': 'module', }; diff --git a/lib/internal/modules/esm/load.js b/lib/internal/modules/esm/load.js index 7b77af35a1dfeb..349847fdcc3930 100644 --- a/lib/internal/modules/esm/load.js +++ b/lib/internal/modules/esm/load.js @@ -152,6 +152,14 @@ async function defaultLoad(url, context = kEmptyObject) { validateAttributes(url, format, importAttributes); + // Allow for .json / .jsonc to be used interchangeably with the json format. + if (format === 'json' || format === 'jsonc') { + const type = importAttributes?.type; + if (type) { + format = type; + } + } + // Use the synchronous commonjs translator which can deal with cycles. if (format === 'commonjs' && getOptionValue('--experimental-require-module')) { format = 'commonjs-sync'; diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index e5775c8d521d58..1521912ed3098c 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -8,6 +8,7 @@ const { ObjectKeys, ObjectPrototypeHasOwnProperty, ReflectApply, + RegExpPrototypeSymbolReplace, SafeArrayIterator, SafeMap, SafeSet, @@ -87,7 +88,7 @@ async function initCJSParse() { initCJSParseSync(); } else { const { parse, init } = - require('internal/deps/cjs-module-lexer/dist/lexer'); + require('internal/deps/cjs-module-lexer/dist/lexer'); try { await init(); cjsParse = parse; @@ -177,7 +178,7 @@ translators.set('module', function moduleStrategy(url, source, isMain) { */ function enrichCJSError(err, content, filename) { if (err != null && ObjectGetPrototypeOf(err) === SyntaxErrorPrototype && - containsModuleSyntax(content, filename)) { + containsModuleSyntax(content, filename)) { // Emit the warning synchronously because we are in the middle of handling // a SyntaxError that will throw and likely terminate the process before an // asynchronous warning would be emitted. @@ -228,7 +229,7 @@ function loadCJSModule(module, source, url, filename) { case '.node': return CJSModule._load(specifier, module); default: - // fall through + // fall through } specifier = `${pathToFileURL(path)}`; } @@ -248,8 +249,7 @@ function loadCJSModule(module, source, url, filename) { }); setOwnProperty(requireFn, 'main', process.mainModule); - ReflectApply(compiledWrapper, module.exports, - [module.exports, requireFn, module, filename, __dirname]); + ReflectApply(compiledWrapper, module.exports, [module.exports, requireFn, module, filename, __dirname]); setOwnProperty(module, 'loaded', true); } @@ -295,7 +295,7 @@ function createCJSModuleWrap(url, source, isMain, loadCJS = loadCJSModule) { } for (const exportName of exportNames) { if (!ObjectPrototypeHasOwnProperty(exports, exportName) || - exportName === 'default') { + exportName === 'default') { continue; } // We might trigger a getter -> dont fail. @@ -417,7 +417,7 @@ function cjsPreparseModuleExports(filename, source) { // `format: 'commonjs'` instead of relying on file extensions. const ext = extname(resolved); if ((ext === '.js' || ext === '.cjs' || !CJSModule._extensions[ext]) && - isAbsolute(resolved)) { + isAbsolute(resolved)) { // TODO: this should be calling the `load` hook chain to get the source // (and fallback to reading the FS only if the source is nullish). const source = readFileSync(resolved, 'utf-8'); @@ -449,65 +449,80 @@ translators.set('builtin', function builtinStrategy(url) { // Strategy for loading a JSON file const isWindows = process.platform === 'win32'; -translators.set('json', function jsonStrategy(url, source) { - emitExperimentalWarning('Importing JSON modules'); - assertBufferSource(source, true, 'load'); - debug(`Loading JSONModule ${url}`); - const pathname = StringPrototypeStartsWith(url, 'file:') ? - fileURLToPath(url) : null; - const shouldCheckAndPopulateCJSModuleCache = - // We want to involve the CJS loader cache only for `file:` URL with no search query and no hash. - pathname && !StringPrototypeIncludes(url, '?') && !StringPrototypeIncludes(url, '#'); - let modulePath; - let module; - if (shouldCheckAndPopulateCJSModuleCache) { - modulePath = isWindows ? - StringPrototypeReplaceAll(pathname, '/', '\\') : pathname; - module = CJSModule._cache[modulePath]; - if (module && module.loaded) { - const exports = module.exports; - return new ModuleWrap(url, undefined, ['default'], function() { - this.setExport('default', exports); - }); +function jsonStrategyBuilder(parser) { + return function jsonStrategy(url, source) { + emitExperimentalWarning('Importing JSON modules'); + assertBufferSource(source, true, 'load'); + debug(`Loading JSONModule ${url}`); + const pathname = StringPrototypeStartsWith(url, 'file:') ? + fileURLToPath(url) : null; + const shouldCheckAndPopulateCJSModuleCache = + // We want to involve the CJS loader cache only for `file:` URL with no search query and no hash. + pathname && !StringPrototypeIncludes(url, '?') && !StringPrototypeIncludes(url, '#'); + let modulePath; + let module; + if (shouldCheckAndPopulateCJSModuleCache) { + modulePath = isWindows ? + StringPrototypeReplaceAll(pathname, '/', '\\') : pathname; + module = CJSModule._cache[modulePath]; + if (module && module.loaded) { + const exports = module.exports; + return new ModuleWrap(url, undefined, ['default'], function() { + this.setExport('default', exports); + }); + } } - } - source = stringify(source); - if (shouldCheckAndPopulateCJSModuleCache) { - // A require call could have been called on the same file during loading and - // that resolves synchronously. To make sure we always return the identical - // export, we have to check again if the module already exists or not. - // TODO: remove CJS loader from here as well. - module = CJSModule._cache[modulePath]; - if (module && module.loaded) { - const exports = module.exports; - return new ModuleWrap(url, undefined, ['default'], function() { - this.setExport('default', exports); - }); + source = stringify(source); + if (shouldCheckAndPopulateCJSModuleCache) { + // A require call could have been called on the same file during loading and + // that resolves synchronously. To make sure we always return the identical + // export, we have to check again if the module already exists or not. + // TODO: remove CJS loader from here as well. + module = CJSModule._cache[modulePath]; + if (module && module.loaded) { + const exports = module.exports; + return new ModuleWrap(url, undefined, ['default'], function() { + this.setExport('default', exports); + }); + } } - } - try { - const exports = JSONParse(stripBOM(source)); - module = { - exports, - loaded: true, - }; - } catch (err) { - // TODO (BridgeAR): We could add a NodeCore error that wraps the JSON - // parse error instead of just manipulating the original error message. - // That would allow to add further properties and maybe additional - // debugging information. - err.message = errPath(url) + ': ' + err.message; - throw err; - } - if (shouldCheckAndPopulateCJSModuleCache) { - CJSModule._cache[modulePath] = module; - } - cjsCache.set(url, module); - return new ModuleWrap(url, undefined, ['default'], function() { - debug(`Parsing JSONModule ${url}`); - this.setExport('default', module.exports); - }); -}); + try { + const exports = parser(stripBOM(source)); + module = { + exports, + loaded: true, + }; + } catch (err) { + // TODO (BridgeAR): We could add a NodeCore error that wraps the JSON + // parse error instead of just manipulating the original error message. + // That would allow to add further properties and maybe additional + // debugging information. + err.message = errPath(url) + ': ' + err.message; + throw err; + } + if (shouldCheckAndPopulateCJSModuleCache) { + CJSModule._cache[modulePath] = module; + } + cjsCache.set(url, module); + return new ModuleWrap(url, undefined, ['default'], function() { + debug(`Parsing JSONModule ${url}`); + this.setExport('default', module.exports); + }); + }; +} + +// This RegExp matches all comments into matching group $2 +// and all strings into matching group $1. This way, we can replace +// the entire match with $1, which will preserve strings (including +// strings containing comments), and remove comments. +// +// eslint-disable-next-line node-core/no-unescaped-regexp-dot +const JSONAntiCommentRE = /("(?:\\.|[^"\\])*?")|(\/\*[^]*?\*\/)|\/\/.*/g; + +translators.set('json', jsonStrategyBuilder(JSONParse)); +translators.set('jsonc', jsonStrategyBuilder(function jsoncParse(source) { + return JSONParse(RegExpPrototypeSymbolReplace(JSONAntiCommentRE, source, '$1')); +})); // Strategy for loading a wasm module translators.set('wasm', async function(url, source) { @@ -528,8 +543,8 @@ translators.set('wasm', async function(url, source) { } const imports = - ArrayPrototypeMap(WebAssembly.Module.imports(compiled), - ({ module }) => module); + ArrayPrototypeMap(WebAssembly.Module.imports(compiled), + ({ module }) => module); const exports = ArrayPrototypeMap(WebAssembly.Module.exports(compiled), ({ name }) => name); diff --git a/test/es-module/test-import-jsonc.js b/test/es-module/test-import-jsonc.js new file mode 100644 index 00000000000000..bd3a0c0c0b2786 --- /dev/null +++ b/test/es-module/test-import-jsonc.js @@ -0,0 +1,22 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); + +const jsonFiles = [ + // [file, [throwWithTypeJSON, throwWithTypeJSONC]] + ['../fixtures/es-modules/json-with-comments/regular.json', [false, false]], + ['../fixtures/es-modules/json-with-comments/comments.json', [true, false]], + ['../fixtures/es-modules/json-with-comments/comments.jsonc', [true, false]], +]; + +for (const [file, [throwWithTypeJSON, throwWithTypeJSONC]] of jsonFiles) { + + assert[throwWithTypeJSON ? 'rejects' : 'doesNotReject'](async () => { + await import(file, { with: { type: 'json' } }); + }); + + assert[throwWithTypeJSONC ? 'rejects' : 'doesNotReject'](async () => { + await import(file, { with: { type: 'jsonc' } }); + }); +} diff --git a/test/fixtures/es-modules/json-with-comments/comments.json b/test/fixtures/es-modules/json-with-comments/comments.json new file mode 100644 index 00000000000000..a794708972918b --- /dev/null +++ b/test/fixtures/es-modules/json-with-comments/comments.json @@ -0,0 +1,6 @@ +{ + "key": "value" + // Some comment + /* Also + a comment */ +} \ No newline at end of file diff --git a/test/fixtures/es-modules/json-with-comments/comments.jsonc b/test/fixtures/es-modules/json-with-comments/comments.jsonc new file mode 100644 index 00000000000000..a794708972918b --- /dev/null +++ b/test/fixtures/es-modules/json-with-comments/comments.jsonc @@ -0,0 +1,6 @@ +{ + "key": "value" + // Some comment + /* Also + a comment */ +} \ No newline at end of file diff --git a/test/fixtures/es-modules/json-with-comments/regular.json b/test/fixtures/es-modules/json-with-comments/regular.json new file mode 100644 index 00000000000000..a977d991bb0e2b --- /dev/null +++ b/test/fixtures/es-modules/json-with-comments/regular.json @@ -0,0 +1,3 @@ +{ + "key": "value" +} \ No newline at end of file