FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

module: do not set CJS variables for Worker eval · nodejs/node@b86f575 · GitHub

/ node Public

Commit b86f575

Browse files
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

3 files changed

Lines changed: 42 additions & 6 deletions

File tree

‎lib/internal/process/execution.js‎

Lines 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

‎src/node_contextify.cc‎

Lines 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;

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

Lines 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)
Back | FazBrowse Home | New Git URL