Skip to content

Commit 5874a03

Browse files
joyeecheungBridgeAR
joyeecheung
authored and
BridgeAR
committedDec 5, 2018
process: refactor the bootstrap mode branching for readability
This patch refactors the branches for choosing the mode to run Node.js in `internal/bootstrap/node.js`. Instead of inlining the decision making all in `startup`, we create a `startExecution()` function which either detects and start the non-user-code mode, or prepares for user code execution (worker setup, preloading modules) and starts user code execution. We use early returns when we decide the mode to run Node.js in for fewer indentations and better readability. This patch also adds a few comments about the command-line switches and a few TODOs to remove underscore properties on `process` that are mainly used for bootstrap mode branching. It also includes a few other refactoring such as inlining functions/variables that are not reused and removing the default argument of `evalScript` for better clarity. PR-URL: #24673 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
1 parent 66d8330 commit 5874a03

12 files changed

+190
-124
lines changed
 

‎lib/internal/bootstrap/node.js

+151-112
Original file line numberDiff line numberDiff line change
@@ -104,18 +104,13 @@
104104
}
105105

106106
const { getOptionValue } = NativeModule.require('internal/options');
107-
const helpOption = getOptionValue('--help');
108-
const completionBashOption = getOptionValue('--completion-bash');
109-
const experimentalModulesOption = getOptionValue('--experimental-modules');
110-
const experimentalVMModulesOption =
111-
getOptionValue('--experimental-vm-modules');
112-
const experimentalWorkerOption = getOptionValue('--experimental-worker');
113-
if (helpOption) {
107+
108+
if (getOptionValue('--help')) {
114109
NativeModule.require('internal/print_help').print(process.stdout);
115110
return;
116111
}
117112

118-
if (completionBashOption) {
113+
if (getOptionValue('--completion-bash')) {
119114
NativeModule.require('internal/bash_completion').print(process.stdout);
120115
return;
121116
}
@@ -135,7 +130,7 @@
135130
setupQueueMicrotask();
136131
}
137132

138-
if (experimentalWorkerOption) {
133+
if (getOptionValue('--experimental-worker')) {
139134
setupDOMException();
140135
}
141136

@@ -167,8 +162,10 @@
167162
'DeprecationWarning', 'DEP0062', startup, true);
168163
}
169164

