Skip to content

Commit c76eb27

Browse files
GeoffreyBoothtargos
GeoffreyBooth
authored and
targos
committed
esm: improve check for ESM syntax
PR-URL: #50127 Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
1 parent 7740bf8 commit c76eb27

File tree

5 files changed

+207
-65
lines changed

5 files changed

+207
-65
lines changed

‎lib/internal/modules/cjs/loader.js‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ const {
9090
makeContextifyScript,
9191
runScriptInThisContext,
9292
} = require('internal/vm');
93+
const { containsModuleSyntax } = internalBinding('contextify');
9394

9495
const assert = require('internal/assert');
9596
const fs = require('fs');
@@ -104,7 +105,6 @@ const {
104105
const {
105106
getCjsConditions,
106107
initializeCjsConditions,
107-
hasEsmSyntax,
108108
loadBuiltinModule,
109109
makeRequireFunction,
110110
normalizeReferrerURL,
@@ -1315,7 +1315,7 @@ function wrapSafe(filename, content, cjsModuleInstance, codeCache) {
13151315
} catch (err) {
13161316
if (process.mainModule === cjsModuleInstance) {
13171317
const { enrichCJSError } = require('internal/modules/esm/translators');
1318-
enrichCJSError(err, content);
1318+
enrichCJSError(err, content, filename);
13191319
}
13201320
throw err;
13211321
}
@@ -1400,10 +1400,11 @@ Module._extensions['.js'] = function(module, filename) {
14001400
const pkg = packageJsonReader.readPackageScope(filename) || { __proto__: null };
14011401
// Function require shouldn't be used in ES modules.
14021402
if (pkg.data?.type === 'module') {
1403+
// This is an error path because `require` of a `.js` file in a `"type": "module"` scope is not allowed.
14031404
const parent = moduleParentCache.get(module);
14041405
const parentPath = parent?.filename;
14051406
const packageJsonPath = path.resolve(pkg.path, 'package.json');
1406-
const usesEsm = hasEsmSyntax(content);
1407+
const usesEsm = containsModuleSyntax(content, filename);
14071408
const err = new ERR_REQUIRE_ESM(filename, usesEsm, parentPath,
14081409
packageJsonPath);
14091410
// Attempt to reconstruct the parent require frame.

‎lib/internal/modules/esm/translators.js‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,11 @@ function lazyTypes() {
3030
return _TYPES = require('internal/util/types');
3131
}
3232

33+
const { containsModuleSyntax } = internalBinding('contextify');
3334
const assert = require('internal/assert');
3435
const { readFileSync } = require('fs');
3536
const { dirname, extname, isAbsolute } = require('path');
3637
const {
37-
hasEsmSyntax,
3838
loadBuiltinModule,
3939
stripBOM,
4040
} = require('internal/modules/helpers');
@@ -166,11 +166,11 @@ translators.set('module', async function moduleStrategy(url, source, isMain) {
166166
* Provide a more informative error for CommonJS imports.
167167
* @param {Error | any} err
168168
* @param {string} [content] Content of the file, if known.
169-
* @param {string} [filename] Useful only if `content` is unknown.
169+
* @param {string} [filename] The filename of the erroring module.
170170
*/
171171
function enrichCJSError(err, content, filename) {
172172
if (err != null && ObjectGetPrototypeOf(err) === SyntaxErrorPrototype &&
173-
hasEsmSyntax(content || readFileSync(filename, 'utf-8'))) {
173+
containsModuleSyntax(content, filename)) {
174174
// Emit the warning synchronously because we are in the middle of handling
175175
// a SyntaxError that will throw and likely terminate the process before an
176176
// asynchronous warning would be emitted.
@@ -217,7 +217,7 @@ function loadCJSModule(module, source, url, filename) {
217217
importModuleDynamically, // importModuleDynamically
218218
).function;
219219
} catch (err) {
220-
enrichCJSError(err, source, url);
220+
enrichCJSError(err, source, filename);
221221
throw err;
222222
}
223223

@@ -344,7 +344,7 @@ translators.set('commonjs', async function commonjsStrategy(url, source,
344344
assert(module === CJSModule._cache[filename]);
345345
CJSModule._load(filename);
346346
} catch (err) {
347-
enrichCJSError(err, source, url);
347+
enrichCJSError(err, source, filename);
348348
throw err;
349349
}
350350
} : loadCJSModule;

‎lib/internal/modules/helpers.js‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
const {
44
ArrayPrototypeForEach,
55
ArrayPrototypeJoin,
6-
ArrayPrototypeSome,
76
ObjectDefineProperty,
87
ObjectPrototypeHasOwnProperty,
98
SafeMap,
@@ -299,32 +298,10 @@ function normalizeReferrerURL(referrer) {
299298
return new URL(referrer).href;
300299
}
301300

302-
/**
303-
* For error messages only, check if ESM syntax is in use.
304-
* @param {string} code
305-
*/
306-
function hasEsmSyntax(code) {
307-
debug('Checking for ESM syntax');
308-
const parser = require('internal/deps/acorn/acorn/dist/acorn').Parser;
309-
let root;
310-
try {
311-
root = parser.parse(code, { sourceType: 'module', ecmaVersion: 'latest' });
312-
} catch {
313-
return false;
314-
}
315-
316-
return ArrayPrototypeSome(root.body, (stmt) =>
317-
stmt.type === 'ExportDefaultDeclaration' ||
318-
stmt.type === 'ExportNamedDeclaration' ||
319-
stmt.type === 'ImportDeclaration' ||
320-
stmt.type === 'ExportAllDeclaration');
321-
}
322-
323301
module.exports = {
324302
addBuiltinLibsToObject,
325303
getCjsConditions,
326304
initializeCjsConditions,
327-
hasEsmSyntax,
328305
loadBuiltinModule,
329306
makeRequireFunction,
330307
normalizeReferrerURL,

‎src/node_contextify.cc‎

Lines changed: 174 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -318,13 +318,15 @@ void ContextifyContext::CreatePerIsolateProperties(
318318
SetMethod(isolate, target, "makeContext", MakeContext);
319319
SetMethod(isolate, target, "isContext", IsContext);
320320
SetMethod(isolate, target, "compileFunction", CompileFunction);
321+
SetMethod(isolate, target, "containsModuleSyntax", ContainsModuleSyntax);
321322
}
322323

323324
void ContextifyContext::RegisterExternalReferences(
324325
ExternalReferenceRegistry* registry) {
325326
registry->Register(MakeContext);
326327
registry->Register(IsContext);
327328
registry->Register(CompileFunction);
329+
registry->Register(ContainsModuleSyntax);
328330
registry->Register(PropertyGetterCallback);
329331
registry->Register(PropertySetterCallback);
330332
registry->Register(PropertyDescriptorCallback);
@@ -1204,33 +1206,18 @@ void ContextifyContext::CompileFunction(
12041206
data + cached_data_buf->ByteOffset(), cached_data_buf->ByteLength());
12051207
}
12061208

1207-
// Set host_defined_options
12081209
Local<PrimitiveArray> host_defined_options =
1209-
PrimitiveArray::New(isolate, loader::HostDefinedOptions::kLength);
1210-
host_defined_options->Set(
1211-
isolate, loader::HostDefinedOptions::kID, id_symbol);
1212-
1213-
ScriptOrigin origin(isolate,
1214-
filename,
1215-
line_offset, // line offset
1216-
column_offset, // column offset
1217-
true, // is cross origin
1218-
-1, // script id
1219-
Local<Value>(), // source map URL
1220-
false, // is opaque (?)
1221-
false, // is WASM
1222-
false, // is ES Module
1223-
host_defined_options);
1224-
1225-
ScriptCompiler::Source source(code, origin, cached_data);
1226-
ScriptCompiler::CompileOptions options;
1227-
if (source.GetCachedData() == nullptr) {
1228-
options = ScriptCompiler::kNoCompileOptions;
1229-
} else {
1230-
options = ScriptCompiler::kConsumeCodeCache;
1231-
}
1210+
GetHostDefinedOptions(isolate, id_symbol);
1211+
ScriptCompiler::Source source =
1212+
GetCommonJSSourceInstance(isolate,
1213+
code,
1214+
filename,
1215+
line_offset,
1216+
column_offset,
1217+
host_defined_options,
1218+
cached_data);
1219+
ScriptCompiler::CompileOptions options = GetCompileOptions(source);
12321220

1233-
TryCatchScope try_catch(env);
12341221
Context::Scope scope(parsing_context);
12351222

12361223
// Read context extensions from buffer
@@ -1255,9 +1242,83 @@ void ContextifyContext::CompileFunction(
12551242
}
12561243
}
12571244

1245+
TryCatchScope try_catch(env);
1246+
Local<Object> result = CompileFunctionAndCacheResult(env,
1247+
parsing_context,
1248+
&source,
1249+
params,
1250+
context_extensions,
1251+
options,
1252+
produce_cached_data,
1253+
id_symbol,
1254+
try_catch);
1255+
1256+
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
1257+
try_catch.ReThrow();
1258+
return;
1259+
}
1260+
1261+
if (result.IsEmpty()) {
1262+
return;
1263+
}
1264+
args.GetReturnValue().Set(result);
1265+
}
1266+
1267+
Local<PrimitiveArray> ContextifyContext::GetHostDefinedOptions(
1268+
Isolate* isolate, Local<Symbol> id_symbol) {
1269+
Local<PrimitiveArray> host_defined_options =
1270+
PrimitiveArray::New(isolate, loader::HostDefinedOptions::kLength);
1271+
host_defined_options->Set(
1272+
isolate, loader::HostDefinedOptions::kID, id_symbol);
1273+
return host_defined_options;
1274+
}
1275+
1276+
ScriptCompiler::Source ContextifyContext::GetCommonJSSourceInstance(
1277+
Isolate* isolate,
1278+
Local<String> code,
1279+
Local<String> filename,
1280+
int line_offset,
1281+
int column_offset,
1282+
Local<PrimitiveArray> host_defined_options,
1283+
ScriptCompiler::CachedData* cached_data) {
1284+
ScriptOrigin origin(isolate,
1285+
filename,
1286+
line_offset, // line offset
1287+
column_offset, // column offset
1288+
true, // is cross origin
1289+
-1, // script id
1290+
Local<Value>(), // source map URL
1291+
false, // is opaque (?)
1292+
false, // is WASM
1293+
false, // is ES Module
1294+
host_defined_options);
1295+
return ScriptCompiler::Source(code, origin, cached_data);
1296+
}
1297+
1298+
ScriptCompiler::CompileOptions ContextifyContext::GetCompileOptions(
1299+
const ScriptCompiler::Source& source) {
1300+
ScriptCompiler::CompileOptions options;
1301+
if (source.GetCachedData() != nullptr) {
1302+
options = ScriptCompiler::kConsumeCodeCache;
1303+
} else {
1304+
options = ScriptCompiler::kNoCompileOptions;
1305+
}
1306+
return options;
1307+
}
1308+
1309+
Local<Object> ContextifyContext::CompileFunctionAndCacheResult(
1310+
Environment* env,
1311+
Local<Context> parsing_context,
1312+
ScriptCompiler::Source* source,
1313+
std::vector<Local<String>> params,
1314+
std::vector<Local<Object>> context_extensions,
1315+
ScriptCompiler::CompileOptions options,
1316+
bool produce_cached_data,
1317+
Local<Symbol> id_symbol,
1318+
const TryCatchScope& try_catch) {
12581319
MaybeLocal<Function> maybe_fn = ScriptCompiler::CompileFunction(
12591320
parsing_context,
1260-
&source,
1321+
source,
12611322
params.size(),
12621323
params.data(),
12631324
context_extensions.size(),
@@ -1269,24 +1330,26 @@ void ContextifyContext::CompileFunction(
12691330
if (!maybe_fn.ToLocal(&fn)) {
12701331
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
12711332
errors::DecorateErrorStack(env, try_catch);
1272-
try_catch.ReThrow();
1333+
return Object::New(env->isolate());
12731334
}
1274-
return;
12751335
}
1336+
1337+
Local<Context> context = env->context();
12761338
if (fn->SetPrivate(context, env->host_defined_option_symbol(), id_symbol)
12771339
.IsNothing()) {
1278-
return;
1340+
return Object::New(env->isolate());
12791341
}
12801342

1343+
Isolate* isolate = env->isolate();
12811344
Local<Object> result = Object::New(isolate);
12821345
if (result->Set(parsing_context, env->function_string(), fn).IsNothing())
1283-
return;
1346+
return Object::New(env->isolate());
12841347
if (result
12851348
->Set(parsing_context,
12861349
env->source_map_url_string(),
12871350
fn->GetScriptOrigin().SourceMapUrl())
12881351
.IsNothing())
1289-
return;
1352+
return Object::New(env->isolate());
12901353

12911354
std::unique_ptr<ScriptCompiler::CachedData> new_cached_data;
12921355
if (produce_cached_data) {
@@ -1295,14 +1358,91 @@ void ContextifyContext::CompileFunction(
12951358
if (StoreCodeCacheResult(env,
12961359
result,
12971360
options,
1298-
source,
1361+
*source,
12991362
produce_cached_data,
13001363
std::move(new_cached_data))
13011364
.IsNothing()) {
1302-
return;
1365+
return Object::New(env->isolate());
13031366
}
13041367

1305-
args.GetReturnValue().Set(result);
1368+
return result;
1369+
}
1370+
1371+
// When compiling as CommonJS source code that contains ESM syntax, the
1372+
// following error messages are returned:
1373+
// - `import` statements: "Cannot use import statement outside a module"
1374+
// - `export` statements: "Unexpected token 'export'"
1375+
// - `import.meta` references: "Cannot use 'import.meta' outside a module"
1376+
// Dynamic `import()` is permitted in CommonJS, so it does not error.
1377+
// While top-level `await` is not permitted in CommonJS, it returns the same
1378+
// error message as when `await` is used in a sync function, so we don't use it
1379+
// as a disambiguation.
1380+
constexpr std::array<std::string_view, 3> esm_syntax_error_messages = {
1381+
"Cannot use import statement outside a module", // `import` statements
1382+
"Unexpected token 'export'", // `export` statements
1383+
"Cannot use 'import.meta' outside a module"}; // `import.meta` references
1384+
1385+
void ContextifyContext::ContainsModuleSyntax(
1386+
const FunctionCallbackInfo<Value>& args) {
1387+
// Argument 1: source code
1388+
CHECK(args[0]->IsString());
1389+
Local<String> code = args[0].As<String>();
1390+
1391+
// Argument 2: filename
1392+
Local<String> filename = String::Empty(args.GetIsolate());
1393+
if (!args[1]->IsUndefined()) {
1394+
CHECK(args[1]->IsString());
1395+
filename = args[1].As<String>();
1396+
}
1397+
1398+
Environment* env = Environment::GetCurrent(args);
1399+
Isolate* isolate = env->isolate();
1400+
Local<Context> context = env->context();
1401+
1402+
// TODO(geoffreybooth): Centralize this rather than matching the logic in
1403+
// cjs/loader.js and translators.js
1404+
Local<String> script_id = String::Concat(
1405+
isolate, String::NewFromUtf8(isolate, "cjs:").ToLocalChecked(), filename);
1406+
Local<Symbol> id_symbol = Symbol::New(isolate, script_id);
1407+
1408+
Local<PrimitiveArray> host_defined_options =
1409+
GetHostDefinedOptions(isolate, id_symbol);
1410+
ScriptCompiler::Source source = GetCommonJSSourceInstance(
1411+
isolate, code, filename, 0, 0, host_defined_options, nullptr);
1412+
ScriptCompiler::CompileOptions options = GetCompileOptions(source);
1413+
1414+
std::vector<Local<String>> params = {
1415+
String::NewFromUtf8(isolate, "exports").ToLocalChecked(),
1416+
String::NewFromUtf8(isolate, "require").ToLocalChecked(),
1417+
String::NewFromUtf8(isolate, "module").ToLocalChecked(),
1418+
String::NewFromUtf8(isolate, "__filename").ToLocalChecked(),
1419+
String::NewFromUtf8(isolate, "__dirname").ToLocalChecked()};
1420+
1421+
TryCatchScope try_catch(env);
1422+
1423+
ContextifyContext::CompileFunctionAndCacheResult(env,
1424+
context,
1425+
&source,
1426+
params,
1427+
std::vector<Local<Object>>(),
1428+
options,
1429+
true,
1430+
id_symbol,
1431+
try_catch);
1432+
1433+
bool found_error_message_caused_by_module_syntax = false;
1434+
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
1435+
Utf8Value message_value(env->isolate(), try_catch.Message()->Get());
1436+
auto message = message_value.ToStringView();
1437+
1438+
for (const auto& error_message : esm_syntax_error_messages) {
1439+
if (message.find(error_message) != std::string_view::npos) {
1440+
found_error_message_caused_by_module_syntax = true;
1441+
break;
1442+
}
1443+
}
1444+
}
1445+
args.GetReturnValue().Set(found_error_message_caused_by_module_syntax);
13061446
}
13071447

13081448
static void StartSigintWatchdog(const FunctionCallbackInfo<Value>& args) {

0 commit comments

Comments
 (0)