Skip to content

Commit

Permalink
feat: add dynamic file-specific ESM warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
mertcanaltin committed Jan 16, 2025
1 parent 732744c commit 621136c
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 13 deletions.
21 changes: 14 additions & 7 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1688,13 +1688,19 @@ static MaybeLocal<Function> CompileFunctionForCJSLoader(
return scope.Escape(fn);
}

static std::string GetRequireEsmWarning(Local<String> filename) {
Isolate* isolate = Isolate::GetCurrent();
Utf8Value filename_utf8(isolate, filename);

std::string warning_message =
"Failed to load the ES module: " + std::string(*filename_utf8) +
". Make sure to set \"type\": \"module\" in the nearest package.json "
"file "
"or use the .mjs extension.";
return warning_message;
}

static bool warned_about_require_esm = false;
// TODO(joyeecheung): this was copied from the warning previously emitted in the
// JS land, but it's not very helpful. There should be specific information
// about which file or which package.json to update.
const char* require_esm_warning =
"To load an ES module, set \"type\": \"module\" in the package.json or use "
"the .mjs extension.";

static bool ShouldRetryAsESM(Realm* realm,
Local<String> message,
Expand Down Expand Up @@ -1780,8 +1786,9 @@ static void CompileFunctionForCJSLoader(
// This needs to call process.emit('warning') in JS which can throw if
// the user listener throws. In that case, don't try to throw the syntax
// error.
std::string warning_message = GetRequireEsmWarning(filename);
should_throw =
ProcessEmitWarningSync(env, require_esm_warning).IsJust();
ProcessEmitWarningSync(env, warning_message.c_str()).IsJust();
}
if (should_throw) {
isolate->ThrowException(cjs_exception);
Expand Down
3 changes: 1 addition & 2 deletions test/es-module/test-esm-cjs-load-error-note.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@ import assert from 'node:assert';
import { execPath } from 'node:process';
import { describe, it } from 'node:test';


// Expect note to be included in the error output
// Don't match the following sentence because it can change as features are
// added.
const expectedNote = 'Warning: To load an ES module';
const expectedNote = 'Failed to load the ES module';

const mustIncludeMessage = {
getMessage: (stderr) => `${expectedNote} not found in ${stderr}`,
Expand Down
10 changes: 6 additions & 4 deletions test/es-module/test-typescript-commonjs.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,14 @@ test('require a .ts file with implicit extension fails', async () => {
});

test('expect failure of an .mts file with CommonJS syntax', async () => {
const result = await spawnPromisified(process.execPath, [
fixtures.path('typescript/cts/test-cts-but-module-syntax.cts'),
]);
const testFilePath = fixtures.path('typescript/cts/test-cts-but-module-syntax.cts');
const result = await spawnPromisified(process.execPath, [testFilePath]);

strictEqual(result.stdout, '');
match(result.stderr, /To load an ES module, set "type": "module" in the package\.json or use the \.mjs extension\./);

const expectedWarning = `Failed to load the ES module: ${testFilePath}. Make sure to set "type": "module" in the nearest package.json file or use the .mjs extension.`;
match(result.stderr, new RegExp(expectedWarning));

strictEqual(result.code, 1);
});

Expand Down

0 comments on commit 621136c

Please sign in to comment.