From 1fa635285379a77a741f92bcd06bee1dc6efd03e Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Mon, 27 Feb 2023 09:31:18 -0500 Subject: [PATCH] url: offload `URLSearchParams` initialization PR-URL: https://github.com/nodejs/node/pull/46867 Reviewed-By: Matteo Collina Reviewed-By: Antoine du Hamel Reviewed-By: James M Snell Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: Benjamin Gruenbaum Reviewed-By: Filip Skokan --- lib/_http_client.js | 5 +- lib/https.js | 6 +- lib/internal/url.js | 82 ++++++++++----------- test/parallel/test-whatwg-url-properties.js | 7 ++ 4 files changed, 48 insertions(+), 52 deletions(-) diff --git a/lib/_http_client.js b/lib/_http_client.js index 024834c7ecfd22..9fd07e1676b8ff 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -63,7 +63,7 @@ const { const Agent = require('_http_agent'); const { Buffer } = require('buffer'); const { defaultTriggerAsyncIdScope } = require('internal/async_hooks'); -const { URL, urlToHttpOptions, searchParamsSymbol } = require('internal/url'); +const { URL, urlToHttpOptions, isURL } = require('internal/url'); const { kOutHeaders, kNeedDrain, @@ -133,8 +133,7 @@ function ClientRequest(input, options, cb) { if (typeof input === 'string') { const urlStr = input; input = urlToHttpOptions(new URL(urlStr)); - } else if (input && input[searchParamsSymbol] && - input[searchParamsSymbol][searchParamsSymbol]) { + } else if (isURL(input)) { // url.URL instance input = urlToHttpOptions(input); } else { diff --git a/lib/https.js b/lib/https.js index 095ac19efcf2f4..34e3465168487d 100644 --- a/lib/https.js +++ b/lib/https.js @@ -52,7 +52,7 @@ const { ClientRequest } = require('_http_client'); let debug = require('internal/util/debuglog').debuglog('https', (fn) => { debug = fn; }); -const { URL, urlToHttpOptions, searchParamsSymbol } = require('internal/url'); +const { URL, urlToHttpOptions, isURL } = require('internal/url'); function Server(opts, requestListener) { if (!(this instanceof Server)) return new Server(opts, requestListener); @@ -344,9 +344,7 @@ function request(...args) { if (typeof args[0] === 'string') { const urlStr = ArrayPrototypeShift(args); options = urlToHttpOptions(new URL(urlStr)); - } else if (args[0] && args[0][searchParamsSymbol] && - args[0][searchParamsSymbol][searchParamsSymbol]) { - // url.URL instance + } else if (isURL(args[0])) { options = urlToHttpOptions(ArrayPrototypeShift(args)); } diff --git a/lib/internal/url.js b/lib/internal/url.js index e3ff475dfabfa1..193965b76d346d 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -224,7 +224,7 @@ class URLSearchParams { } else { // USVString init = toUSVString(init); - initSearchParams(this, init); + this[searchParams] = init ? parseParams(init) : []; } // "associated url object" @@ -534,7 +534,7 @@ ObjectDefineProperties(URLSearchParams.prototype, { }, }); -function isURLThis(self) { +function isURL(self) { return self != null && ObjectPrototypeHasOwnProperty(self, context); } @@ -602,27 +602,25 @@ class URL { ctx.password = password; ctx.port = port; ctx.hash = hash; - if (!this[searchParams]) { // Invoked from URL constructor - this[searchParams] = new URLSearchParams(); - this[searchParams][context] = this; + if (this[searchParams]) { + this[searchParams][searchParams] = parseParams(search); } - initSearchParams(this[searchParams], ctx.search); }; toString() { - if (!isURLThis(this)) + if (!isURL(this)) throw new ERR_INVALID_THIS('URL'); return this[context].href; } get href() { - if (!isURLThis(this)) + if (!isURL(this)) throw new ERR_INVALID_THIS('URL'); return this[context].href; } set href(value) { - if (!isURLThis(this)) + if (!isURL(this)) throw new ERR_INVALID_THIS('URL'); const valid = updateUrl(this[context].href, updateActions.kHref, `${value}`, this.#onParseComplete); if (!valid) { throw ERR_INVALID_URL(`${value}`); } @@ -630,49 +628,49 @@ class URL { // readonly get origin() { - if (!isURLThis(this)) + if (!isURL(this)) throw new ERR_INVALID_THIS('URL'); return this[context].origin; } get protocol() { - if (!isURLThis(this)) + if (!isURL(this)) throw new ERR_INVALID_THIS('URL'); return this[context].protocol; } set protocol(value) { - if (!isURLThis(this)) + if (!isURL(this)) throw new ERR_INVALID_THIS('URL'); updateUrl(this[context].href, updateActions.kProtocol, `${value}`, this.#onParseComplete); } get username() { - if (!isURLThis(this)) + if (!isURL(this)) throw new ERR_INVALID_THIS('URL'); return this[context].username; } set username(value) { - if (!isURLThis(this)) + if (!isURL(this)) throw new ERR_INVALID_THIS('URL'); updateUrl(this[context].href, updateActions.kUsername, `${value}`, this.#onParseComplete); } get password() { - if (!isURLThis(this)) + if (!isURL(this)) throw new ERR_INVALID_THIS('URL'); return this[context].password; } set password(value) { - if (!isURLThis(this)) + if (!isURL(this)) throw new ERR_INVALID_THIS('URL'); updateUrl(this[context].href, updateActions.kPassword, `${value}`, this.#onParseComplete); } get host() { - if (!isURLThis(this)) + if (!isURL(this)) throw new ERR_INVALID_THIS('URL'); const port = this[context].port; const suffix = port.length > 0 ? `:${port}` : ''; @@ -680,82 +678,85 @@ class URL { } set host(value) { - if (!isURLThis(this)) + if (!isURL(this)) throw new ERR_INVALID_THIS('URL'); updateUrl(this[context].href, updateActions.kHost, `${value}`, this.#onParseComplete); } get hostname() { - if (!isURLThis(this)) + if (!isURL(this)) throw new ERR_INVALID_THIS('URL'); return this[context].hostname; } set hostname(value) { - if (!isURLThis(this)) + if (!isURL(this)) throw new ERR_INVALID_THIS('URL'); updateUrl(this[context].href, updateActions.kHostname, `${value}`, this.#onParseComplete); } get port() { - if (!isURLThis(this)) + if (!isURL(this)) throw new ERR_INVALID_THIS('URL'); return this[context].port; } set port(value) { - if (!isURLThis(this)) + if (!isURL(this)) throw new ERR_INVALID_THIS('URL'); updateUrl(this[context].href, updateActions.kPort, `${value}`, this.#onParseComplete); } get pathname() { - if (!isURLThis(this)) + if (!isURL(this)) throw new ERR_INVALID_THIS('URL'); return this[context].pathname; } set pathname(value) { - if (!isURLThis(this)) + if (!isURL(this)) throw new ERR_INVALID_THIS('URL'); updateUrl(this[context].href, updateActions.kPathname, `${value}`, this.#onParseComplete); } get search() { - if (!isURLThis(this)) + if (!isURL(this)) throw new ERR_INVALID_THIS('URL'); return this[context].search; } - set search(search) { - if (!isURLThis(this)) + set search(value) { + if (!isURL(this)) throw new ERR_INVALID_THIS('URL'); - search = toUSVString(search); - updateUrl(this[context].href, updateActions.kSearch, search, this.#onParseComplete); - initSearchParams(this[searchParams], this[context].search); + updateUrl(this[context].href, updateActions.kSearch, toUSVString(value), this.#onParseComplete); } // readonly get searchParams() { - if (!isURLThis(this)) + if (!isURL(this)) throw new ERR_INVALID_THIS('URL'); + // Create URLSearchParams on demand to greatly improve the URL performance. + if (this[searchParams] == null) { + this[searchParams] = new URLSearchParams(this[context].search); + this[searchParams][context] = this; + } return this[searchParams]; } get hash() { - if (!isURLThis(this)) + if (!isURL(this)) throw new ERR_INVALID_THIS('URL'); return this[context].hash; } set hash(value) { - if (!isURLThis(this)) + if (!isURL(this)) throw new ERR_INVALID_THIS('URL'); updateUrl(this[context].href, updateActions.kHash, `${value}`, this.#onParseComplete); } toJSON() { - if (!isURLThis(this)) + if (!isURL(this)) throw new ERR_INVALID_THIS('URL'); return this[context].href; } @@ -813,14 +814,6 @@ ObjectDefineProperties(URL, { revokeObjectURL: kEnumerableProperty, }); -function initSearchParams(url, init) { - if (!init) { - url[searchParams] = []; - return; - } - url[searchParams] = parseParams(init); -} - // application/x-www-form-urlencoded parser // Ref: https://url.spec.whatwg.org/#concept-urlencoded-parser function parseParams(qs) { @@ -1139,8 +1132,7 @@ function domainToUnicode(domain) { function urlToHttpOptions(url) { const options = { protocol: url.protocol, - hostname: typeof url.hostname === 'string' && - StringPrototypeStartsWith(url.hostname, '[') ? + hostname: url.hostname && StringPrototypeStartsWith(url.hostname, '[') ? StringPrototypeSlice(url.hostname, 1, -1) : url.hostname, hash: url.hash, @@ -1311,6 +1303,6 @@ module.exports = { domainToASCII, domainToUnicode, urlToHttpOptions, - searchParamsSymbol: searchParams, encodeStr, + isURL, }; diff --git a/test/parallel/test-whatwg-url-properties.js b/test/parallel/test-whatwg-url-properties.js index 69ce14a431a9b7..b76832a7d30912 100644 --- a/test/parallel/test-whatwg-url-properties.js +++ b/test/parallel/test-whatwg-url-properties.js @@ -73,6 +73,13 @@ const { URL, URLSearchParams, format } = require('url'); assert.strictEqual(params.size, 3); } +{ + const u = new URL('https://abc.com/?q=old'); + const s = u.searchParams; + u.href = 'http://abc.com/?q=new'; + assert.strictEqual(s.get('q'), 'new'); +} + function stringifyName(name) { if (typeof name === 'symbol') { const { description } = name;