diff --git a/src/node_contextify.cc b/src/node_contextify.cc index b0398693cd235b..10ba8e62632db6 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -1702,13 +1702,19 @@ static MaybeLocal CompileFunctionForCJSLoader( return scope.Escape(fn); } +static std::string GetRequireEsmWarning(Local filename) { + Isolate* isolate = Isolate::GetCurrent(); + Utf8Value filename_utf8(isolate, filename); + + std::string warning_message = + "Failed to load the ES module: " + filename_utf8.ToString() + + ". 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 message, @@ -1794,8 +1800,8 @@ 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. - should_throw = - ProcessEmitWarningSync(env, require_esm_warning).IsJust(); + std::string warning_message = GetRequireEsmWarning(filename); + should_throw = ProcessEmitWarningSync(env, warning_message).IsJust(); } if (should_throw) { isolate->ThrowException(cjs_exception); diff --git a/src/node_process_events.cc b/src/node_process_events.cc index 8aac953b3e0db5..6f18f3f7e7f654 100644 --- a/src/node_process_events.cc +++ b/src/node_process_events.cc @@ -13,6 +13,7 @@ using v8::Just; using v8::Local; using v8::Maybe; using v8::MaybeLocal; +using v8::NewStringType; using v8::Nothing; using v8::Object; using v8::String; @@ -21,7 +22,14 @@ using v8::Value; Maybe ProcessEmitWarningSync(Environment* env, std::string_view message) { Isolate* isolate = env->isolate(); Local context = env->context(); - Local message_string = OneByteString(isolate, message); + Local message_string; + if (!String::NewFromUtf8(isolate, + message.data(), + NewStringType::kNormal, + static_cast(message.size())) + .ToLocal(&message_string)) { + return Nothing(); + } Local argv[] = {message_string}; Local emit_function = env->process_emit_warning_sync(); diff --git a/test/es-module/test-esm-cjs-load-error-note.mjs b/test/es-module/test-esm-cjs-load-error-note.mjs index 2875f4844de359..54000b53fea9b4 100644 --- a/test/es-module/test-esm-cjs-load-error-note.mjs +++ b/test/es-module/test-esm-cjs-load-error-note.mjs @@ -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}`, diff --git a/test/es-module/test-esm-long-path-win.js b/test/es-module/test-esm-long-path-win.js index ef47759698992c..fca22172a85995 100644 --- a/test/es-module/test-esm-long-path-win.js +++ b/test/es-module/test-esm-long-path-win.js @@ -182,7 +182,7 @@ describe('long path on Windows', () => { fs.writeFileSync(cjsIndexJSPath, 'import fs from "node:fs/promises";'); const { code, signal, stderr } = await spawnPromisified(execPath, [cjsIndexJSPath]); - assert.ok(stderr.includes('Warning: To load an ES module')); + assert.ok(stderr.includes('Failed to load the ES module')); assert.strictEqual(code, 1); assert.strictEqual(signal, null); }); diff --git a/test/es-module/test-typescript-commonjs.mjs b/test/es-module/test-typescript-commonjs.mjs index 5b15860ab32796..e757cfd231fbfb 100644 --- a/test/es-module/test-typescript-commonjs.mjs +++ b/test/es-module/test-typescript-commonjs.mjs @@ -1,6 +1,6 @@ import { skip, spawnPromisified } from '../common/index.mjs'; import * as fixtures from '../common/fixtures.mjs'; -import { match, strictEqual } from 'node:assert'; +import assert, { match, strictEqual } from 'node:assert'; import { test } from 'node:test'; if (!process.config.variables.node_use_amaro) skip('Requires Amaro'); @@ -59,13 +59,35 @@ 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'), - ]); - - strictEqual(result.stdout, ''); - match(result.stderr, /To load an ES module, set "type": "module" in the package\.json or use the \.mjs extension\./); - strictEqual(result.code, 1); + const testFilePath = fixtures.path( + 'typescript/cts/test-cts-but-module-syntax.cts' + ); + const result = await spawnPromisified(process.execPath, [testFilePath]); + + assert.strictEqual(result.stdout, ''); + + 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.`; + + try { + assert.ok( + result.stderr.includes(expectedWarning), + `Expected stderr to include: ${expectedWarning}` + ); + } catch (e) { + if (e?.code === 'ERR_ASSERTION') { + assert.match( + result.stderr, + /Failed to load the ES module:.*test-cts-but-module-syntax\.cts/ + ); + e.expected = expectedWarning; + e.actual = result.stderr; + e.operator = 'includes'; + } + throw e; + } + + + assert.strictEqual(result.code, 1); }); test('execute a .cts file importing a .cts file', async () => {