From 96e6873dd0f45c2f56cfcb40de5df24edfedd60b Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Tue, 13 Nov 2018 14:34:07 -0800 Subject: [PATCH] Revert "url: make the context non-enumerable" This reverts commit 5e1bf058f4906c0a49c34c6a1353d0b34784c80a, as it causes major performance regressions during object construction. Refs: https://github.com/nodejs/node/pull/24218 PR-URL: https://github.com/nodejs/node/pull/24495 Reviewed-By: Anna Henningsen Reviewed-By: Joyee Cheung Reviewed-By: Gus Caplan Reviewed-By: Franziska Hinkelmann Reviewed-By: Daijiro Wachi Reviewed-By: Ruben Bridgewater Reviewed-By: Colin Ihrig Reviewed-By: James M Snell --- lib/internal/url.js | 15 ++------------- ...est-whatwg-url-custom-no-enumerable-context.js | 14 -------------- 2 files changed, 2 insertions(+), 27 deletions(-) delete mode 100644 test/parallel/test-whatwg-url-custom-no-enumerable-context.js diff --git a/lib/internal/url.js b/lib/internal/url.js index 44b658b2d92111..310e9ff9e2d827 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -246,14 +246,7 @@ function onParseError(flags, input) { // Reused by URL constructor and URL#href setter. function parse(url, input, base) { const base_context = base ? base[context] : undefined; - // In the URL#href setter - if (!url[context]) { - Object.defineProperty(url, context, { - enumerable: false, - configurable: false, - value: new URLContext() - }); - } + url[context] = new URLContext(); _parse(input.trim(), -1, base_context, undefined, onParseComplete.bind(url), onParseError); } @@ -1381,11 +1374,7 @@ function toPathIfFileURL(fileURLOrPath) { } function NativeURL(ctx) { - Object.defineProperty(this, context, { - enumerable: false, - configurable: false, - value: ctx - }); + this[context] = ctx; } NativeURL.prototype = URL.prototype; diff --git a/test/parallel/test-whatwg-url-custom-no-enumerable-context.js b/test/parallel/test-whatwg-url-custom-no-enumerable-context.js deleted file mode 100644 index f24a47b6d5c8f0..00000000000000 --- a/test/parallel/test-whatwg-url-custom-no-enumerable-context.js +++ /dev/null @@ -1,14 +0,0 @@ -'use strict'; -// This tests that the context of URL objects are not -// enumerable and thus considered by assert libraries. -// See https://github.com/nodejs/node/issues/24211 - -// Tests below are not from WPT. - -require('../common'); -const assert = require('assert'); - -assert.deepStrictEqual( - new URL('./foo', 'https://example.com/'), - new URL('https://example.com/foo') -);