GeezerVison is showing github.com — module: do not set CJS variables for Worker eval · nodejs/node@b86f575
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: lib/internal/process/execution.js +1-1Lines changed: 1 addition & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
@@ -83,7 +83,7 @@ function evalScript(name, body, breakFirstLine, print, shouldLoadESM = false) { | 83 | 83 |
| 84 | 84 | if (getOptionValue('--experimental-detect-module') && | 85 | 85 | getOptionValue('--input-type') === '' && getOptionValue('--experimental-default-type') === '' && | 86 | | - containsModuleSyntax(body, name)) { | | 86 | + containsModuleSyntax(body, name, null, 'no CJS variables')) { | 87 | 87 | return evalModuleEntryPoint(body, print); | 88 | 88 | } | 89 | 89 |
| |
Collapse file Copy file name to clipboardExpand all lines: src/node_contextify.cc +13-5Lines changed: 13 additions & 5 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -1445,7 +1445,8 @@ static MaybeLocal<Function> CompileFunctionForCJSLoader(Environment* env, | 1445 | 1445 | Local<Context> context, | 1446 | 1446 | Local<String> code, | 1447 | 1447 | Local<String> filename, | 1448 | | - bool* cache_rejected) { | | 1448 | + bool* cache_rejected, | | 1449 | + bool is_cjs_scope) { | 1449 | 1450 | Isolate* isolate = context->GetIsolate(); | 1450 | 1451 | EscapableHandleScope scope(isolate); | 1451 | 1452 |
| @@ -1485,7 +1486,10 @@ static MaybeLocal<Function> CompileFunctionForCJSLoader(Environment* env, | 1485 | 1486 | options = ScriptCompiler::kConsumeCodeCache; | 1486 | 1487 | } | 1487 | 1488 |
| 1488 | | - std::vector<Local<String>> params = GetCJSParameters(env->isolate_data()); | | 1489 | + std::vector<Local<String>> params; | | 1490 | + if (is_cjs_scope) { | | 1491 | + params = GetCJSParameters(env->isolate_data()); | | 1492 | + } | 1489 | 1493 | MaybeLocal<Function> maybe_fn = ScriptCompiler::CompileFunction( | 1490 | 1494 | context, | 1491 | 1495 | &source, | @@ -1544,7 +1548,7 @@ static void CompileFunctionForCJSLoader( | 1544 | 1548 | ShouldNotAbortOnUncaughtScope no_abort_scope(realm->env()); | 1545 | 1549 | TryCatchScope try_catch(env); | 1546 | 1550 | if (!CompileFunctionForCJSLoader( | 1547 | | - env, context, code, filename, &cache_rejected) | | 1551 | + env, context, code, filename, &cache_rejected, true) | 1548 | 1552 | .ToLocal(&fn)) { | 1549 | 1553 | CHECK(try_catch.HasCaught()); | 1550 | 1554 | CHECK(!try_catch.HasTerminated()); | @@ -1682,11 +1686,15 @@ static void ContainsModuleSyntax(const FunctionCallbackInfo<Value>& args) { | 1682 | 1686 | CHECK(args[1]->IsString()); | 1683 | 1687 | Local<String> filename = args[1].As<String>(); | 1684 | 1688 |
| 1685 | | - // Argument 2: resource name (URL for ES module). | | 1689 | + // Argument 3: resource name (URL for ES module). | 1686 | 1690 | Local<String> resource_name = filename; | 1687 | 1691 | if (args[2]->IsString()) { | 1688 | 1692 | resource_name = args[2].As<String>(); | 1689 | 1693 | } | | 1694 | + // Argument 4: flag to indicate if CJS variables should not be in scope | | 1695 | + // (they should be for normal CommonJS modules, but not for the | | 1696 | + // CommonJS eval scope). | | 1697 | + bool cjs_var = !args[3]->IsString(); | 1690 | 1698 |
| 1691 | 1699 | bool cache_rejected = false; | 1692 | 1700 | Local<String> message; | @@ -1695,7 +1703,7 @@ static void ContainsModuleSyntax(const FunctionCallbackInfo<Value>& args) { | 1695 | 1703 | TryCatchScope try_catch(env); | 1696 | 1704 | ShouldNotAbortOnUncaughtScope no_abort_scope(env); | 1697 | 1705 | if (CompileFunctionForCJSLoader( | 1698 | | - env, context, code, filename, &cache_rejected) | | 1706 | + env, context, code, filename, &cache_rejected, cjs_var) | 1699 | 1707 | .ToLocal(&fn)) { | 1700 | 1708 | args.GetReturnValue().Set(false); | 1701 | 1709 | return; | |
Collapse file Copy file name to clipboardExpand all lines: test/es-module/test-esm-detect-ambiguous.mjs +28Lines changed: 28 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -44,6 +44,19 @@ describe('--experimental-detect-module', { concurrency: true }, () => { | 44 | 44 | strictEqual(signal, null); | 45 | 45 | }); | 46 | 46 |
| | 47 | + it('should not switch to module if code is parsable as script', async () => { | | 48 | + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ | | 49 | + '--experimental-detect-module', | | 50 | + '--eval', | | 51 | + 'let __filename,__dirname,require,module,exports;this.a', | | 52 | + ]); | | 53 | + | | 54 | + strictEqual(stderr, ''); | | 55 | + strictEqual(stdout, ''); | | 56 | + strictEqual(code, 0); | | 57 | + strictEqual(signal, null); | | 58 | + }); | | 59 | + | 47 | 60 | it('should be overridden by --experimental-default-type', async () => { | 48 | 61 | const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ | 49 | 62 | '--experimental-detect-module', | @@ -393,3 +406,18 @@ describe('Wrapping a `require` of an ES module while using `--abort-on-uncaught- | 393 | 406 | strictEqual(signal, null); | 394 | 407 | }); | 395 | 408 | }); | | 409 | + | | 410 | +describe('when working with Worker threads', () => { | | 411 | + it('should support sloppy scripts that declare CJS "global-like" variables', async () => { | | 412 | + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ | | 413 | + '--experimental-detect-module', | | 414 | + '--eval', | | 415 | + 'new worker_threads.Worker("let __filename,__dirname,require,module,exports;this.a",{eval:true})', | | 416 | + ]); | | 417 | + | | 418 | + strictEqual(stderr, ''); | | 419 | + strictEqual(stdout, ''); | | 420 | + strictEqual(code, 0); | | 421 | + strictEqual(signal, null); | | 422 | + }); | | 423 | +}); |
|