feat: add dynamic file-specific ESM warnings

mertcanaltin · mertcanaltin · commit 621136ce0b8a · 2025-01-16T18:09:08.000+03:00
diff --git a/src/node_contextify.cc b/src/node_contextify.cc
@@ -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,
@@ -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);
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-typescript-commonjs.mjs b/test/es-module/test-typescript-commonjs.mjs
@@ -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);
 });
 
