Skip to content

Commit

Permalink
module: fix error reporting when commonjs requires an ES module
Browse files Browse the repository at this point in the history
  • Loading branch information
geeksilva97 committed Nov 30, 2024
1 parent 03ec900 commit 96f5831
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 0 deletions.
2 changes: 2 additions & 0 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const {
ArrayPrototypeUnshiftApply,
Boolean,
Error,
ErrorCaptureStackTrace,
JSONParse,
ObjectDefineProperty,
ObjectFreeze,
Expand Down Expand Up @@ -1674,6 +1675,7 @@ function getRequireESMError(mod, pkg, content, filename) {
const usesEsm = containsModuleSyntax(content, filename);
const err = new ERR_REQUIRE_ESM(filename, usesEsm, parentPath,
packageJsonPath);
ErrorCaptureStackTrace(err, Module.prototype.require);
// Attempt to reconstruct the parent require frame.
const parentModule = Module._cache[parentPath];
if (parentModule) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import nothing from 'somewhere';
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
function main() {
func1(func2(func3()))
}

function func1() {
require('./app.js')
}
function func2() {}
function func3() {}

main()
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"type": "module"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import '../common/index.mjs';
import { test } from 'node:test';
import * as fixtures from '../common/fixtures.mjs';
import { spawnSync } from 'node:child_process';
import assert from 'node:assert';
import { EOL } from 'node:os';

test('correctly reports errors when an ESM module is required with --no-experimental-require-module', () => {
// The following regex matches the error message that is expected to be thrown
//
// package-type-module/require-esm-error-annotation/index.cjs:1
// const app = require('./app');
// ^

const fixture = fixtures.path('es-modules/package-type-module/require-esm-error-annotation/index.cjs');
const args = ['--no-experimental-require-module', fixture];

const lineNumber = 1;
const lineContent = `const app = require('./app');`;
const fullMessage = `${fixture}:${lineNumber}${EOL}${lineContent}${EOL} ^${EOL}`;

const result = spawnSync(process.execPath, args);

console.log(result.stderr.toString());

assert.strictEqual(result.status, 1);
assert(result.stderr.toString().indexOf(fullMessage) > -1);
assert.strictEqual(result.stdout.toString(), '');
});

test('correctly reports error for a longer stack trace', () => {
// The following regex matches the error message that is expected to be thrown
//
// package-type-module/require-esm-error-annotation/longer-stack.cjs:6
// require('./app.js')
// ^

const fixture = fixtures.path('es-modules/package-type-module/require-esm-error-annotation/longer-stack.cjs');
const args = ['--no-experimental-require-module', fixture];

const lineNumber = 6;
const lineContent = "require('./app.js')";
const fullMessage = `${fixture}:${lineNumber}${EOL} ${lineContent}${EOL} ^${EOL}`;

const result = spawnSync(process.execPath, args);

console.log(result.stderr.toString());

assert.strictEqual(result.status, 1);
assert.strictEqual(result.stdout.toString(), '');
assert(result.stderr.toString().indexOf(fullMessage) > -1);
});

0 comments on commit 96f5831

Please sign in to comment.