Skip to content

Commit

Permalink
src: use internalBinding('config').hasInspector in JS land
Browse files Browse the repository at this point in the history
Instead of `process.config.variables.v8_enable_inspector`
which depends on the variable name in gyp files, or detecting
`internalBinding('inspector').Connection`.

PR-URL: nodejs#25291
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
joyeecheung authored and refack committed Jan 10, 2019
1 parent 0bdb88a commit 468b245
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 12 deletions.
10 changes: 6 additions & 4 deletions lib/inspector.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
'use strict';

const EventEmitter = require('events');
const {
ERR_INSPECTOR_ALREADY_CONNECTED,
ERR_INSPECTOR_CLOSED,
Expand All @@ -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');
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/bootstrap/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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',
Expand Down
7 changes: 4 additions & 3 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
const { internalBinding, NativeModule } = loaderExports;

const { getOptionValue } = NativeModule.require('internal/options');
const config = internalBinding('config');

function startup() {
setupTraceCategoryState();
Expand Down Expand Up @@ -164,7 +165,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();
}

Expand Down Expand Up @@ -280,7 +281,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,
Expand Down Expand Up @@ -630,7 +631,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);
Expand Down
5 changes: 3 additions & 2 deletions lib/internal/process/coverage.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/util/inspector.js
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
6 changes: 6 additions & 0 deletions src/node_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 468b245

Please sign in to comment.