process: expose process.features.inspector

Instead of using process.config.variables.v8_enable_inspector
to detect whether inspector is enabled in the build.

PR-URL: #25819
Refs: #25343
Reviewed-By: Eugene Ostroukhov <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>

Author: joyeecheung

lib/internal/bootstrap/node.js

@@ -292,12 +292,13 @@ process.assert = deprecate(
 
 // TODO(joyeecheung): this property has not been well-maintained, should we
 // deprecate it in favor of a better API?
-const { isDebugBuild, hasOpenSSL } = config;
+const { isDebugBuild, hasOpenSSL, hasInspector } = config;
 Object.defineProperty(process, 'features', {
   enumerable: true,
   writable: false,
   configurable: false,
   value: {
+    inspector: hasInspector,
     debug: isDebugBuild,
     uv: true,
     ipv6: true,  // TODO(bnoordhuis) ping libuv

lib/internal/util/inspector.js

@@ -3,8 +3,8 @@
 let session;
 function sendInspectorCommand(cb, onError) {
   const { hasInspector } = internalBinding('config');
-  const inspector = hasInspector ? require('inspector') : undefined;
   if (!hasInspector) return onError();
+  const inspector = require('inspector');
   if (session === undefined) session = new inspector.Session();
   try {
     session.connect();

src/node_config.cc

@@ -57,12 +57,6 @@ static void Initialize(Local<Object> target,
   READONLY_TRUE_PROPERTY(target, "hasTracing");
 #endif
 
-#if HAVE_INSPECTOR
-  READONLY_TRUE_PROPERTY(target, "hasInspector");
-#else
-  READONLY_FALSE_PROPERTY(target, "hasInspector");
-#endif
-
 #if !defined(NODE_WITHOUT_NODE_OPTIONS)
   READONLY_TRUE_PROPERTY(target, "hasNodeOptions");
 #endif
@@ -73,6 +67,12 @@ static void Initialize(Local<Object> target,
 
 #endif  // NODE_HAVE_I18N_SUPPORT
 
+#if HAVE_INSPECTOR
+  READONLY_TRUE_PROPERTY(target, "hasInspector");
+#else
+  READONLY_FALSE_PROPERTY(target, "hasInspector");
+#endif
+
   READONLY_PROPERTY(target,
                     "bits",
                     Number::New(env->isolate(), 8 * sizeof(intptr_t)));

test/common/index.js

@@ -80,8 +80,7 @@ if (process.argv.length === 2 &&
           hasCrypto &&
           // If the binary is build without `intl` the inspect option is
           // invalid. The test itself should handle this case.
-          (process.config.variables.v8_enable_inspector !== 0 ||
-            !flag.startsWith('--inspect'))) {
+          (process.features.inspector || !flag.startsWith('--inspect'))) {
         throw new Error(`Test has to be started with the flag: '${flag}'`);
       }
     }
@@ -629,7 +628,7 @@ function expectsError(fn, settings, exact) {
 }
 
 function skipIfInspectorDisabled() {
-  if (process.config.variables.v8_enable_inspector === 0) {
+  if (!process.features.inspector) {
     skip('V8 inspector is disabled');
   }
 }

test/known_issues/test-inspector-cluster-port-clash.js

@@ -13,7 +13,7 @@ const assert = require('assert');
 
 // This following check should be replaced by common.skipIfInspectorDisabled()
 // if moved out of the known_issues directory.
-if (process.config.variables.v8_enable_inspector === 0) {
+if (!process.features.inspector) {
   // When the V8 inspector is disabled, using either --without-inspector or
   // --without-ssl, this test will not fail which it is expected to do.
   // The following fail will allow this test to be skipped by failing it.

test/parallel/test-cli-bad-options.js

@@ -6,7 +6,7 @@ require('../common');
 const assert = require('assert');
 const spawn = require('child_process').spawnSync;
 
-if (process.config.variables.v8_enable_inspector === 1) {
+if (process.features.inspector) {
   requiresArgument('--inspect-port');
   requiresArgument('--inspect-port=');
   requiresArgument('--debug-port');

test/parallel/test-cli-node-print-help.js

@@ -22,10 +22,9 @@ function startPrintHelpTest() {
 }
 
 function validateNodePrintHelp() {
-  const config = process.config;
   const HAVE_OPENSSL = common.hasCrypto;
   const NODE_HAVE_I18N_SUPPORT = common.hasIntl;
-  const HAVE_INSPECTOR = config.variables.v8_enable_inspector === 1;
+  const HAVE_INSPECTOR = process.features.inspector;
 
   const cliHelpOptions = [
     { compileConstant: HAVE_OPENSSL,

test/parallel/test-module-cjs-helpers.js

@@ -5,7 +5,5 @@ require('../common');
 const assert = require('assert');
 const { builtinLibs } = require('internal/modules/cjs/helpers');
 
-const hasInspector = process.config.variables.v8_enable_inspector === 1;
-
-const expectedLibs = hasInspector ? 34 : 33;
+const expectedLibs = process.features.inspector ? 34 : 33;
 assert.strictEqual(builtinLibs.length, expectedLibs);

test/parallel/test-process-env-allowed-flags.js

@@ -16,7 +16,7 @@ require('../common');
     'r',
     '--stack-trace-limit=100',
     '--stack-trace-limit=-=xX_nodejs_Xx=-',
-  ].concat(process.config.variables.v8_enable_inspector ? [
+  ].concat(process.features.inspector ? [
     '--inspect-brk',
     'inspect-brk',
     '--inspect_brk',

test/parallel/test-process-features.js

@@ -6,6 +6,7 @@ const assert = require('assert');
 const keys = new Set(Object.keys(process.features));
 
 assert.deepStrictEqual(keys, new Set([
+  'inspector',
   'debug',
   'uv',
   'ipv6',