From 0ee8fdb1b3c05fc2b5cd302def2e8a661efe0e21 Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Thu, 19 Jan 2017 17:19:41 -0800 Subject: [PATCH 1/3] url: do not public expose inspect methods --- lib/internal/url.js | 4 ++-- test/parallel/test-whatwg-url-parsing.js | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/internal/url.js b/lib/internal/url.js index 679e14d72a9c39..753ec1a5eaa8e1 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -77,7 +77,7 @@ class TupleOrigin { return result; } - inspect() { + [util.inspect.custom]() { return `TupleOrigin { scheme: ${this[kScheme]}, host: ${this[kHost]}, @@ -235,7 +235,7 @@ class URL { return (this[context].flags & binding.URL_FLAGS_CANNOT_BE_BASE) !== 0; } - inspect(depth, opts) { + [util.inspect.custom](depth, opts) { const ctx = this[context]; var ret = 'URL {\n'; ret += ` href: ${this.href}\n`; diff --git a/test/parallel/test-whatwg-url-parsing.js b/test/parallel/test-whatwg-url-parsing.js index 9ea6cc74bff5e6..f1fd2ae9051533 100644 --- a/test/parallel/test-whatwg-url-parsing.js +++ b/test/parallel/test-whatwg-url-parsing.js @@ -1,6 +1,7 @@ 'use strict'; const common = require('../common'); +const util = require('util'); if (!common.hasIntl) { // A handful of the tests fail when ICU is not included. @@ -144,8 +145,8 @@ for (const test of allTests) { const url = test.url ? new URL(test.url) : new URL(test.input, test.base); for (const showHidden of [true, false]) { - const res = url.inspect(null, { - showHidden: showHidden + const res = util.inspect(url, { + showHidden }); const lines = res.split('\n'); From 2616fd955751d20c9788f5ae0bd9d0a7962a8a16 Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Thu, 19 Jan 2017 17:15:45 -0800 Subject: [PATCH 2/3] url: do not define @@toStringTag with a getter --- lib/internal/url.js | 11 ++++++++--- test/parallel/test-whatwg-url-properties.js | 4 ++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/internal/url.js b/lib/internal/url.js index 753ec1a5eaa8e1..cbd641f721b6ca 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -221,10 +221,11 @@ class URL { if (base !== undefined && !(base instanceof URL)) base = new URL(String(base)); parse(this, input, base); - } - get [Symbol.toStringTag]() { - return this instanceof URL ? 'URL' : 'URLPrototype'; + Object.defineProperty(this, Symbol.toStringTag, { + configurable: true, + value: 'URL' + }); } get [special]() { @@ -314,6 +315,10 @@ Object.defineProperties(URL.prototype, { return ret; } }, + [Symbol.toStringTag]: { + configurable: true, + value: 'URLPrototype' + }, href: { enumerable: true, configurable: true, diff --git a/test/parallel/test-whatwg-url-properties.js b/test/parallel/test-whatwg-url-properties.js index 0b5e633677170a..ffeeace0a3b2fc 100644 --- a/test/parallel/test-whatwg-url-properties.js +++ b/test/parallel/test-whatwg-url-properties.js @@ -45,7 +45,7 @@ assert.strictEqual(url.searchParams, oldParams); // [SameObject] // Note: this error message is subject to change in V8 updates assert.throws(() => url.origin = 'http://foo.bar.com:22', new RegExp('TypeError: Cannot set property origin of' + - ' \\[object Object\\] which has only a getter')); + ' \\[object URLPrototype\\] which has only a getter')); assert.strictEqual(url.origin, 'http://foo.bar.com:21'); assert.strictEqual(url.toString(), 'http://user:pass@foo.bar.com:21/aaa/zzz?l=25#test'); @@ -121,7 +121,7 @@ assert.strictEqual(url.hash, '#abcd'); // Note: this error message is subject to change in V8 updates assert.throws(() => url.searchParams = '?k=88', new RegExp('TypeError: Cannot set property searchParams of' + - ' \\[object Object\\] which has only a getter')); + ' \\[object URLPrototype\\] which has only a getter')); assert.strictEqual(url.searchParams, oldParams); assert.strictEqual(url.toString(), 'https://user2:pass2@foo.bar.org:23/aaa/bbb?k=99#abcd'); From b9eb7379a887ec5de700768fce689e5026806222 Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Mon, 23 Jan 2017 12:35:15 -0800 Subject: [PATCH 3/3] Do not assign different @@toStringTag to URL.prototype For performance and consistency with Chrome. --- lib/internal/url.js | 7 +------ test/parallel/test-whatwg-url-properties.js | 4 ++-- test/parallel/test-whatwg-url-tostringtag.js | 7 +++++-- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/lib/internal/url.js b/lib/internal/url.js index cbd641f721b6ca..0b98352cd04653 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -221,11 +221,6 @@ class URL { if (base !== undefined && !(base instanceof URL)) base = new URL(String(base)); parse(this, input, base); - - Object.defineProperty(this, Symbol.toStringTag, { - configurable: true, - value: 'URL' - }); } get [special]() { @@ -317,7 +312,7 @@ Object.defineProperties(URL.prototype, { }, [Symbol.toStringTag]: { configurable: true, - value: 'URLPrototype' + value: 'URL' }, href: { enumerable: true, diff --git a/test/parallel/test-whatwg-url-properties.js b/test/parallel/test-whatwg-url-properties.js index ffeeace0a3b2fc..b762659fb6eadc 100644 --- a/test/parallel/test-whatwg-url-properties.js +++ b/test/parallel/test-whatwg-url-properties.js @@ -45,7 +45,7 @@ assert.strictEqual(url.searchParams, oldParams); // [SameObject] // Note: this error message is subject to change in V8 updates assert.throws(() => url.origin = 'http://foo.bar.com:22', new RegExp('TypeError: Cannot set property origin of' + - ' \\[object URLPrototype\\] which has only a getter')); + ' \\[object URL\\] which has only a getter')); assert.strictEqual(url.origin, 'http://foo.bar.com:21'); assert.strictEqual(url.toString(), 'http://user:pass@foo.bar.com:21/aaa/zzz?l=25#test'); @@ -121,7 +121,7 @@ assert.strictEqual(url.hash, '#abcd'); // Note: this error message is subject to change in V8 updates assert.throws(() => url.searchParams = '?k=88', new RegExp('TypeError: Cannot set property searchParams of' + - ' \\[object URLPrototype\\] which has only a getter')); + ' \\[object URL\\] which has only a getter')); assert.strictEqual(url.searchParams, oldParams); assert.strictEqual(url.toString(), 'https://user2:pass2@foo.bar.org:23/aaa/bbb?k=99#abcd'); diff --git a/test/parallel/test-whatwg-url-tostringtag.js b/test/parallel/test-whatwg-url-tostringtag.js index 9e0927955feaab..55b0a48ce741ac 100644 --- a/test/parallel/test-whatwg-url-tostringtag.js +++ b/test/parallel/test-whatwg-url-tostringtag.js @@ -10,9 +10,12 @@ const sp = url.searchParams; const test = [ [toString.call(url), 'URL'], - [toString.call(Object.getPrototypeOf(url)), 'URLPrototype'], [toString.call(sp), 'URLSearchParams'], - [toString.call(Object.getPrototypeOf(sp)), 'URLSearchParamsPrototype'] + [toString.call(Object.getPrototypeOf(sp)), 'URLSearchParamsPrototype'], + // Web IDL spec says we have to return 'URLPrototype', but it is too + // expensive to implement; therefore, use Chrome's behavior for now, until + // spec is changed. + [toString.call(Object.getPrototypeOf(url)), 'URL'] ]; test.forEach((row) => {