src,lib: prefer internal/options over process._foo

This addresses a couple `TODO` comments and allows us to remove a number of underscored properties from `process` (in a semver-major follow-up). PR-URL: #25063 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs · Dec 15, 2018 · 37ba20112a1ba469f4e759811af26d64d6dd406a · 37ba201
1 parent b1500d9
commit 37ba20112a1ba469f4e759811af26d64d6dd406a
Unified Split

Showing with 18 additions and 19 deletions.

+10 −15 lib/internal/bootstrap/node.js

+8 −4 src/node.cc
diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
@@ -30,6 +30,7 @@ const {
 const { internalBinding, NativeModule } = loaderExports;
 const exceptionHandlerState = { captureFn: null };
+  
+let getOptionValue;
 function startup() {
  setupTraceCategoryState();
@@ -105,7 +106,7 @@ function startup() {
    NativeModule.require('internal/inspector_async_hook').setup();
  }
-  
-  const { getOptionValue } = NativeModule.require('internal/options');
+  
+  getOptionValue = NativeModule.require('internal/options').getOptionValue;
  if (getOptionValue('--help')) {
    NativeModule.require('internal/print_help').print(process.stdout);
@@ -265,8 +266,7 @@ function startExecution() {
  }
  // `node --prof-process`
-  
-  // TODO(joyeecheung): use internal/options instead of process.profProcess
-  
-  if (process.profProcess) {
+  
+  if (getOptionValue('--prof-process')) {
    NativeModule.require('internal/v8_prof_processor');
    return;
  }
@@ -288,13 +288,12 @@ function prepareUserCodeExecution() {
  }
  // For user code, we preload modules if `-r` is passed
-  
-  // TODO(joyeecheung): use internal/options instead of
-  
-  // process._preload_modules
-  
-  if (process._preload_modules) {
+  
+  const preloadModules = getOptionValue('--require');
+  
+  if (preloadModules) {
    const {
      _preloadModules
    } = NativeModule.require('internal/modules/cjs/loader');
-  
-    _preloadModules(process._preload_modules);
+  
+    _preloadModules(preloadModules);
  }
 }
@@ -303,14 +302,12 @@ function executeUserCode() {
  // `--interactive`.
  // Note that the name `forceRepl` is merely an alias of `interactive`
  // in code.
-  
-  // TODO(joyeecheung): use internal/options instead of
-  
-  // process._eval/process._forceRepl
-  
-  if (process._eval != null && !process._forceRepl) {
+  
+  if (getOptionValue('[has_eval_string]') && !getOptionValue('--interactive')) {
    const {
      addBuiltinLibsToObject
    } = NativeModule.require('internal/modules/cjs/helpers');
    addBuiltinLibsToObject(global);
-  
-    evalScript('[eval]', wrapForBreakOnFirstLine(process._eval));
+  
+    evalScript('[eval]', wrapForBreakOnFirstLine(getOptionValue('--eval')));
    return;
  }
@@ -324,9 +321,7 @@ function executeUserCode() {
    // If user passed `-c` or `--check` arguments to Node, check its syntax
    // instead of actually running the file.
-  
-    // TODO(joyeecheung): use internal/options instead of
-  
-    // process._syntax_check_only
-  
-    if (process._syntax_check_only != null) {
+  
+    if (getOptionValue('--check')) {
      const fs = NativeModule.require('fs');
      // Read the source.
      const filename = CJSModule._resolveFilename(process.argv[1]);
@@ -682,7 +677,7 @@ function evalScript(name, body) {
                  `${JSON.stringify(body)}, { filename: ` +
                  `${JSON.stringify(name)}, displayErrors: true });\n`;
  const result = module._compile(script, `${name}-wrapper`);
-  
-  if (process._print_eval) console.log(result);
+  
+  if (getOptionValue('--print')) console.log(result);
  // Handle any nextTicks added in the first tick of the program.
  process._tickCallback();
 }

diff --git a/src/node.cc b/src/node.cc
@@ -1002,6 +1002,7 @@ void SetupProcessObject(Environment* env,
                             GetParentProcessId).FromJust());
  // -e, --eval
+  
+  // TODO(addaleax): Remove this.
  if (env->options()->has_eval_string) {
    READONLY_PROPERTY(process,
                      "_eval",
@@ -1012,23 +1013,27 @@ void SetupProcessObject(Environment* env,
  }
  // -p, --print
+  
+  // TODO(addaleax): Remove this.
  if (env->options()->print_eval) {
    READONLY_PROPERTY(process, "_print_eval", True(env->isolate()));
  }
  // -c, --check
+  
+  // TODO(addaleax): Remove this.
  if (env->options()->syntax_check_only) {
    READONLY_PROPERTY(process, "_syntax_check_only", True(env->isolate()));
  }
  // -i, --interactive
+  
+  // TODO(addaleax): Remove this.
  if (env->options()->force_repl) {
    READONLY_PROPERTY(process, "_forceRepl", True(env->isolate()));
  }
  // -r, --require
-  
-  std::vector<std::string> preload_modules =
-  
-      std::move(env->options()->preload_modules);
+  
+  // TODO(addaleax): Remove this.
+  
+  const std::vector<std::string>& preload_modules =
+  
+      env->options()->preload_modules;
  if (!preload_modules.empty()) {
    Local<Array> array = Array::New(env->isolate());
    for (unsigned int i = 0; i < preload_modules.size(); ++i) {
@@ -1041,8 +1046,6 @@ void SetupProcessObject(Environment* env,
    READONLY_PROPERTY(process,
                      "_preload_modules",
                      array);
-  
-
-  
-    preload_modules.clear();
  }
  // --no-deprecation
@@ -1071,6 +1074,7 @@ void SetupProcessObject(Environment* env,
 #endif  // NODE_NO_BROWSER_GLOBALS
  // --prof-process
+  
+  // TODO(addaleax): Remove this.
  if (env->options()->prof_process) {
    READONLY_PROPERTY(process, "profProcess", True(env->isolate()));
  }