170-
if (experimentalModulesOption || experimentalVMModulesOption) {
171-
if (experimentalModulesOption) {
165+
const experimentalModules = getOptionValue('--experimental-modules');
166+
const experimentalVMModules = getOptionValue('--experimental-vm-modules');
167+
if (experimentalModules || experimentalVMModules) {
168+
if (experimentalModules) {
172169
process.emitWarning(
173170
'The ESM module loader is experimental.',
174171
'ExperimentalWarning', undefined);
@@ -228,22 +225,33 @@
228225

229226
setupAllowedFlags();
230227

231-
// There are various modes that Node can run in. The most common two
232-
// are running from a script and running the REPL - but there are a few
233-
// others like the debugger or running --eval arguments. Here we decide
234-
// which mode we run in.
228+
startExecution();
229+
}
230+
231+
// There are various modes that Node can run in. The most common two
232+
// are running from a script and running the REPL - but there are a few
233+
// others like the debugger or running --eval arguments. Here we decide
234+
// which mode we run in.
235+
function startExecution() {
236+
// This means we are in a Worker context, and any script execution
237+
// will be directed by the worker module.
235238
if (internalBinding('worker').getEnvMessagePort() !== undefined) {
236-
// This means we are in a Worker context, and any script execution
237-
// will be directed by the worker module.
238239
NativeModule.require('internal/worker').setupChild(evalScript);
239-
} else if (NativeModule.exists('_third_party_main')) {
240-
// To allow people to extend Node in different ways, this hook allows
241-
// one to drop a file lib/_third_party_main.js into the build
242-
// directory which will be executed instead of Node's normal loading.
240+
return;
241+
}
242+
243+
// To allow people to extend Node in different ways, this hook allows
244+
// one to drop a file lib/_third_party_main.js into the build
245+
// directory which will be executed instead of Node's normal loading.
246+
if (NativeModule.exists('_third_party_main')) {
243247
process.nextTick(() => {
244248
NativeModule.require('_third_party_main');
245249
});
246-
} else if (process.argv[1] === 'inspect' || process.argv[1] === 'debug') {
250+
return;
251+
}
252+
253+
// `node inspect ...` or `node debug ...`
254+
if (process.argv[1] === 'inspect' || process.argv[1] === 'debug') {
247255
if (process.argv[1] === 'debug') {
248256
process.emitWarning(
249257
'`node debug` is deprecated. Please use `node inspect` instead.',
@@ -254,95 +262,136 @@
254262
process.nextTick(() => {
255263
NativeModule.require('internal/deps/node-inspect/lib/_inspect').start();
256264
});
265+
return;
266+
}
257267

258-
} else if (process.profProcess) {
268+
// `node --prof-process`
269+
// TODO(joyeecheung): use internal/options instead of process.profProcess
270+
if (process.profProcess) {
259271
NativeModule.require('internal/v8_prof_processor');
260-
} else {
261-
// There is user code to be run.
262-
263-
// If this is a worker in cluster mode, start up the communication
264-
// channel. This needs to be done before any user code gets executed
265-
// (including preload modules).
266-
if (process.argv[1] && process.env.NODE_UNIQUE_ID) {
267-
const cluster = NativeModule.require('cluster');
268-
cluster._setupWorker();
269-
// Make sure it's not accidentally inherited by child processes.
270-
delete process.env.NODE_UNIQUE_ID;
272+
return;
273+
}
274+
275+
// There is user code to be run.
276+
prepareUserCodeExecution();
277+
executeUserCode();
278+
}
279+
280+
function prepareUserCodeExecution() {
281+
// If this is a worker in cluster mode, start up the communication
282+
// channel. This needs to be done before any user code gets executed
283+
// (including preload modules).
284+
if (process.argv[1] && process.env.NODE_UNIQUE_ID) {
285+
const cluster = NativeModule.require('cluster');
286+
cluster._setupWorker();
287+
// Make sure it's not accidentally inherited by child processes.
288+
delete process.env.NODE_UNIQUE_ID;
289+
}
290+
291+
// For user code, we preload modules if `-r` is passed
292+
// TODO(joyeecheung): use internal/options instead of
293+
// process._preload_modules
294+
if (process._preload_modules) {
295+
const {
296+
_preloadModules
297+
} = NativeModule.require('internal/modules/cjs/loader');
298+
_preloadModules(process._preload_modules);
299+
}
300+
}
301+
302+
function executeUserCode() {
303+
// User passed `-e` or `--eval` arguments to Node without `-i` or
304+
// `--interactive`.
305+
// Note that the name `forceRepl` is merely an alias of `interactive`
306+
// in code.
307+
// TODO(joyeecheung): use internal/options instead of
308+
// process._eval/process._forceRepl
309+
if (process._eval != null && !process._forceRepl) {
310+
const {
311+
addBuiltinLibsToObject
312+
} = NativeModule.require('internal/modules/cjs/helpers');
313+
addBuiltinLibsToObject(global);
314+
evalScript('[eval]', wrapForBreakOnFirstLine(process._eval));
315+
return;
316+
}
317+
318+
// If the first argument is a file name, run it as a main script
319+
if (process.argv[1] && process.argv[1] !== '-') {
320+
// Expand process.argv[1] into a full path.
321+
const path = NativeModule.require('path');
322+
process.argv[1] = path.resolve(process.argv[1]);
323+
324+
const CJSModule = NativeModule.require('internal/modules/cjs/loader');
325+
326+
// If user passed `-c` or `--check` arguments to Node, check its syntax
327+
// instead of actually running the file.
328+
// TODO(joyeecheung): use internal/options instead of
329+
// process._syntax_check_only
330+
if (process._syntax_check_only != null) {
331+
const fs = NativeModule.require('fs');
332+
// Read the source.
333+
const filename = CJSModule._resolveFilename(process.argv[1]);
334+
const source = fs.readFileSync(filename, 'utf-8');
335+
checkScriptSyntax(source, filename);
336+
process.exit(0);
271337
}
272338

273-
if (process._eval != null && !process._forceRepl) {
274-
// User passed '-e' or '--eval' arguments to Node without '-i' or
275-
// '--interactive'.
276-
preloadModules();
277-
278-
const {
279-
addBuiltinLibsToObject
280-
} = NativeModule.require('internal/modules/cjs/helpers');
281-
addBuiltinLibsToObject(global);
282-
evalScript('[eval]');
283-
} else if (process.argv[1] && process.argv[1] !== '-') {
284-
// Make process.argv[1] into a full path.
285-
const path = NativeModule.require('path');
286-
process.argv[1] = path.resolve(process.argv[1]);
287-
288-
const CJSModule = NativeModule.require('internal/modules/cjs/loader');
289-
290-
preloadModules();
291-
// Check if user passed `-c` or `--check` arguments to Node.
292-
if (process._syntax_check_only != null) {
293-
const fs = NativeModule.require('fs');
294-
// Read the source.
295-
const filename = CJSModule._resolveFilename(process.argv[1]);
296-
const source = fs.readFileSync(filename, 'utf-8');
297-
checkScriptSyntax(source, filename);
298-
process.exit(0);
339+
// Note: this actually tries to run the module as a ESM first if
340+
// --experimental-modules is on.
341+
// TODO(joyeecheung): can we move that logic to here? Note that this
342+
// is an undocumented method available via `require('module').runMain`
343+
CJSModule.runMain();
344+
return;
345+
}
346+
347+
// Create the REPL if `-i` or `--interactive` is passed, or if
348+
// stdin is a TTY.
349+
// Note that the name `forceRepl` is merely an alias of `interactive`
350+
// in code.
351+
if (process._forceRepl || NativeModule.require('tty').isatty(0)) {
352+
const cliRepl = NativeModule.require('internal/repl');
353+
cliRepl.createInternalRepl(process.env, (err, repl) => {
354+
if (err) {
355+
throw err;
299356
}
300-
CJSModule.runMain();
301-
} else {
302-
preloadModules();
303-
// If -i or --interactive were passed, or stdin is a TTY.
304-
if (process._forceRepl || NativeModule.require('tty').isatty(0)) {
305-
// REPL
306-
const cliRepl = NativeModule.require('internal/repl');
307-
cliRepl.createInternalRepl(process.env, (err, repl) => {
308-
if (err) {
309-
throw err;
310-
}
311-
repl.on('exit', () => {
312-
if (repl._flushing) {
313-
repl.pause();
314-
return repl.once('flushHistory', () => {
315-
process.exit();
316-
});
317-
}
357+
repl.on('exit', () => {
358+
if (repl._flushing) {
359+
repl.pause();
360+
return repl.once('flushHistory', () => {
318361
process.exit();
319362
});
320-
});
321-
322-
if (process._eval != null) {
323-
// User passed '-e' or '--eval'
324-
evalScript('[eval]');
325363
}
326-
} else {
327-
// Read all of stdin - execute it.
328-
process.stdin.setEncoding('utf8');
329-
330-
let code = '';
331-
process.stdin.on('data', (d) => {
332-
code += d;
333-
});
334-
335-
process.stdin.on('end', function() {
336-
if (process._syntax_check_only != null) {
337-
checkScriptSyntax(code, '[stdin]');
338-
} else {
339-
process._eval = code;
340-
evalScript('[stdin]');
341-
}
342-
});
343-
}
364+
process.exit();
365+
});
366+
});
367+
368+
// User passed '-e' or '--eval' along with `-i` or `--interactive`
369+
if (process._eval != null) {
370+
evalScript('[eval]', wrapForBreakOnFirstLine(process._eval));
344371
}
372+
return;
345373
}
374+
375+
// Stdin is not a TTY, we will read it and execute it.
376+
readAndExecuteStdin();
377+
}
378+
379+
function readAndExecuteStdin() {
380+
process.stdin.setEncoding('utf8');
381+
382+
let code = '';
383+
process.stdin.on('data', (d) => {
384+
code += d;
385+
});
386+
387+
process.stdin.on('end', () => {
388+
if (process._syntax_check_only != null) {
389+
checkScriptSyntax(code, '[stdin]');
390+
} else {
391+
process._eval = code;
392+
evalScript('[stdin]', wrapForBreakOnFirstLine(process._eval));
393+
}
394+
});
346395
}
347396

348397
function setupTraceCategoryState() {
@@ -651,7 +700,7 @@
651700
return `process.binding('inspector').callAndPauseOnStart(${fn}, {})`;
652701
}
653702

654-
function evalScript(name, body = wrapForBreakOnFirstLine(process._eval)) {
703+
function evalScript(name, body) {
655704
const CJSModule = NativeModule.require('internal/modules/cjs/loader');
656705
const path = NativeModule.require('path');
657706
const cwd = tryGetCwd(path);
@@ -673,16 +722,6 @@
673722
process._tickCallback();
674723
}
675724

676-
// Load preload modules.
677-
function preloadModules() {
678-
if (process._preload_modules) {
679-
const {
680-
_preloadModules
681-
} = NativeModule.require('internal/modules/cjs/loader');
682-
_preloadModules(process._preload_modules);
683-
}
684-
}
685-
686725
function checkScriptSyntax(source, filename) {
687726
const CJSModule = NativeModule.require('internal/modules/cjs/loader');
688727
const vm = NativeModule.require('vm');

‎test/message/assert_throws_stack.out

+1
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,4 @@ AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
1818
at *
1919
at *
2020
at *
21+
at *

‎test/message/core_line_numbers.out

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,4 @@ RangeError: Invalid input
1212
at tryModuleLoad (internal/modules/cjs/loader.js:*:*)
1313
at Function.Module._load (internal/modules/cjs/loader.js:*:*)
1414
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
15-
at startup (internal/bootstrap/node.js:*:*)
15+
at executeUserCode (internal/bootstrap/node.js:*:*)

‎test/message/error_exit.out

+2-1
Original file line numberDiff line numberDiff line change
@@ -14,5 +14,6 @@ AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
1414
at tryModuleLoad (internal/modules/cjs/loader.js:*:*)
1515
at Function.Module._load (internal/modules/cjs/loader.js:*:*)
1616
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
17+
at executeUserCode (internal/bootstrap/node.js:*:*)
18+
at startExecution (internal/bootstrap/node.js:*:*)
1719
at startup (internal/bootstrap/node.js:*:*)
18-
at bootstrapNodeJSCore (internal/bootstrap/node.js:*:*)

‎test/message/eval_messages.out

+8
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ SyntaxError: Strict mode code may not include a with statement
99
at Object.<anonymous> ([eval]-wrapper:*:*)
1010
at Module._compile (internal/modules/cjs/loader.js:*:*)
1111
at evalScript (internal/bootstrap/node.js:*:*)
12+
at executeUserCode (internal/bootstrap/node.js:*:*)
13+
at startExecution (internal/bootstrap/node.js:*:*)
1214
at startup (internal/bootstrap/node.js:*:*)
1315
at bootstrapNodeJSCore (internal/bootstrap/node.js:*:*)
1416
42
@@ -24,6 +26,8 @@ Error: hello
2426
at Object.<anonymous> ([eval]-wrapper:*:*)
2527
at Module._compile (internal/modules/cjs/loader.js:*:*)
2628
at evalScript (internal/bootstrap/node.js:*:*)
29+
at executeUserCode (internal/bootstrap/node.js:*:*)
30+
at startExecution (internal/bootstrap/node.js:*:*)
2731
at startup (internal/bootstrap/node.js:*:*)
2832
at bootstrapNodeJSCore (internal/bootstrap/node.js:*:*)
2933

@@ -38,6 +42,8 @@ Error: hello
3842
at Object.<anonymous> ([eval]-wrapper:*:*)
3943
at Module._compile (internal/modules/cjs/loader.js:*:*)
4044
at evalScript (internal/bootstrap/node.js:*:*)
45+
at executeUserCode (internal/bootstrap/node.js:*:*)
46+
at startExecution (internal/bootstrap/node.js:*:*)
4147
at startup (internal/bootstrap/node.js:*:*)
4248
at bootstrapNodeJSCore (internal/bootstrap/node.js:*:*)
4349
100
@@ -52,6 +58,8 @@ ReferenceError: y is not defined
5258
at Object.<anonymous> ([eval]-wrapper:*:*)
5359
at Module._compile (internal/modules/cjs/loader.js:*:*)
5460
at evalScript (internal/bootstrap/node.js:*:*)
61+
at executeUserCode (internal/bootstrap/node.js:*:*)
62+
at startExecution (internal/bootstrap/node.js:*:*)
5563
at startup (internal/bootstrap/node.js:*:*)
5664
at bootstrapNodeJSCore (internal/bootstrap/node.js:*:*)
5765

0 commit comments

Comments
 (0)