8000 console: refactor inspector console extension installation · nodejs/node@3c661f0 · GitHub
[go: up one dir, main page]
Skip to content
< 8000 /react-partial>

Commit 3c661f0

Browse files
joyeecheungaddaleax
joyeecheung
authored and
addaleax
committed
console: refactor inspector console extension installation
- Instead of creating the console extensions eagerly during bootstrap and storing them on an object, wrap the code into a function to be called during `installAdditionalCommandLineAPI` only when the extensions are actually needed, which may not even happen if the user do not use the console in an inspector session, and does not need to happen during bootstrap unconditionally. - Simplify the console methods wrapping and move the `consoleFromVM` storage to `internal/util/inspector.js` PR-URL: #25450 Reviewed-By: John-David Dalton <john.david.dalton@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
1 parent 647a37f commit 3c661f0

File tree

9 files changed

+67
-90
lines changed
  • src

    • 9 files changed

    +67
    -90
    lines changed

    ‎lib/inspector.js‎

    Lines changed: 1 addition & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -106,6 +106,6 @@ module.exports = {
    106106
    url: url,
    107107
    // This is dynamically added during bootstrap,
    108108
    // where the console from the VM is still available
    109-
    console: require('internal/console/inspector').consoleFromVM,
    109+
    console: require('internal/util/inspector').consoleFromVM,
    110110
    Session
    111111
    };

    ‎lib/internal/bootstrap/node.js‎

    Lines changed: 8 additions & 4 deletions
    Original file line numberDiff line numberDiff line change
    @@ -705,13 +705,17 @@ function setupGlobalConsole() {
    705705
    value: consoleFromNode,
    706706
    writable: true
    707707
    });
    708-
    // TODO(joyeecheung): can we skip this if inspector is not active?
    708+
    709709
    if (config.hasInspector) {
    710-
    const inspector =
    711-
    NativeModule.require('internal/console/inspector');
    712-
    inspector.addInspectorApis(consoleFromNode, consoleFromVM);
    710+
    const inspector = NativeModule.require('internal/util/inspector');
    713711
    // This will be exposed by `require('inspector').console` later.
    714712
    inspector.consoleFromVM = consoleFromVM;
    713+
    // TODO(joyeecheung): postpone this until the first time inspector
    714+
    // is activated.
    715+
    inspector.wrapConsole(consoleFromNode, consoleFromVM);
    716+
    const { setConsoleExtensionInstaller } = internalBinding('inspector');
    717+
    // Setup inspector command line API.
    718+
    setConsoleExtensionInstaller(inspector.installConsoleExtensions);
    715719
    }
    716720
    }
    717721

    ‎lib/internal/console/inspector.js‎

    Lines changed: 0 additions & 52 deletions
    This file was deleted.

    ‎lib/internal/util/inspector.js‎

    Lines changed: 46 additions & 5 deletions
    Original file line numberDiff line numberDiff line change
    @@ -1,11 +1,9 @@
    11
    'use strict';
    22

    3-
    const { hasInspector } = internalBinding('config');
    4-
    const inspector = hasInspector ? require('inspector') : undefined;
    5-
    63
    let session;
    7-
    84
    function sendInspectorCommand(cb, onError) {
    5+
    const { hasInspector } = internalBinding('config');
    6+
    const inspector = hasInspector ? require('inspector') : undefined;
    97
    if (!hasInspector) return onError();
    108
    if (session === undefined) session = new inspector.Session();
    119
    try {
    @@ -20,6 +18,49 @@ function sendInspectorCommand(cb, onError) {
    2018
    }
    2119
    }
    2220

    21+
    // Create a special require function for the inspector command line API
    22+
    function installConsoleExtensions(commandLineApi) {
    23+
    if (commandLineApi.require) { return; }
    24+
    const { tryGetCwd } = require('internal/process/execution');
    25+
    const CJSModule = require('internal/modules/cjs/loader');
    26+
    const { makeRequireFunction } = require('internal/modules/cjs/helpers');
    27+
    const consoleAPIModule = new CJSModule('<inspector console>');
    28+
    const cwd = tryGetCwd();
    29+
    consoleAPIModule.paths =
    30+
    CJSModule._nodeModulePaths(cwd).concat(CJSModule.globalPaths);
    31+
    commandLineApi.require = makeRequireFunction(consoleAPIModule);
    32+
    }
    33+
    34+
    // Wrap a console implemented by Node.js with features from the VM inspector
    35+
    function wrapConsole(consoleFromNode, consoleFromVM) {
    36+
    const config = {};
    37+
    const { consoleCall } = internalBinding('inspector');
    38+
    for (const key of Object.keys(consoleFromVM)) {
    39+
    // If global console has the same method as inspector console,
    40+
    // then wrap these two methods into one. Native wrapper will preserve
    41+
    // the original stack.
    42+
    if (consoleFromNode.hasOwnProperty(key)) {
    43+
    consoleFromNode[key] = consoleCall.bind(consoleFromNode,
    44+
    consoleFromVM[key],
    45+
    consoleFromNode[key],
    46+
    config);
    47+
    } else {
    48+
    // Add additional console APIs from the inspector
    49+
    consoleFromNode[key] = consoleFromVM[key];
    50+
    }
    51+
    }
    52+
    }
    53+
    54+
    // Stores the console from VM, should be set during bootstrap.
    55+
    let consoleFromVM;
    2356
    module.exports = {
    24-
    sendInspectorCommand
    57+
    installConsoleExtensions,
    58+
    sendInspectorCommand,
    59+
    wrapConsole,
    60+
    get consoleFromVM() {
    61+
    return consoleFromVM;
    62+
    },
    63+
    set consoleFromVM(val) {
    64+
    consoleFromVM = val;
    65+
    }
    2566
    };

    ‎node.gyp‎

    Lines changed: 0 additions & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -98,7 +98,6 @@
    9898
    'lib/internal/cluster/worker.js',
    9999
    'lib/internal/console/constructor.js',
    100100
    'lib/internal/console/global.js',
    101-
    'lib/internal/console/inspector.js',
    102101
    'lib/internal/coverage-gen/with_profiler.js',
    103102
    'lib/internal/coverage-gen/with_instrumentation.js',
    104103
    'lib/internal/crypto/certificate.js',

    ‎src/env.cc‎

    Lines changed: 0 additions & 7 deletions
    Original file line numberDiff line numberDiff line change
    @@ -352,13 +352,6 @@ void Environment::Start(const std::vector<std::string>& args,
    352352
    static uv_once_t init_once = UV_ONCE_INIT;
    353353
    uv_once(&init_once, InitThreadLocalOnce);
    354354
    uv_key_set(&thread_local_env, this);
    355-
    356-
    #if HAVE_INSPECTOR
    357-
    // This needs to be set before we start the inspector
    358-
    Local<Object> obj = Object::New(isolate());
    359-
    CHECK(obj->SetPrototype(context(), Null(isolate())).FromJust());
    360-
    set_inspector_console_api_object(obj);
    361-
    #endif // HAVE_INSPECTOR
    362355
    }
    363356

    364357
    void Environment::RegisterHandleCleanups() {

    ‎src/env.h‎

    Lines changed: 1 addition & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -354,7 +354,7 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2;
    354354
    V(http2settings_constructor_template, v8::ObjectTemplate) \
    355355
    V(http2stream_constructor_template, v8::ObjectTemplate) \
    356356
    V(immediate_callback_function, v8::Function) \
    357-
    V(inspector_console_api_object, v8::Object) \
    357+
    V(inspector_console_extension_installer, v8::Function) \
    358358
    V(libuv_stream_wrap_ctor_template, v8::FunctionTemplate) \
    359359
    V(message_port, v8::Object) \
    360360
    V(message_port_constructor_template, v8::FunctionTemplate) \

    ‎src/inspector_agent.cc‎

    Lines changed: 5 additions & 11 deletions
    Original file line numberDiff line numberDiff line change
    @@ -32,7 +32,6 @@ namespace {
    3232

    3333
    using node::FatalError;
    3434

    35-
    using v8::Array;
    3635
    using v8::Context;
    3736
    using v8::Function;
    3837
    using v8::HandleScope;
    @@ -493,16 +492,11 @@ class NodeInspectorClient : public V8InspectorClient {
    493492

    494493
    void installAdditionalCommandLineAPI(Local<Context> context,
    495494
    Local<Object> target) override {
    496-
    Local<Object> console_api = env_->inspector_console_api_object();
    497-
    CHECK(!console_api.IsEmpty());
    498-
    499-
    Local<Array> properties =
    500-
    console_api->GetOwnPropertyNames(context).ToLocalChecked();
    501-
    for (uint32_t i = 0; i < properties->Length(); ++i) {
    502-
    Local<Value> key = properties->Get(context, i).ToLocalChecked();
    503-
    target->Set(context,
    504-
    key,
    505-
    console_api->Get(context, key).ToLocalChecked()).FromJust();
    495+
    Local<Function> installer = env_->inspector_console_extension_installer();
    496+
    if (!installer.IsEmpty()) {
    497+
    Local<Value> argv[] = {target};
    498+
    // If there is an exception, proceed in JS land
    499+
    USE(installer->Call(context, target, arraysize(argv), argv));
    506500
    }
    507501
    }
    508502

    ‎src/inspector_js_api.cc‎

    Lines changed: 6 additions & 8 deletions
    Original file line numberDiff line numberDiff line change
    @@ -122,16 +122,13 @@ static bool InspectorEnabled(Environment* env) {
    122122
    return agent->IsActive();
    123123
    }
    124124

    125-
    void AddCommandLineAPI(const FunctionCallbackInfo<Value>& info) {
    125+
    void SetConsoleExtensionInstaller(const FunctionCallbackInfo<Value>& info) {
    126126
    auto env = Environment::GetCurrent(info);
    127-
    Local<Context> context = env->context();
    128127

    129-
    // inspector.addCommandLineAPI takes 2 arguments: a string and a value.
    130-
    CHECK_EQ(info.Length(), 2);
    131-
    CHECK(info[0]->IsString());
    128+
    CHECK_EQ(info.Length(), 1);
    129+
    CHECK(info[0]->IsFunction());
    132130

    133-
    Local<Object> console_api = env->inspector_console_api_object();
    134-
    console_api->Set(context, info[0], info[1]).FromJust();
    131+
    env->set_inspector_console_extension_installer(info[0].As<Function>());
    135132
    }
    136133

    137134
    void CallAndPauseOnStart(const FunctionCallbackInfo<v8::Value>& args) {
    @@ -279,7 +276,8 @@ void Initialize(Local<Object> target, Local<Value> unused,
    279276

    280277
    Agent* agent = env->inspector_agent();
    281278
    env->SetMethod(target, "consoleCall", InspectorConsoleCall);
    282-
    env->SetMethod(target, "addCommandLineAPI", AddCommandLineAPI);
    279+
    env->SetMethod(
    280+
    target, "setConsoleExtensionInstaller", SetConsoleExtensionInstaller);
    283281
    if (agent->WillWaitForConnect())
    284282
    env->SetMethod(target, "callAndPauseOnStart", CallAndPauseOnStart);
    285283
    env->SetMethod(target, "open", Open);

    0 commit comments

    Comments
     (0)
    0