src: use internalBinding('config').hasInspector in JS land

Instead of `process.config.variables.v8_enable_inspector` which depends on the variable name in gyp files, or detecting `internalBinding('inspector').Connection`. PR-URL: #25291 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs · Jan 15, 2019 · 582c34d · 582c34d
1 parent 59b5e77
commit 582c34d
Show file tree

Hide file tree

Showing 6 changed files with 22 additions and 12 deletions.
diff --git a/lib/inspector.js b/lib/inspector.js
@@ -1,6 +1,5 @@
 'use strict';
 
-const EventEmitter = require('events');
 const {
   ERR_INSPECTOR_ALREADY_CONNECTED,
   ERR_INSPECTOR_CLOSED,
@@ -9,13 +8,16 @@ const {
   ERR_INVALID_ARG_TYPE,
   ERR_INVALID_CALLBACK
 } = require('internal/errors').codes;
+
+const { hasInspector } = internalBinding('config');
+if (!hasInspector)
+  throw new ERR_INSPECTOR_NOT_AVAILABLE();
+
+const EventEmitter = require('events');
 const { validateString } = require('internal/validators');
 const util = require('util');
 const { Connection, open, url } = internalBinding('inspector');
 
-if (!Connection)
-  throw new ERR_INSPECTOR_NOT_AVAILABLE();
-
 const connectionSymbol = Symbol('connectionProperty');
 const messageCallbacksSymbol = Symbol('messageCallbacks');
 const nextIdSymbol = Symbol('nextId');

diff --git a/lib/internal/bootstrap/cache.js b/lib/internal/bootstrap/cache.js
@@ -9,7 +9,7 @@ const { NativeModule } = require('internal/bootstrap/loaders');
 const {
   source, getCodeCache, compileFunction
 } = internalBinding('native_module');
-const { hasTracing } = process.binding('config');
+const { hasTracing, hasInspector } = process.binding('config');
 
 const depsModule = Object.keys(source).filter(
   (key) => NativeModule.isDepsModule(key) || key.startsWith('internal/deps')
@@ -33,7 +33,7 @@ const cannotUseCache = [
 
 // Skip modules that cannot be required when they are not
 // built into the binary.
-if (process.config.variables.v8_enable_inspector !== 1) {
+if (!hasInspector) {
   cannotUseCache.push(
     'inspector',
     'internal/util/inspector',

diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
@@ -19,6 +19,7 @@
 const { internalBinding, NativeModule } = loaderExports;
 
 const { getOptionValue } = NativeModule.require('internal/options');
+const config = internalBinding('config');
 
 function startup() {
   setupTraceCategoryState();
@@ -156,7 +157,7 @@ function startup() {
     NativeModule.require('internal/process/coverage').setupExitHooks();
   }
 
-  if (process.config.variables.v8_enable_inspector) {
+  if (config.hasInspector) {
     NativeModule.require('internal/inspector_async_hook').setup();
   }
 
@@ -296,7 +297,7 @@ function startup() {
 
   // TODO(joyeecheung): this property has not been well-maintained, should we
   // deprecate it in favor of a better API?
-  const { isDebugBuild, hasOpenSSL } = internalBinding('config');
+  const { isDebugBuild, hasOpenSSL } = config;
   Object.defineProperty(process, 'features', {
     enumerable: true,
     writable: false,
@@ -636,7 +637,7 @@ function setupGlobalConsole() {
     writable: true
   });
   // TODO(joyeecheung): can we skip this if inspector is not active?
-  if (process.config.variables.v8_enable_inspector) {
+  if (config.hasInspector) {
     const inspector =
       NativeModule.require('internal/console/inspector');
     inspector.addInspectorApis(consoleFromNode, consoleFromVM);

diff --git a/lib/internal/process/coverage.js b/lib/internal/process/coverage.js
@@ -51,12 +51,13 @@ function disableAllAsyncHooks() {
 exports.writeCoverage = writeCoverage;
 
 function setup() {
-  const { Connection } = internalBinding('inspector');
-  if (!Connection) {
+  const { hasInspector } = internalBinding('config');
+  if (!hasInspector) {
     process._rawDebug('inspector not enabled');
     return;
   }
 
+  const { Connection } = internalBinding('inspector');
   coverageConnection = new Connection((res) => {
     if (coverageConnection._coverageCallback) {
       coverageConnection._coverageCallback(res);

diff --git a/lib/internal/util/inspector.js b/lib/internal/util/inspector.js
@@ -1,6 +1,6 @@
 'use strict';
 
-const hasInspector = process.config.variables.v8_enable_inspector === 1;
+const { hasInspector } = internalBinding('config');
 const inspector = hasInspector ? require('inspector') : undefined;
 
 let session;

diff --git a/src/node_config.cc b/src/node_config.cc
@@ -57,6 +57,12 @@ 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