From ed954adf5bd4bed3e38984a9f370cd5be7da1941 Mon Sep 17 00:00:00 2001
From: Geoffrey Booth <webadmin@geoffreybooth.com>
Date: Sun, 15 Oct 2023 17:54:34 -0700
Subject: [PATCH] code review notes

---
 lib/internal/modules/esm/translators.js |  2 +-
 src/node_contextify.cc                  | 29 +++++++++++++++----------
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js
index b6ba614e210f9b..7a62615cfe4210 100644
--- a/lib/internal/modules/esm/translators.js
+++ b/lib/internal/modules/esm/translators.js
@@ -170,7 +170,7 @@ translators.set('module', async function moduleStrategy(url, source, isMain) {
  */
 function enrichCJSError(err, content, filename) {
   if (err != null && ObjectGetPrototypeOf(err) === SyntaxErrorPrototype &&
-      containsModuleSyntax(content, filename ?? '')) {
+      containsModuleSyntax(content, filename)) {
     // Emit the warning synchronously because we are in the middle of handling
     // a SyntaxError that will throw and likely terminate the process before an
     // asynchronous warning would be emitted.
diff --git a/src/node_contextify.cc b/src/node_contextify.cc
index 1aa46fd95ddbd4..e9f78d472210b4 100644
--- a/src/node_contextify.cc
+++ b/src/node_contextify.cc
@@ -1368,17 +1368,19 @@ Local<Object> ContextifyContext::CompileFunctionAndCacheResult(
   return result;
 }
 
-// These are the error messages thrown due to ESM syntax in a CommonJS module.
+// When compiling as CommonJS source code that contains ESM syntax, the
+// following error messages are returned:
+// - `import` statements: "Cannot use import statement outside a module"
+// - `export` statements: "Unexpected token 'export'"
+// - `import.meta` references: "Cannot use 'import.meta' outside a module"
+// Dynamic `import()` is permitted in CommonJS, so it does not error.
+// While top-level `await` is not permitted in CommonJS, it returns the same
+// error message as when `await` is used in a sync function, so we don't use it
+// as a disambiguation.
 constexpr std::array<std::string_view, 3> esm_syntax_error_messages = {
-    // `import` statements return an error with the message:
-    "Cannot use import statement outside a module",
-    // `export` statements return an error with the message:
-    "Unexpected token 'export'",
-    // `import.meta` returns an error with the message:
-    "Cannot use 'import.meta' outside a module"};
-// Top-level `await` currently returns the same error message as when `await` is
-// used in a sync function, so we don't use it as a disambiguation. Dynamic
-// `import()` is permitted in CommonJS, so we don't use it as a disambiguation.
+    "Cannot use import statement outside a module",  // `import` statements
+    "Unexpected token 'export'",                     // `export` statements
+    "Cannot use 'import.meta' outside a module"};    // `import.meta` references
 
 void ContextifyContext::ContainsModuleSyntax(
     const FunctionCallbackInfo<Value>& args) {
@@ -1387,8 +1389,11 @@ void ContextifyContext::ContainsModuleSyntax(
   Local<String> code = args[0].As<String>();
 
   // Argument 2: filename
-  CHECK(args[1]->IsString());
-  Local<String> filename = args[1].As<String>();
+  Local<String> filename = String::Empty(args.GetIsolate());
+  if (!args[1]->IsUndefined()) {
+    CHECK(args[1]->IsString());
+    filename = args[1].As<String>();
+  }
 
   Environment* env = Environment::GetCurrent(args);
   Isolate* isolate = env->isolate();