Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit b86f575

Browse filesBrowse files
aduh95marco-ippolito
aduh95
authored and
marco-ippolito
committed
module: do not set CJS variables for Worker eval
PR-URL: #53050 Backport-PR-URL: #56927 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: James M Snell <jasnell@gmail.com> Refs: #52697
1 parent 30ed93d commit b86f575
Copy full SHA for b86f575

File tree

Expand file treeCollapse file tree

3 files changed

+42
-6
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+42
-6
lines changed
Open diff view settings
Collapse file

‎lib/internal/process/execution.js‎

Copy file name to clipboardExpand all lines: lib/internal/process/execution.js
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ function evalScript(name, body, breakFirstLine, print, shouldLoadESM = false) {
8383

8484
if (getOptionValue('--experimental-detect-module') &&
8585
getOptionValue('--input-type') === '' && getOptionValue('--experimental-default-type') === '' &&
86-
containsModuleSyntax(body, name)) {
86+
containsModuleSyntax(body, name, null, 'no CJS variables')) {
8787
return evalModuleEntryPoint(body, print);
8888
}
8989

Collapse file

‎src/node_contextify.cc‎

Copy file name to clipboardExpand all lines: src/node_contextify.cc
+13-5Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1445,7 +1445,8 @@ static MaybeLocal<Function> CompileFunctionForCJSLoader(Environment* env,
14451445
Local<Context> context,
14461446
Local<String> code,
14471447
Local<String> filename,
1448-
bool* cache_rejected) {
1448+
bool* cache_rejected,
1449+
bool is_cjs_scope) {
14491450
Isolate* isolate = context->GetIsolate();
14501451
EscapableHandleScope scope(isolate);
14511452

@@ -1485,7 +1486,10 @@ static MaybeLocal<Function> CompileFunctionForCJSLoader(Environment* env,
14851486
options = ScriptCompiler::kConsumeCodeCache;
14861487
}
14871488

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+
}
14891493
MaybeLocal<Function> maybe_fn = ScriptCompiler::CompileFunction(
14901494
context,
14911495
&source,
@@ -1544,7 +1548,7 @@ static void CompileFunctionForCJSLoader(
15441548
ShouldNotAbortOnUncaughtScope no_abort_scope(realm->env());
15451549
TryCatchScope try_catch(env);
15461550
if (!CompileFunctionForCJSLoader(
1547-
env, context, code, filename, &cache_rejected)
1551+
env, context, code, filename, &cache_rejected, true)
15481552
.ToLocal(&fn)) {
15491553
CHECK(try_catch.HasCaught());
15501554
CHECK(!try_catch.HasTerminated());
@@ -1682,11 +1686,15 @@ static void ContainsModuleSyntax(const FunctionCallbackInfo<Value>& args) {
16821686
CHECK(args[1]->IsString());
16831687
Local<String> filename = args[1].As<String>();
16841688

1685-
// Argument 2: resource name (URL for ES module).
1689+
// Argument 3: resource name (URL for ES module).
16861690
Local<String> resource_name = filename;
16871691
if (args[2]->IsString()) {
16881692
resource_name = args[2].As<String>();
16891693
}
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();
16901698

16911699
bool cache_rejected = false;
16921700
Local<String> message;
@@ -1695,7 +1703,7 @@ static void ContainsModuleSyntax(const FunctionCallbackInfo<Value>& args) {
16951703
TryCatchScope try_catch(env);
16961704
ShouldNotAbortOnUncaughtScope no_abort_scope(env);
16971705
if (CompileFunctionForCJSLoader(
1698-
env, context, code, filename, &cache_rejected)
1706+
env, context, code, filename, &cache_rejected, cjs_var)
16991707
.ToLocal(&fn)) {
17001708
args.GetReturnValue().Set(false);
17011709
return;
Collapse file

‎test/es-module/test-esm-detect-ambiguous.mjs‎

Copy file name to clipboardExpand all lines: test/es-module/test-esm-detect-ambiguous.mjs
+28Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,19 @@ describe('--experimental-detect-module', { concurrency: true }, () => {
4444
strictEqual(signal, null);
4545
});
4646

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+
4760
it('should be overridden by --experimental-default-type', async () => {
4861
const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [
4962
'--experimental-detect-module',
@@ -393,3 +406,18 @@ describe('Wrapping a `require` of an ES module while using `--abort-on-uncaught-
393406
strictEqual(signal, null);
394407
});
395408
});
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+
});

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.