diff --git a/lib/internal/errors.js b/lib/internal/errors.js index fb01b8f6731ff1..6fb884c7df4167 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -14,6 +14,7 @@ const { AggregateError, ArrayFrom, ArrayIsArray, + ArrayPrototypeFilter, ArrayPrototypeIncludes, ArrayPrototypeIndexOf, ArrayPrototypeJoin, @@ -788,6 +789,34 @@ const fatalExceptionStackEnhancers = { } }; +// Ensures the printed error line is from user code. +let _kArrowMessagePrivateSymbol, _setHiddenValue; +function setArrowMessage(err, arrowMessage) { + if (!_kArrowMessagePrivateSymbol) { + ({ + arrow_message_private_symbol: _kArrowMessagePrivateSymbol, + setHiddenValue: _setHiddenValue, + } = internalBinding('util')); + } + _setHiddenValue(err, _kArrowMessagePrivateSymbol, arrowMessage); +} + +// Hide stack lines before the first user code line. +function hideInternalStackFrames(error) { + overrideStackTrace.set(error, (error, stackFrames) => { + let frames = stackFrames; + if (typeof stackFrames === 'object') { + frames = ArrayPrototypeFilter( + stackFrames, + (frm) => !StringPrototypeStartsWith(frm.getFileName(), + 'node:internal') + ); + } + ArrayPrototypeUnshift(frames, error); + return ArrayPrototypeJoin(frames, '\n at '); + }); +} + // Node uses an AbortError that isn't exactly the same as the DOMException // to make usage of the error in userland and readable-stream easier. // It is a regular error with `.code` and `.name`. @@ -806,8 +835,10 @@ module.exports = { exceptionWithHostPort, getMessage, hideStackFrames, + hideInternalStackFrames, isErrorStackTraceLimitWritable, isStackOverflowError, + setArrowMessage, connResetException, uvErrmapGet, uvException, @@ -842,6 +873,7 @@ module.exports = { // Note: Please try to keep these in alphabetical order // // Note: Node.js specific errors must begin with the prefix ERR_ + E('ERR_AMBIGUOUS_ARGUMENT', 'The "%s" argument is ambiguous. %s', TypeError); E('ERR_ARG_NOT_ITERABLE', '%s must be iterable', TypeError); E('ERR_ASSERTION', '%s', Error); @@ -1406,23 +1438,32 @@ E('ERR_PERFORMANCE_INVALID_TIMESTAMP', '%d is not a valid timestamp', TypeError); E('ERR_PERFORMANCE_MEASURE_INVALID_OPTIONS', '%s', TypeError); E('ERR_REQUIRE_ESM', - (filename, parentPath = null, packageJsonPath = null) => { - let msg = `Must use import to load ES Module: ${filename}`; - if (parentPath && packageJsonPath) { - const path = require('path'); - const basename = path.basename(filename) === path.basename(parentPath) ? - filename : path.basename(filename); - msg += - '\nrequire() of ES modules is not supported.\nrequire() of ' + - `${filename} from ${parentPath} ` + - 'is an ES module file as it is a .js file whose nearest parent ' + - 'package.json contains "type": "module" which defines all .js ' + - 'files in that package scope as ES modules.\nInstead rename ' + - `${basename} to end in .cjs, change the requiring code to use ` + - 'import(), or remove "type": "module" from ' + - `${packageJsonPath}.\n`; + function(filename, hasEsmSyntax, parentPath = null, packageJsonPath = null) { + hideInternalStackFrames(this); + let msg = `require() of ES Module ${filename}${parentPath ? ` from ${ + parentPath}` : ''} not supported.`; + if (!packageJsonPath) { + if (StringPrototypeEndsWith(filename, '.mjs')) + msg += `\nInstead change the require of ${filename} to a dynamic ` + + 'import() which is available in all CommonJS modules.'; + return msg; + } + const path = require('path'); + const basename = path.basename(filename) === path.basename(parentPath) ? + filename : path.basename(filename); + if (hasEsmSyntax) { + msg += `\nInstead change the require of ${basename} in ${parentPath} to` + + ' a dynamic import() which is available in all CommonJS modules.'; return msg; } + msg += `\n${basename} is treated as an ES module file as it is a .js ` + + 'file whose nearest parent package.json contains "type": "module" ' + + 'which declares all .js files in that package scope as ES modules.' + + `\nInstead rename ${basename} to end in .cjs, change the requiring ` + + 'code to use dynamic import() which is available in all CommonJS ' + + 'modules, or change "type": "module" to "type": "commonjs" in ' + + `${packageJsonPath} to treat all .js files as CommonJS (using .mjs for ` + + 'all ES modules instead).\n'; return msg; }, Error); E('ERR_SCRIPT_EXECUTION_INTERRUPTED', diff --git a/lib/internal/modules/cjs/helpers.js b/lib/internal/modules/cjs/helpers.js index 94602b5309efff..bac854e0fadbac 100644 --- a/lib/internal/modules/cjs/helpers.js +++ b/lib/internal/modules/cjs/helpers.js @@ -3,6 +3,7 @@ const { ArrayPrototypeForEach, ArrayPrototypeJoin, + ArrayPrototypeSome, ObjectDefineProperty, ObjectPrototypeHasOwnProperty, SafeMap, @@ -184,9 +185,26 @@ function normalizeReferrerURL(referrer) { return new URL(referrer).href; } +// For error messages only - used to check if ESM syntax is in use. +function hasEsmSyntax(code) { + const parser = require('internal/deps/acorn/acorn/dist/acorn').Parser; + let root; + try { + root = parser.parse(code, { sourceType: 'module', ecmaVersion: 'latest' }); + } catch { + return false; + } + + return ArrayPrototypeSome(root.body, (stmt) => + stmt.type === 'ExportNamedDeclaration' || + stmt.type === 'ImportDeclaration' || + stmt.type === 'ExportAllDeclaration'); +} + module.exports = { addBuiltinLibsToObject, cjsConditions, + hasEsmSyntax, loadNativeModule, makeRequireFunction, normalizeReferrerURL, diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index d969a6449cf545..31ab3a8ee9c86f 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -48,6 +48,7 @@ const { Proxy, ReflectApply, ReflectSet, + RegExpPrototypeExec, RegExpPrototypeTest, SafeMap, SafeWeakMap, @@ -58,6 +59,7 @@ const { StringPrototypeLastIndexOf, StringPrototypeIndexOf, StringPrototypeMatch, + StringPrototypeRepeat, StringPrototypeSlice, StringPrototypeSplit, StringPrototypeStartsWith, @@ -88,11 +90,12 @@ const { internalModuleStat } = internalBinding('fs'); const packageJsonReader = require('internal/modules/package_json_reader'); const { safeGetenv } = internalBinding('credentials'); const { + cjsConditions, + hasEsmSyntax, + loadNativeModule, makeRequireFunction, normalizeReferrerURL, stripBOM, - cjsConditions, - loadNativeModule } = require('internal/modules/cjs/helpers'); const { getOptionValue } = require('internal/options'); const preserveSymlinks = getOptionValue('--preserve-symlinks'); @@ -107,11 +110,14 @@ const policy = getOptionValue('--experimental-policy') ? let hasLoadedAnyUserCJSModule = false; const { - ERR_INVALID_ARG_VALUE, - ERR_INVALID_MODULE_SPECIFIER, - ERR_REQUIRE_ESM, - ERR_UNKNOWN_BUILTIN_MODULE, -} = require('internal/errors').codes; + codes: { + ERR_INVALID_ARG_VALUE, + ERR_INVALID_MODULE_SPECIFIER, + ERR_REQUIRE_ESM, + ERR_UNKNOWN_BUILTIN_MODULE, + }, + setArrowMessage, +} = require('internal/errors'); const { validateString } = require('internal/validators'); const pendingDeprecation = getOptionValue('--pending-deprecation'); @@ -970,7 +976,7 @@ Module.prototype.load = function(filename) { const extension = findLongestRegisteredExtension(filename); // allow .mjs to be overridden if (StringPrototypeEndsWith(filename, '.mjs') && !Module._extensions['.mjs']) - throw new ERR_REQUIRE_ESM(filename); + throw new ERR_REQUIRE_ESM(filename, true); Module._extensions[extension](this, filename); this.loaded = true; @@ -1102,16 +1108,6 @@ Module.prototype._compile = function(content, filename) { // Native extension for .js Module._extensions['.js'] = function(module, filename) { - if (StringPrototypeEndsWith(filename, '.js')) { - const pkg = readPackageScope(filename); - // Function require shouldn't be used in ES modules. - if (pkg?.data?.type === 'module') { - const parent = moduleParentCache.get(module); - const parentPath = parent?.filename; - const packageJsonPath = path.resolve(pkg.path, 'package.json'); - throw new ERR_REQUIRE_ESM(filename, parentPath, packageJsonPath); - } - } // If already analyzed the source, then it will be cached. const cached = cjsParseCache.get(module); let content; @@ -1121,6 +1117,39 @@ Module._extensions['.js'] = function(module, filename) { } else { content = fs.readFileSync(filename, 'utf8'); } + if (StringPrototypeEndsWith(filename, '.js')) { + const pkg = readPackageScope(filename); + // Function require shouldn't be used in ES modules. + if (pkg?.data?.type === 'module') { + const parent = moduleParentCache.get(module); + const parentPath = parent?.filename; + const packageJsonPath = path.resolve(pkg.path, 'package.json'); + const usesEsm = hasEsmSyntax(content); + const err = new ERR_REQUIRE_ESM(filename, usesEsm, parentPath, + packageJsonPath); + // Attempt to reconstruct the parent require frame. + if (Module._cache[parentPath]) { + let parentSource; + try { + parentSource = fs.readFileSync(parentPath, 'utf8'); + } catch {} + if (parentSource) { + const errLine = StringPrototypeSplit( + StringPrototypeSlice(err.stack, StringPrototypeIndexOf( + err.stack, ' at ')), '\n', 1)[0]; + const { 1: line, 2: col } = + RegExpPrototypeExec(/(\d+):(\d+)\)/, errLine) || []; + if (line && col) { + const srcLine = StringPrototypeSplit(parentSource, '\n')[line - 1]; + const frame = `${parentPath}:${line}\n${srcLine}\n${ + StringPrototypeRepeat(' ', col - 1)}^\n`; + setArrowMessage(err, frame); + } + } + } + throw err; + } + } module._compile(content, filename); }; diff --git a/src/node_errors.cc b/src/node_errors.cc index 6f71d95c391e93..0e4ba26e1bcb70 100644 --- a/src/node_errors.cc +++ b/src/node_errors.cc @@ -215,6 +215,12 @@ void AppendExceptionLine(Environment* env, Local err_obj; if (!er.IsEmpty() && er->IsObject()) { err_obj = er.As(); + // If arrow_message is already set, skip. + auto maybe_value = err_obj->GetPrivate(env->context(), + env->arrow_message_private_symbol()); + Local lvalue; + if (!maybe_value.ToLocal(&lvalue) || lvalue->IsString()) + return; } bool added_exception_line = false; diff --git a/test/es-module/test-cjs-esm-warn.js b/test/es-module/test-cjs-esm-warn.js index ddeda72fc84b41..d7eeb65b152a4a 100644 --- a/test/es-module/test-cjs-esm-warn.js +++ b/test/es-module/test-cjs-esm-warn.js @@ -6,36 +6,59 @@ const { spawn } = require('child_process'); const assert = require('assert'); const path = require('path'); -const requiring = path.resolve(fixtures.path('/es-modules/cjs-esm.js')); -const required = path.resolve( - fixtures.path('/es-modules/package-type-module/cjs.js') -); +const requiringCjsAsEsm = path.resolve(fixtures.path('/es-modules/cjs-esm.js')); +const requiringEsm = path.resolve(fixtures.path('/es-modules/cjs-esm-esm.js')); const pjson = path.resolve( fixtures.path('/es-modules/package-type-module/package.json') ); -const basename = 'cjs.js'; +{ + const required = path.resolve( + fixtures.path('/es-modules/package-type-module/cjs.js') + ); + const basename = 'cjs.js'; + const child = spawn(process.execPath, [requiringCjsAsEsm]); + let stderr = ''; + child.stderr.setEncoding('utf8'); + child.stderr.on('data', (data) => { + stderr += data; + }); + child.on('close', common.mustCall((code, signal) => { + assert.strictEqual(code, 1); + assert.strictEqual(signal, null); + + assert.ok(stderr.replaceAll('\r', '').includes( + `Error [ERR_REQUIRE_ESM]: require() of ES Module ${required} from ${ + requiringCjsAsEsm} not supported.\n`)); + assert.ok(stderr.replaceAll('\r', '').includes( + `Instead rename ${basename} to end in .cjs, change the requiring ` + + 'code to use dynamic import() which is available in all CommonJS ' + + `modules, or change "type": "module" to "type": "commonjs" in ${pjson} to ` + + 'treat all .js files as CommonJS (using .mjs for all ES modules ' + + 'instead).\n')); + })); +} -const child = spawn(process.execPath, [requiring]); -let stderr = ''; -child.stderr.setEncoding('utf8'); -child.stderr.on('data', (data) => { - stderr += data; -}); -child.on('close', common.mustCall((code, signal) => { - assert.strictEqual(code, 1); - assert.strictEqual(signal, null); +{ + const required = path.resolve( + fixtures.path('/es-modules/package-type-module/esm.js') + ); + const basename = 'esm.js'; + const child = spawn(process.execPath, [requiringEsm]); + let stderr = ''; + child.stderr.setEncoding('utf8'); + child.stderr.on('data', (data) => { + stderr += data; + }); + child.on('close', common.mustCall((code, signal) => { + assert.strictEqual(code, 1); + assert.strictEqual(signal, null); - assert.ok(stderr.replace(/\r/g, '').includes( - `Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: ${required}` + - '\nrequire() of ES modules is not supported.\nrequire() of ' + - `${required} from ${requiring} ` + - 'is an ES module file as it is a .js file whose nearest parent ' + - 'package.json contains "type": "module" which defines all .js ' + - 'files in that package scope as ES modules.\nInstead rename ' + - `${basename} to end in .cjs, change the requiring code to use ` + - 'import(), or remove "type": "module" from ' + - `${pjson}.\n`)); - assert.ok(stderr.includes( - 'Error [ERR_REQUIRE_ESM]: Must use import to load ES Module')); -})); + assert.ok(stderr.replace(/\r/g, '').includes( + `Error [ERR_REQUIRE_ESM]: require() of ES Module ${required} from ${ + requiringEsm} not supported.\n`)); + assert.ok(stderr.replace(/\r/g, '').includes( + `Instead change the require of ${basename} in ${requiringEsm} to` + + ' a dynamic import() which is available in all CommonJS modules.\n')); + })); +} diff --git a/test/es-module/test-esm-type-flag-errors.js b/test/es-module/test-esm-type-flag-errors.js index e0dedc16cabf8c..d9d264cc25aa25 100644 --- a/test/es-module/test-esm-type-flag-errors.js +++ b/test/es-module/test-esm-type-flag-errors.js @@ -29,8 +29,8 @@ try { } catch (e) { assert.strictEqual(e.name, 'Error'); assert.strictEqual(e.code, 'ERR_REQUIRE_ESM'); - assert(e.toString().match(/Must use import to load ES Module/g)); - assert(e.message.match(/Must use import to load ES Module/g)); + assert(e.toString().match(/require\(\) of ES Module/g)); + assert(e.message.match(/require\(\) of ES Module/g)); } function expect(opt = '', inputFile, want, wantsError = false) { diff --git a/test/fixtures/es-modules/cjs-esm-esm.js b/test/fixtures/es-modules/cjs-esm-esm.js new file mode 100644 index 00000000000000..0a0cdfb1e56327 --- /dev/null +++ b/test/fixtures/es-modules/cjs-esm-esm.js @@ -0,0 +1 @@ +require('./package-type-module/esm.js'); diff --git a/test/fixtures/es-modules/package-type-module/esm.js b/test/fixtures/es-modules/package-type-module/esm.js new file mode 100644 index 00000000000000..1413bf5968ad73 --- /dev/null +++ b/test/fixtures/es-modules/package-type-module/esm.js @@ -0,0 +1 @@ +export var p = 5; diff --git a/test/parallel/test-require-mjs.js b/test/parallel/test-require-mjs.js index 69f8527555db71..112f08879d4290 100644 --- a/test/parallel/test-require-mjs.js +++ b/test/parallel/test-require-mjs.js @@ -5,7 +5,7 @@ const assert = require('assert'); assert.throws( () => require('../fixtures/es-modules/test-esm-ok.mjs'), { - message: /Must use import to load ES Module/, + message: /dynamic import\(\) which is available in all CommonJS modules/, code: 'ERR_REQUIRE_ESM' } );