From 6223236151f25975e380eef8470243ff8cf3f61d Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Fri, 12 Oct 2018 08:38:40 -0700 Subject: [PATCH] lib: make the global console [[Prototype]] an empty object From the WHATWG console spec: > For historical web-compatibility reasons, the namespace object for > console must have as its [[Prototype]] an empty object, created as > if by ObjectCreate(%ObjectPrototype%), instead of %ObjectPrototype%. Since in Node.js, the Console constructor has been exposed through require('console'), we need to keep the Console constructor but we cannot actually use `new Console` to construct the global console. This patch changes the prototype chain of the global console object, so the console.Console.prototype is not in the global console prototype chain anymore. ``` const proto = Object.getPrototypeOf(global.console); // Before this patch proto.constructor === global.console.Console // After this patch proto.constructor === Object ``` But, we still maintain that ``` global.console instanceof global.console.Console ``` through a custom Symbol.hasInstance function of Console that tests for a special symbol kIsConsole for backwards compatibility. This fixes a case in the console Web Platform Test that we commented out. PR-URL: https://github.com/nodejs/node/pull/23509 Refs: https://github.com/whatwg/console/issues/3 Refs: https://console.spec.whatwg.org/#console-namespace Reviewed-By: Gus Caplan Reviewed-By: Matteo Collina Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: Denys Otrishko Reviewed-By: James M Snell Reviewed-By: Rich Trott Reviewed-By: Colin Ihrig Reviewed-By: Sakthipriyan Vairamani Reviewed-By: John-David Dalton --- lib/console.js | 56 +++++++++++-- test/parallel/test-console-instance.js | 79 +++++++++++-------- .../test-whatwg-console-is-a-namespace.js | 4 +- 3 files changed, 99 insertions(+), 40 deletions(-) diff --git a/lib/console.js b/lib/console.js index 96bd1855859809..2e56f7bbba5ad8 100644 --- a/lib/console.js +++ b/lib/console.js @@ -60,17 +60,21 @@ let cliTable; // Track amount of indentation required via `console.group()`. const kGroupIndent = Symbol('kGroupIndent'); - const kFormatForStderr = Symbol('kFormatForStderr'); const kFormatForStdout = Symbol('kFormatForStdout'); const kGetInspectOptions = Symbol('kGetInspectOptions'); const kColorMode = Symbol('kColorMode'); +const kIsConsole = Symbol('kIsConsole'); function Console(options /* or: stdout, stderr, ignoreErrors = true */) { - if (!(this instanceof Console)) { + // We have to test new.target here to see if this function is called + // with new, because we need to define a custom instanceof to accommodate + // the global console. + if (!new.target) { return new Console(...arguments); } + this[kIsConsole] = true; if (!options || typeof options.write === 'function') { options = { stdout: options, @@ -125,7 +129,7 @@ function Console(options /* or: stdout, stderr, ignoreErrors = true */) { var keys = Object.keys(Console.prototype); for (var v = 0; v < keys.length; v++) { var k = keys[v]; - this[k] = this[k].bind(this); + this[k] = Console.prototype[k].bind(this); } } @@ -465,10 +469,50 @@ Console.prototype.table = function(tabularData, properties) { return final(keys, values); }; -module.exports = new Console({ +function noop() {} + +// See https://console.spec.whatwg.org/#console-namespace +// > For historical web-compatibility reasons, the namespace object +// > for console must have as its [[Prototype]] an empty object, +// > created as if by ObjectCreate(%ObjectPrototype%), +// > instead of %ObjectPrototype%. + +// Since in Node.js, the Console constructor has been exposed through +// require('console'), we need to keep the Console constructor but +// we cannot actually use `new Console` to construct the global console. +// Therefore, the console.Console.prototype is not +// in the global console prototype chain anymore. +const globalConsole = Object.create({}); +const tempConsole = new Console({ stdout: process.stdout, stderr: process.stderr }); -module.exports.Console = Console; -function noop() {} +// Since Console is not on the prototype chain of the global console, +// the symbol properties on Console.prototype have to be looked up from +// the global console itself. +for (const prop of Object.getOwnPropertySymbols(Console.prototype)) { + globalConsole[prop] = Console.prototype[prop]; +} + +// Reflect.ownKeys() is used here for retrieving Symbols +for (const prop of Reflect.ownKeys(tempConsole)) { + const desc = { ...(Reflect.getOwnPropertyDescriptor(tempConsole, prop)) }; + // Since Console would bind method calls onto the instance, + // make sure the methods are called on globalConsole instead of + // tempConsole. + if (typeof Console.prototype[prop] === 'function') { + desc.value = Console.prototype[prop].bind(globalConsole); + } + Reflect.defineProperty(globalConsole, prop, desc); +} + +globalConsole.Console = Console; + +Object.defineProperty(Console, Symbol.hasInstance, { + value(instance) { + return instance[kIsConsole]; + } +}); + +module.exports = globalConsole; diff --git a/test/parallel/test-console-instance.js b/test/parallel/test-console-instance.js index 0b69212948dd2c..87708808a8652c 100644 --- a/test/parallel/test-console-instance.js +++ b/test/parallel/test-console-instance.js @@ -23,7 +23,8 @@ const common = require('../common'); const assert = require('assert'); const Stream = require('stream'); -const Console = require('console').Console; +const requiredConsole = require('console'); +const Console = requiredConsole.Console; const out = new Stream(); const err = new Stream(); @@ -35,6 +36,11 @@ process.stdout.write = process.stderr.write = common.mustNotCall(); // Make sure that the "Console" function exists. assert.strictEqual('function', typeof Console); +assert.strictEqual(requiredConsole, global.console); +// Make sure the custom instanceof of Console works +assert.ok(global.console instanceof Console); +assert.ok(!({} instanceof Console)); + // Make sure that the Console constructor throws // when not given a writable stream instance. common.expectsError( @@ -62,46 +68,57 @@ common.expectsError( out.write = err.write = (d) => {}; -const c = new Console(out, err); +{ + const c = new Console(out, err); + assert.ok(c instanceof Console); -out.write = err.write = common.mustCall((d) => { - assert.strictEqual(d, 'test\n'); -}, 2); + out.write = err.write = common.mustCall((d) => { + assert.strictEqual(d, 'test\n'); + }, 2); -c.log('test'); -c.error('test'); + c.log('test'); + c.error('test'); -out.write = common.mustCall((d) => { - assert.strictEqual(d, '{ foo: 1 }\n'); -}); + out.write = common.mustCall((d) => { + assert.strictEqual(d, '{ foo: 1 }\n'); + }); -c.dir({ foo: 1 }); + c.dir({ foo: 1 }); -// Ensure that the console functions are bound to the console instance. -let called = 0; -out.write = common.mustCall((d) => { - called++; - assert.strictEqual(d, `${called} ${called - 1} [ 1, 2, 3 ]\n`); -}, 3); + // Ensure that the console functions are bound to the console instance. + let called = 0; + out.write = common.mustCall((d) => { + called++; + assert.strictEqual(d, `${called} ${called - 1} [ 1, 2, 3 ]\n`); + }, 3); -[1, 2, 3].forEach(c.log); + [1, 2, 3].forEach(c.log); +} -// Console() detects if it is called without `new` keyword. -Console(out, err); +// Test calling Console without the `new` keyword. +{ + const withoutNew = Console(out, err); + assert.ok(withoutNew instanceof Console); +} -// Extending Console works. -class MyConsole extends Console { - hello() {} +// Test extending Console +{ + class MyConsole extends Console { + hello() {} + } + const myConsole = new MyConsole(process.stdout); + assert.strictEqual(typeof myConsole.hello, 'function'); + assert.ok(myConsole instanceof Console); } -const myConsole = new MyConsole(process.stdout); -assert.strictEqual(typeof myConsole.hello, 'function'); // Instance that does not ignore the stream errors. -const c2 = new Console(out, err, false); +{ + const c2 = new Console(out, err, false); -out.write = () => { throw new Error('out'); }; -err.write = () => { throw new Error('err'); }; + out.write = () => { throw new Error('out'); }; + err.write = () => { throw new Error('err'); }; -assert.throws(() => c2.log('foo'), /^Error: out$/); -assert.throws(() => c2.warn('foo'), /^Error: err$/); -assert.throws(() => c2.dir('foo'), /^Error: out$/); + assert.throws(() => c2.log('foo'), /^Error: out$/); + assert.throws(() => c2.warn('foo'), /^Error: err$/); + assert.throws(() => c2.dir('foo'), /^Error: out$/); +} diff --git a/test/parallel/test-whatwg-console-is-a-namespace.js b/test/parallel/test-whatwg-console-is-a-namespace.js index fb28e751a322af..8d6569ad6d6521 100644 --- a/test/parallel/test-whatwg-console-is-a-namespace.js +++ b/test/parallel/test-whatwg-console-is-a-namespace.js @@ -38,9 +38,7 @@ test(() => { const prototype1 = Object.getPrototypeOf(console); const prototype2 = Object.getPrototypeOf(prototype1); - // This got commented out from the original test because in Node.js all - // functions are declared on the prototype. - // assert_equals(Object.getOwnPropertyNames(prototype1).length, 0, "The [[Prototype]] must have no properties"); + assert_equals(Object.getOwnPropertyNames(prototype1).length, 0, "The [[Prototype]] must have no properties"); assert_equals(prototype2, Object.prototype, "The [[Prototype]]'s [[Prototype]] must be %ObjectPrototype%"); }, "The prototype chain must be correct");