Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
joyeecheung wants to merge 2 commits into from
Closed

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ac1a309
src: use `internalBinding('config').hasInspector` in JS land
joyeecheung Dec 26, 2018
b249b07
fixup! src: use `internalBinding('config').hasInspector` in JS land
joyeecheung Jan 2, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
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`.
  • Loading branch information
@joyeecheung
joyeecheung committed Jan 2, 2019
commit ac1a3091f38af973d54482b927dc52891c739494
10 changes: 6 additions & 4 deletions lib/inspector.js
View file
Open in desktop
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
View file
Open in desktop
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) {
Copy link
Copy Markdown
Member

@lpinca lpinca Jan 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be !hasInspector

cannotUseCache.push(
'inspector',
'internal/util/inspector',
Expand Down
7 changes: 4 additions & 3 deletions lib/internal/bootstrap/node.js
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion lib/internal/util/inspector.js
View file
Open in desktop
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
View file
Open in desktop
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