module: add dynamic file-specific ESM warnings

mertcanaltin · RafaelGSS · commit 4c29cc7e6b47 · 2025-04-17T10:51:17.000-03:00
PR-URL: #56628 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
diff --git a/src/node_contextify.cc b/src/node_contextify.cc
@@ -1693,13 +1693,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: " + 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<String> message,
@@ -1785,8 +1791,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
@@ -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<bool> ProcessEmitWarningSync(Environment* env, std::string_view message) {
   Isolate* isolate = env->isolate();
   Local<Context> context = env->context();
-  Local<String> message_string = OneByteString(isolate, message);
+  Local<String> message_string;
+  if (!String::NewFromUtf8(isolate,
+                           message.data(),
+                           NewStringType::kNormal,
+                           static_cast<int>(message.size()))
+           .ToLocal(&message_string)) {
+    return Nothing<bool>();
+  }
 
   Local<Value> argv[] = {message_string};
   Local<Function> 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
@@ -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
@@ -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
@@ -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');
@@ -63,14 +63,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, [
-    '--experimental-strip-types',
-    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, ['--experimental-strip-types', 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 () => {
