code review notes

Author: GeoffreyBooth

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.

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();