Skip to content

Commit

Permalink
src,lib: migrate to console on context's extra binding
Browse files Browse the repository at this point in the history
Since `globalThis.console` is not an ECMAScript defined
builtin, V8's globally installed `console` implementation
is been moved to the context's extra binding object.

We need to migrate to that one before the globally
installed console object is removed in V8.

PR-URL: #43142
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
  • Loading branch information
legendecas authored and bengl committed May 30, 2022
1 parent 9539cfa commit 41b69e3
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 21 deletions.
7 changes: 3 additions & 4 deletions lib/inspector.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ const {
open,
url,
isEnabled,
waitForDebugger
waitForDebugger,
console,
} = internalBinding('inspector');

const connectionSymbol = Symbol('connectionProperty');
Expand Down Expand Up @@ -188,8 +189,6 @@ module.exports = {
close: process._debugEnd,
url,
waitForDebugger: inspectorWaitForDebugger,
// This is dynamically added during bootstrap,
// where the console from the VM is still available
console: require('internal/util/inspector').consoleFromVM,
console,
Session
};
10 changes: 3 additions & 7 deletions lib/internal/bootstrap/browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,9 @@ const {
} = require('internal/util');
const config = internalBinding('config');

// Override global console from the one provided by the VM
// to the one implemented by Node.js
// https://console.spec.whatwg.org/#console-namespace
exposeNamespace(globalThis, 'console',
createGlobalConsole(globalThis.console));
createGlobalConsole());

const { URL, URLSearchParams } = require('internal/url');
// https://url.spec.whatwg.org/#url
Expand Down Expand Up @@ -71,16 +69,14 @@ defineOperation(globalThis, 'setTimeout', timers.setTimeout);
defineReplacableAttribute(globalThis, 'performance',
require('perf_hooks').performance);

function createGlobalConsole(consoleFromVM) {
function createGlobalConsole() {
const consoleFromNode =
require('internal/console/global');
if (config.hasInspector) {
const inspector = require('internal/util/inspector');
// This will be exposed by `require('inspector').console` later.
inspector.consoleFromVM = consoleFromVM;
// TODO(joyeecheung): postpone this until the first time inspector
// is activated.
inspector.wrapConsole(consoleFromNode, consoleFromVM);
inspector.wrapConsole(consoleFromNode);
const { setConsoleExtensionInstaller } = internalBinding('inspector');
// Setup inspector command line API.
setConsoleExtensionInstaller(inspector.installConsoleExtensions);
Expand Down
12 changes: 2 additions & 10 deletions lib/internal/util/inspector.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ function installConsoleExtensions(commandLineApi) {
}

// Wrap a console implemented by Node.js with features from the VM inspector
function wrapConsole(consoleFromNode, consoleFromVM) {
const { consoleCall } = internalBinding('inspector');
function wrapConsole(consoleFromNode) {
const { consoleCall, console: consoleFromVM } = internalBinding('inspector');
for (const key of ObjectKeys(consoleFromVM)) {
// If global console has the same method as inspector console,
// then wrap these two methods into one. Native wrapper will preserve
Expand All @@ -61,16 +61,8 @@ function wrapConsole(consoleFromNode, consoleFromVM) {
}
}

// Stores the console from VM, should be set during bootstrap.
let consoleFromVM;
module.exports = {
installConsoleExtensions,
sendInspectorCommand,
wrapConsole,
get consoleFromVM() {
return consoleFromVM;
},
set consoleFromVM(val) {
consoleFromVM = val;
}
};
12 changes: 12 additions & 0 deletions src/inspector_js_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,18 @@ void Initialize(Local<Object> target, Local<Value> unused,
env->SetMethod(target, "registerAsyncHook", RegisterAsyncHookWrapper);
env->SetMethodNoSideEffect(target, "isEnabled", IsEnabled);

Local<String> console_string =
FIXED_ONE_BYTE_STRING(env->isolate(), "console");

// Grab the console from the binding object and expose those to our binding
// layer.
Local<Object> binding = context->GetExtrasBindingObject();
target
->Set(context,
console_string,
binding->Get(context, console_string).ToLocalChecked())
.Check();

JSBindingsConnection<LocalConnection>::Bind(env, target);
JSBindingsConnection<MainThreadConnection>::Bind(env, target);
}
Expand Down

0 comments on commit 41b69e3

Please sign in to comment.