From 2b2d11b5c48c5a085b75e768864bcb8596121d68 Mon Sep 17 00:00:00 2001
From: MattIPv4 <me@mattcowley.co.uk>
Date: Fri, 19 Jan 2024 02:33:52 +0000
Subject: [PATCH 01/18] url: remove #context from URLSearchParams

---
 lib/internal/url.js | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/lib/internal/url.js b/lib/internal/url.js
index 38f97926064595..9a2f73492b8db7 100644
--- a/lib/internal/url.js
+++ b/lib/internal/url.js
@@ -206,7 +206,6 @@ class URLContext {
   }
 }
 
-let setURLSearchParamsContext;
 let getURLSearchParamsList;
 let setURLSearchParams;
 
@@ -308,13 +307,7 @@ ObjectDefineProperties(URLSearchParamsIterator.prototype, {
 class URLSearchParams {
   #searchParams = [];
 
-  // "associated url object"
-  #context;
-
   static {
-    setURLSearchParamsContext = (obj, ctx) => {
-      obj.#context = ctx;
-    };
     getURLSearchParamsList = (obj) => obj.#searchParams;
     setURLSearchParams = (obj, query) => {
       if (query === undefined) {
@@ -475,9 +468,6 @@ class URLSearchParams {
     name = StringPrototypeToWellFormed(`${name}`);
     value = StringPrototypeToWellFormed(`${value}`);
     ArrayPrototypePush(this.#searchParams, name, value);
-    if (this.#context) {
-      this.#context.search = this.toString();
-    }
   }
 
   delete(name, value = undefined) {
@@ -509,9 +499,6 @@ class URLSearchParams {
         }
       }
     }
-    if (this.#context) {
-      this.#context.search = this.toString();
-    }
   }
 
   get(name) {
@@ -613,10 +600,6 @@ class URLSearchParams {
     if (!found) {
       ArrayPrototypePush(list, name, value);
     }
-
-    if (this.#context) {
-      this.#context.search = this.toString();
-    }
   }
 
   sort() {
@@ -662,10 +645,6 @@ class URLSearchParams {
         }
       }
     }
-
-    if (this.#context) {
-      this.#context.search = this.toString();
-    }
   }
 
   // https://heycam.github.io/webidl/#es-iterators
@@ -1021,7 +1000,6 @@ class URL {
     // Create URLSearchParams on demand to greatly improve the URL performance.
     if (this.#searchParams == null) {
       this.#searchParams = new URLSearchParams(this.search);
-      setURLSearchParamsContext(this.#searchParams, this);
     }
     return this.#searchParams;
   }

From 129a1a9bcef888f038063a87c87016001bcbf052 Mon Sep 17 00:00:00 2001
From: MattIPv4 <me@mattcowley.co.uk>
Date: Fri, 19 Jan 2024 02:57:25 +0000
Subject: [PATCH 02/18] url: rely on URLSearchParams if in use for search

---
 lib/internal/url.js | 43 +++++++++++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 14 deletions(-)

diff --git a/lib/internal/url.js b/lib/internal/url.js
index 9a2f73492b8db7..10bb0e921661c1 100644
--- a/lib/internal/url.js
+++ b/lib/internal/url.js
@@ -793,6 +793,14 @@ class URL {
     return `${constructor.name} ${inspect(obj, opts)}`;
   }
 
+  #getContextSearch() {
+    if (!this.#context.hasSearch) { return ''; }
+    let endsAt = this.#context.href.length;
+    if (this.#context.hasHash) { endsAt = this.#context.hash_start; }
+    if (endsAt - this.#context.search_start <= 1) { return ''; }
+    return StringPrototypeSlice(this.#context.href, this.#context.search_start, endsAt);
+  }
+
   #updateContext(href) {
     this.#context.href = href;
 
@@ -818,20 +826,20 @@ class URL {
     this.#context.hash_start = hash_start;
     this.#context.scheme_type = scheme_type;
 
+    // If URLSearchParams is in use, store the search there, not here.
     if (this.#searchParams) {
       if (this.#context.hasSearch) {
-        setURLSearchParams(this.#searchParams, this.search);
-      } else {
-        setURLSearchParams(this.#searchParams, undefined);
+        setURLSearchParams(this.#searchParams, this.#getContextSearch());
+        this.#updateContext(bindingUrl.update(href, updateActions.kSearch, ''));
       }
     }
   }
 
-  toString() {
-    return this.#context.href;
-  }
-
   get href() {
+    // URLSearchParams is used on demand, and stores the search if it's used.
+    if (this.#searchParams) {
+      return bindingUrl.update(this.#context.href, updateActions.kSearch, this.#searchParams.toString());
+    }
     return this.#context.href;
   }
 
@@ -981,11 +989,9 @@ class URL {
   }
 
   get search() {
-    if (!this.#context.hasSearch) { return ''; }
-    let endsAt = this.#context.href.length;
-    if (this.#context.hasHash) { endsAt = this.#context.hash_start; }
-    if (endsAt - this.#context.search_start <= 1) { return ''; }
-    return StringPrototypeSlice(this.#context.href, this.#context.search_start, endsAt);
+    // URLSearchParams is used on demand, and stores the search if it's used.
+    if (this.#searchParams) return this.#searchParams.toString();
+    return this.#getContextSearch();
   }
 
   set search(value) {
@@ -999,7 +1005,12 @@ class URL {
   get searchParams() {
     // Create URLSearchParams on demand to greatly improve the URL performance.
     if (this.#searchParams == null) {
-      this.#searchParams = new URLSearchParams(this.search);
+      this.#searchParams = new URLSearchParams(this.#getContextSearch());
+
+      // Remove search from the URL so we're not storing it twice.
+      if (this.#context.hasSearch) {
+        this.#updateContext(bindingUrl.update(this.#context.href, updateActions.kSearch, ''));
+      }
     }
     return this.#searchParams;
   }
@@ -1018,8 +1029,12 @@ class URL {
     }
   }
 
+  toString() {
+    return this.href;
+  }
+
   toJSON() {
-    return this.#context.href;
+    return this.href;
   }
 
   static canParse(url, base = undefined) {

From c2e8526f5baa1c454f24fba7653fd2fcef7d24b0 Mon Sep 17 00:00:00 2001
From: MattIPv4 <me@mattcowley.co.uk>
Date: Fri, 19 Jan 2024 03:08:26 +0000
Subject: [PATCH 03/18] url: update URLSearchParams directly from URL search
 setter

---
 lib/internal/url.js | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/internal/url.js b/lib/internal/url.js
index 10bb0e921661c1..61bb8ee991ada5 100644
--- a/lib/internal/url.js
+++ b/lib/internal/url.js
@@ -995,6 +995,12 @@ class URL {
   }
 
   set search(value) {
+    // URLSearchParams is used on demand, and stores the search if it's used.
+    if (this.#searchParams) {
+      setURLSearchParams(this.#searchParams, StringPrototypeToWellFormed(`${value}`));
+      return;
+    }
+
     const href = bindingUrl.update(this.#context.href, updateActions.kSearch, StringPrototypeToWellFormed(`${value}`));
     if (href) {
       this.#updateContext(href);

From 039380ef27d869ae7a4f0cc5b461e5dd251f877f Mon Sep 17 00:00:00 2001
From: MattIPv4 <me@mattcowley.co.uk>
Date: Fri, 19 Jan 2024 03:37:58 +0000
Subject: [PATCH 04/18] url: create href using searchParams without bindingUrl
 call

---
 lib/internal/url.js | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/internal/url.js b/lib/internal/url.js
index 61bb8ee991ada5..3ab160f1767b05 100644
--- a/lib/internal/url.js
+++ b/lib/internal/url.js
@@ -838,7 +838,9 @@ class URL {
   get href() {
     // URLSearchParams is used on demand, and stores the search if it's used.
     if (this.#searchParams) {
-      return bindingUrl.update(this.#context.href, updateActions.kSearch, this.#searchParams.toString());
+      return StringPrototypeSlice(this.#context.href, 0, this.#context.hash_start) +
+        this.#searchParams.toString() +
+        StringPrototypeSlice(this.#context.href, this.#context.hash_start);
     }
     return this.#context.href;
   }

From e015de2b393b742a6bc22df7bcb99de880eb88ce Mon Sep 17 00:00:00 2001
From: MattIPv4 <me@mattcowley.co.uk>
Date: Fri, 19 Jan 2024 03:48:53 +0000
Subject: [PATCH 05/18] url: ensure ? is present when relying on
 URLSearchParams

---
 lib/internal/url.js | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/lib/internal/url.js b/lib/internal/url.js
index 3ab160f1767b05..8d46e1e312619e 100644
--- a/lib/internal/url.js
+++ b/lib/internal/url.js
@@ -794,13 +794,19 @@ class URL {
   }
 
   #getContextSearch() {
-    if (!this.#context.hasSearch) { return ''; }
+    if (!this.#context.hasSearch) return '';
     let endsAt = this.#context.href.length;
-    if (this.#context.hasHash) { endsAt = this.#context.hash_start; }
-    if (endsAt - this.#context.search_start <= 1) { return ''; }
+    if (this.#context.hasHash) endsAt = this.#context.hash_start;
+    if (endsAt - this.#context.search_start <= 1) return '';
     return StringPrototypeSlice(this.#context.href, this.#context.search_start, endsAt);
   }
 
+  #getParamsSearch() {
+    if (!this.#searchParams) return '';
+    if (!this.#searchParams.size) return '';
+    return `?${this.#searchParams.toString()}`;
+  }
+
   #updateContext(href) {
     this.#context.href = href;
 
@@ -839,7 +845,7 @@ class URL {
     // URLSearchParams is used on demand, and stores the search if it's used.
     if (this.#searchParams) {
       return StringPrototypeSlice(this.#context.href, 0, this.#context.hash_start) +
-        this.#searchParams.toString() +
+        this.#getParamsSearch() +
         StringPrototypeSlice(this.#context.href, this.#context.hash_start);
     }
     return this.#context.href;
@@ -992,7 +998,7 @@ class URL {
 
   get search() {
     // URLSearchParams is used on demand, and stores the search if it's used.
-    if (this.#searchParams) return this.#searchParams.toString();
+    if (this.#searchParams) return this.#getParamsSearch();
     return this.#getContextSearch();
   }
 

From 2487726934472b00aea84bbcd707380cdccc0db0 Mon Sep 17 00:00:00 2001
From: MattIPv4 <me@mattcowley.co.uk>
Date: Fri, 19 Jan 2024 04:24:20 +0000
Subject: [PATCH 06/18] url: don't use public getter for toString/toJSON

This appears to keep the "calling stringifier with this = {} didn't throw TypeError" test happy
---
 lib/internal/url.js | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/lib/internal/url.js b/lib/internal/url.js
index 8d46e1e312619e..e69b591a46cf96 100644
--- a/lib/internal/url.js
+++ b/lib/internal/url.js
@@ -807,6 +807,16 @@ class URL {
     return `?${this.#searchParams.toString()}`;
   }
 
+  #getHref() {
+    // URLSearchParams is used on demand, and stores the search if it's used.
+    if (this.#searchParams) {
+      return StringPrototypeSlice(this.#context.href, 0, this.#context.hash_start) +
+        this.#getParamsSearch() +
+        StringPrototypeSlice(this.#context.href, this.#context.hash_start);
+    }
+    return this.#context.href;
+  }
+
   #updateContext(href) {
     this.#context.href = href;
 
@@ -842,13 +852,7 @@ class URL {
   }
 
   get href() {
-    // URLSearchParams is used on demand, and stores the search if it's used.
-    if (this.#searchParams) {
-      return StringPrototypeSlice(this.#context.href, 0, this.#context.hash_start) +
-        this.#getParamsSearch() +
-        StringPrototypeSlice(this.#context.href, this.#context.hash_start);
-    }
-    return this.#context.href;
+    return this.#getHref();
   }
 
   set href(value) {
@@ -1044,11 +1048,11 @@ class URL {
   }
 
   toString() {
-    return this.href;
+    return this.#getHref();
   }
 
   toJSON() {
-    return this.href;
+    return this.#getHref();
   }
 
   static canParse(url, base = undefined) {

From b0270bcc1aa0ee2ded35150f529c94a94238a07f Mon Sep 17 00:00:00 2001
From: MattIPv4 <me@mattcowley.co.uk>
Date: Fri, 19 Jan 2024 04:24:38 +0000
Subject: [PATCH 07/18] url: update searchParams tests to expect encoded
 results

---
 test/fixtures/wpt/url/url-searchparams.any.js            | 9 +++++----
 test/fixtures/wpt/url/urlsearchparams-stringifier.any.js | 2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/test/fixtures/wpt/url/url-searchparams.any.js b/test/fixtures/wpt/url/url-searchparams.any.js
index 9bba12e89cefd1..d5c40295e68fa4 100644
--- a/test/fixtures/wpt/url/url-searchparams.any.js
+++ b/test/fixtures/wpt/url/url-searchparams.any.js
@@ -61,12 +61,13 @@ function runURLSearchParamTests() {
     assert_equals(searchParams.get('e'), 'updated')
 
     var url2 = bURL('http://example.org/file??a=b&c=d')
-    assert_equals(url2.search, '??a=b&c=d')
-    assert_equals(url2.searchParams.toString(), '%3Fa=b&c=d')
+    var searchParams2 = url2.searchParams
+    assert_equals(url2.search, '?%3Fa=b&c=d')
+    assert_equals(searchParams2.toString(), '%3Fa=b&c=d')
 
     url2.href = 'http://example.org/file??a=b'
-    assert_equals(url2.search, '??a=b')
-    assert_equals(url2.searchParams.toString(), '%3Fa=b')
+    assert_equals(url2.search, '?%3Fa=b')
+    assert_equals(searchParams2.toString(), '%3Fa=b')
   }, 'URL.searchParams and URL.search setters, update propagation')
 }
 runURLSearchParamTests()
diff --git a/test/fixtures/wpt/url/urlsearchparams-stringifier.any.js b/test/fixtures/wpt/url/urlsearchparams-stringifier.any.js
index 6187db64b1747d..828c8a6579bb32 100644
--- a/test/fixtures/wpt/url/urlsearchparams-stringifier.any.js
+++ b/test/fixtures/wpt/url/urlsearchparams-stringifier.any.js
@@ -125,7 +125,7 @@ test(() => {
     const url = new URL('http://www.example.com/?a=b,c');
     const params = url.searchParams;
 
-    assert_equals(url.toString(), 'http://www.example.com/?a=b,c');
+    assert_equals(url.toString(), 'http://www.example.com/?a=b%2Cc');
     assert_equals(params.toString(), 'a=b%2Cc');
 
     params.append('x', 'y');

From d9c5381a28b08ceac052d734d0fe5547949bbc33 Mon Sep 17 00:00:00 2001
From: MattIPv4 <me@mattcowley.co.uk>
Date: Fri, 19 Jan 2024 06:11:20 +0000
Subject: [PATCH 08/18] url: only use URLSearchParams as store when modified

---
 lib/internal/url.js                           | 80 ++++++++++++-------
 test/fixtures/wpt/url/url-searchparams.any.js |  9 +--
 .../url/urlsearchparams-stringifier.any.js    |  2 +-
 3 files changed, 58 insertions(+), 33 deletions(-)

diff --git a/lib/internal/url.js b/lib/internal/url.js
index e69b591a46cf96..cd51ebc87c7a16 100644
--- a/lib/internal/url.js
+++ b/lib/internal/url.js
@@ -206,6 +206,8 @@ class URLContext {
   }
 }
 
+let setURLUsingSearchParams;
+let setURLSearchParamsContext;
 let getURLSearchParamsList;
 let setURLSearchParams;
 
@@ -307,7 +309,13 @@ ObjectDefineProperties(URLSearchParamsIterator.prototype, {
 class URLSearchParams {
   #searchParams = [];
 
+  // "associated url object"
+  #context;
+
   static {
+    setURLSearchParamsContext = (obj, ctx) => {
+      obj.#context = ctx;
+    };
     getURLSearchParamsList = (obj) => obj.#searchParams;
     setURLSearchParams = (obj, query) => {
       if (query === undefined) {
@@ -468,6 +476,8 @@ class URLSearchParams {
     name = StringPrototypeToWellFormed(`${name}`);
     value = StringPrototypeToWellFormed(`${value}`);
     ArrayPrototypePush(this.#searchParams, name, value);
+
+    if (this.#context) setURLUsingSearchParams(this.#context);
   }
 
   delete(name, value = undefined) {
@@ -499,6 +509,8 @@ class URLSearchParams {
         }
       }
     }
+
+    if (this.#context) setURLUsingSearchParams(this.#context);
   }
 
   get(name) {
@@ -600,6 +612,8 @@ class URLSearchParams {
     if (!found) {
       ArrayPrototypePush(list, name, value);
     }
+
+    if (this.#context) setURLUsingSearchParams(this.#context);
   }
 
   sort() {
@@ -645,6 +659,8 @@ class URLSearchParams {
         }
       }
     }
+
+    if (this.#context) setURLUsingSearchParams(this.#context);
   }
 
   // https://heycam.github.io/webidl/#es-iterators
@@ -748,6 +764,19 @@ function isURL(self) {
 class URL {
   #context = new URLContext();
   #searchParams;
+  #usingSearchParams;
+
+  static {
+    setURLUsingSearchParams = (obj) => {
+      // If URLSearchParams gets updated, use that as the source-of-truth for search.
+      obj.#usingSearchParams = true;
+
+      // If URL has an existing search, remove it without cascading back to URLSearchParams
+      if (obj.#context.hasSearch) {
+        obj.#updateContext(bindingUrl.update(obj.#context.href, updateActions.kSearch, ''), false);
+      }
+    };
+  }
 
   constructor(input, base = undefined) {
     markTransferMode(this, false, false);
@@ -793,7 +822,7 @@ class URL {
     return `${constructor.name} ${inspect(obj, opts)}`;
   }
 
-  #getContextSearch() {
+  #getSearchFromContext() {
     if (!this.#context.hasSearch) return '';
     let endsAt = this.#context.href.length;
     if (this.#context.hasHash) endsAt = this.#context.hash_start;
@@ -801,24 +830,26 @@ class URL {
     return StringPrototypeSlice(this.#context.href, this.#context.search_start, endsAt);
   }
 
-  #getParamsSearch() {
+  #getSearchFromParams() {
     if (!this.#searchParams) return '';
     if (!this.#searchParams.size) return '';
     return `?${this.#searchParams.toString()}`;
   }
 
   #getHref() {
-    // URLSearchParams is used on demand, and stores the search if it's used.
-    if (this.#searchParams) {
+    // URLSearchParams is created on demand, and is used to store the search once updated.
+    if (this.#usingSearchParams) {
       return StringPrototypeSlice(this.#context.href, 0, this.#context.hash_start) +
-        this.#getParamsSearch() +
+        this.#getSearchFromParams() +
         StringPrototypeSlice(this.#context.href, this.#context.hash_start);
     }
     return this.#context.href;
   }
 
-  #updateContext(href) {
-    this.#context.href = href;
+  #updateContext(href, updateSearchParams = !!this.#searchParams) {
+    // We'll need the previous search state if we want to update URLSearchParams.
+    const previousSearch = updateSearchParams && (this.#usingSearchParams ?
+      this.#getSearchFromParams() : this.#getSearchFromContext());
 
     const {
       0: protocol_end,
@@ -832,6 +863,7 @@ class URL {
       8: scheme_type,
     } = bindingUrl.urlComponents;
 
+    this.#context.href = href;
     this.#context.protocol_end = protocol_end;
     this.#context.username_end = username_end;
     this.#context.host_start = host_start;
@@ -842,12 +874,13 @@ class URL {
     this.#context.hash_start = hash_start;
     this.#context.scheme_type = scheme_type;
 
-    // If URLSearchParams is in use, store the search there, not here.
-    if (this.#searchParams) {
-      if (this.#context.hasSearch) {
-        setURLSearchParams(this.#searchParams, this.#getContextSearch());
-        this.#updateContext(bindingUrl.update(href, updateActions.kSearch, ''));
-      }
+    // If URLSearchParams exists, and the search changed, update it but don't use it.
+    // We want to use URL as the source-of-truth until URLSearchParams is modified.
+    // updateSearchParams is forced to false if the context update comes from URLSearchParams.
+    const currentSearch = updateSearchParams && this.#getSearchFromContext();
+    if (updateSearchParams && previousSearch !== currentSearch) {
+      setURLSearchParams(this.#searchParams, currentSearch);
+      this.#usingSearchParams = false;
     }
   }
 
@@ -1001,18 +1034,12 @@ class URL {
   }
 
   get search() {
-    // URLSearchParams is used on demand, and stores the search if it's used.
-    if (this.#searchParams) return this.#getParamsSearch();
-    return this.#getContextSearch();
+    // URLSearchParams is created on demand, and is used to store the search once updated.
+    if (this.#usingSearchParams) return this.#getSearchFromParams();
+    return this.#getSearchFromContext();
   }
 
   set search(value) {
-    // URLSearchParams is used on demand, and stores the search if it's used.
-    if (this.#searchParams) {
-      setURLSearchParams(this.#searchParams, StringPrototypeToWellFormed(`${value}`));
-      return;
-    }
-
     const href = bindingUrl.update(this.#context.href, updateActions.kSearch, StringPrototypeToWellFormed(`${value}`));
     if (href) {
       this.#updateContext(href);
@@ -1023,12 +1050,11 @@ class URL {
   get searchParams() {
     // Create URLSearchParams on demand to greatly improve the URL performance.
     if (this.#searchParams == null) {
-      this.#searchParams = new URLSearchParams(this.#getContextSearch());
+      this.#searchParams = new URLSearchParams(this.#getSearchFromContext());
+      setURLSearchParamsContext(this.#searchParams, this);
 
-      // Remove search from the URL so we're not storing it twice.
-      if (this.#context.hasSearch) {
-        this.#updateContext(bindingUrl.update(this.#context.href, updateActions.kSearch, ''));
-      }
+      // Only start using it as the source-of-truth once it is modified.
+      this.#usingSearchParams = false;
     }
     return this.#searchParams;
   }
diff --git a/test/fixtures/wpt/url/url-searchparams.any.js b/test/fixtures/wpt/url/url-searchparams.any.js
index d5c40295e68fa4..9bba12e89cefd1 100644
--- a/test/fixtures/wpt/url/url-searchparams.any.js
+++ b/test/fixtures/wpt/url/url-searchparams.any.js
@@ -61,13 +61,12 @@ function runURLSearchParamTests() {
     assert_equals(searchParams.get('e'), 'updated')
 
     var url2 = bURL('http://example.org/file??a=b&c=d')
-    var searchParams2 = url2.searchParams
-    assert_equals(url2.search, '?%3Fa=b&c=d')
-    assert_equals(searchParams2.toString(), '%3Fa=b&c=d')
+    assert_equals(url2.search, '??a=b&c=d')
+    assert_equals(url2.searchParams.toString(), '%3Fa=b&c=d')
 
     url2.href = 'http://example.org/file??a=b'
-    assert_equals(url2.search, '?%3Fa=b')
-    assert_equals(searchParams2.toString(), '%3Fa=b')
+    assert_equals(url2.search, '??a=b')
+    assert_equals(url2.searchParams.toString(), '%3Fa=b')
   }, 'URL.searchParams and URL.search setters, update propagation')
 }
 runURLSearchParamTests()
diff --git a/test/fixtures/wpt/url/urlsearchparams-stringifier.any.js b/test/fixtures/wpt/url/urlsearchparams-stringifier.any.js
index 828c8a6579bb32..6187db64b1747d 100644
--- a/test/fixtures/wpt/url/urlsearchparams-stringifier.any.js
+++ b/test/fixtures/wpt/url/urlsearchparams-stringifier.any.js
@@ -125,7 +125,7 @@ test(() => {
     const url = new URL('http://www.example.com/?a=b,c');
     const params = url.searchParams;
 
-    assert_equals(url.toString(), 'http://www.example.com/?a=b%2Cc');
+    assert_equals(url.toString(), 'http://www.example.com/?a=b,c');
     assert_equals(params.toString(), 'a=b%2Cc');
 
     params.append('x', 'y');

From a05675c7a8eb2749e004331f2e470c9e3d68e8bc Mon Sep 17 00:00:00 2001
From: MattIPv4 <me@mattcowley.co.uk>
Date: Fri, 19 Jan 2024 07:30:00 +0000
Subject: [PATCH 09/18] url: store search in URL on first access/update after
 URLSearchParams updates

---
 lib/internal/url.js | 63 +++++++++++++++++++++++----------------------
 1 file changed, 32 insertions(+), 31 deletions(-)

diff --git a/lib/internal/url.js b/lib/internal/url.js
index cd51ebc87c7a16..2b7113a6480b03 100644
--- a/lib/internal/url.js
+++ b/lib/internal/url.js
@@ -206,7 +206,7 @@ class URLContext {
   }
 }
 
-let setURLUsingSearchParams;
+let setURLSearchParamsModified;
 let setURLSearchParamsContext;
 let getURLSearchParamsList;
 let setURLSearchParams;
@@ -477,7 +477,7 @@ class URLSearchParams {
     value = StringPrototypeToWellFormed(`${value}`);
     ArrayPrototypePush(this.#searchParams, name, value);
 
-    if (this.#context) setURLUsingSearchParams(this.#context);
+    if (this.#context) setURLSearchParamsModified(this.#context);
   }
 
   delete(name, value = undefined) {
@@ -510,7 +510,7 @@ class URLSearchParams {
       }
     }
 
-    if (this.#context) setURLUsingSearchParams(this.#context);
+    if (this.#context) setURLSearchParamsModified(this.#context);
   }
 
   get(name) {
@@ -613,7 +613,7 @@ class URLSearchParams {
       ArrayPrototypePush(list, name, value);
     }
 
-    if (this.#context) setURLUsingSearchParams(this.#context);
+    if (this.#context) setURLSearchParamsModified(this.#context);
   }
 
   sort() {
@@ -660,7 +660,7 @@ class URLSearchParams {
       }
     }
 
-    if (this.#context) setURLUsingSearchParams(this.#context);
+    if (this.#context) setURLSearchParamsModified(this.#context);
   }
 
   // https://heycam.github.io/webidl/#es-iterators
@@ -764,12 +764,12 @@ function isURL(self) {
 class URL {
   #context = new URLContext();
   #searchParams;
-  #usingSearchParams;
+  #searchParamsModified;
 
   static {
-    setURLUsingSearchParams = (obj) => {
+    setURLSearchParamsModified = (obj) => {
       // If URLSearchParams gets updated, use that as the source-of-truth for search.
-      obj.#usingSearchParams = true;
+      obj.#searchParamsModified = true;
 
       // If URL has an existing search, remove it without cascading back to URLSearchParams
       if (obj.#context.hasSearch) {
@@ -836,19 +836,17 @@ class URL {
     return `?${this.#searchParams.toString()}`;
   }
 
-  #getHref() {
-    // URLSearchParams is created on demand, and is used to store the search once updated.
-    if (this.#usingSearchParams) {
-      return StringPrototypeSlice(this.#context.href, 0, this.#context.hash_start) +
-        this.#getSearchFromParams() +
-        StringPrototypeSlice(this.#context.href, this.#context.hash_start);
+  #checkSearchParams() {
+    // If URLSearchParams has been modified, reflect that back into URL.
+    if (this.#searchParamsModified) {
+      const href = bindingUrl.update(this.#context.href, updateActions.kSearch, this.#getSearchFromParams());
+      this.#updateContext(href, false);
+      this.#searchParamsModified = false;
     }
-    return this.#context.href;
   }
 
   #updateContext(href, updateSearchParams = !!this.#searchParams) {
-    // We'll need the previous search state if we want to update URLSearchParams.
-    const previousSearch = updateSearchParams && (this.#usingSearchParams ?
+    const previousSearch = updateSearchParams && (this.#searchParamsModified ?
       this.#getSearchFromParams() : this.#getSearchFromContext());
 
     const {
@@ -874,18 +872,22 @@ class URL {
     this.#context.hash_start = hash_start;
     this.#context.scheme_type = scheme_type;
 
-    // If URLSearchParams exists, and the search changed, update it but don't use it.
-    // We want to use URL as the source-of-truth until URLSearchParams is modified.
-    // updateSearchParams is forced to false if the context update comes from URLSearchParams.
+    // If URLSearchParams exists, and the search has changed, update it.
+    // Otherwise, check if URLSearchParams has been modified.
     const currentSearch = updateSearchParams && this.#getSearchFromContext();
-    if (updateSearchParams && previousSearch !== currentSearch) {
-      setURLSearchParams(this.#searchParams, currentSearch);
-      this.#usingSearchParams = false;
+    if (updateSearchParams) {
+      if (previousSearch !== currentSearch) {
+        setURLSearchParams(this.#searchParams, currentSearch);
+        this.#searchParamsModified = false;
+      } else {
+        this.#checkSearchParams();
+      }
     }
   }
 
   get href() {
-    return this.#getHref();
+    this.#checkSearchParams();
+    return this.#context.href;
   }
 
   set href(value) {
@@ -1034,8 +1036,7 @@ class URL {
   }
 
   get search() {
-    // URLSearchParams is created on demand, and is used to store the search once updated.
-    if (this.#usingSearchParams) return this.#getSearchFromParams();
+    this.#checkSearchParams();
     return this.#getSearchFromContext();
   }
 
@@ -1052,9 +1053,7 @@ class URL {
     if (this.#searchParams == null) {
       this.#searchParams = new URLSearchParams(this.#getSearchFromContext());
       setURLSearchParamsContext(this.#searchParams, this);
-
-      // Only start using it as the source-of-truth once it is modified.
-      this.#usingSearchParams = false;
+      this.#searchParamsModified = false;
     }
     return this.#searchParams;
   }
@@ -1074,11 +1073,13 @@ class URL {
   }
 
   toString() {
-    return this.#getHref();
+    this.#checkSearchParams();
+    return this.#context.href;
   }
 
   toJSON() {
-    return this.#getHref();
+    this.#checkSearchParams();
+    return this.#context.href;
   }
 
   static canParse(url, base = undefined) {

From 310d275a1c400e02b5cfff0a5ff49b9af221895b Mon Sep 17 00:00:00 2001
From: MattIPv4 <me@mattcowley.co.uk>
Date: Fri, 19 Jan 2024 07:45:24 +0000
Subject: [PATCH 10/18] url: add benchmark for param append for URL and
 URLSearchParams

---
 benchmark/url/url-searchparams-append.js | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 benchmark/url/url-searchparams-append.js

diff --git a/benchmark/url/url-searchparams-append.js b/benchmark/url/url-searchparams-append.js
new file mode 100644
index 00000000000000..cd8099b517c6f7
--- /dev/null
+++ b/benchmark/url/url-searchparams-append.js
@@ -0,0 +1,19 @@
+'use strict';
+const common = require('../common.js');
+
+const bench = common.createBenchmark(main, {
+  type: ['URL', 'URLSearchParams'],
+  n: [1e3, 1e6],
+});
+
+function main({ type, n }) {
+  const params = type === 'URL' ?
+    new URL('https://nodejs.org').searchParams :
+    new URLSearchParams();
+
+  bench.start();
+  for (let i = 0; i < n; i++) {
+    params.append('test', i);
+  }
+  bench.end(n);
+}

From b118a908f1f901002d406e4ca1049149909af5da Mon Sep 17 00:00:00 2001
From: MattIPv4 <me@mattcowley.co.uk>
Date: Fri, 19 Jan 2024 08:24:44 +0000
Subject: [PATCH 11/18] url: add benchmark to check impact of searchParams
 checking

---
 benchmark/url/url-searchparams-update.js | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 benchmark/url/url-searchparams-update.js

diff --git a/benchmark/url/url-searchparams-update.js b/benchmark/url/url-searchparams-update.js
new file mode 100644
index 00000000000000..7047df3cf0092d
--- /dev/null
+++ b/benchmark/url/url-searchparams-update.js
@@ -0,0 +1,23 @@
+'use strict';
+const common = require('../common.js');
+
+const bench = common.createBenchmark(main, {
+  searchParams: ['true', 'false'],
+  property: ['pathname', 'search'],
+  n: [1e6],
+});
+
+function main({ searchParams, property, n }) {
+  const url = new URL('https://nodejs.org');
+  if (searchParams === 'true') url.searchParams; // eslint-disable-line no-unused-expressions
+
+  const method = property === 'pathname' ?
+    (x) => url.pathname = `/${x}` :
+    (x) => url.search = `?${x}`;
+
+  bench.start();
+  for (let i = 0; i < n; i++) {
+    method(i);
+  }
+  bench.end(n);
+}

From 916374ef5605102c1c7660a313115bf8612c355a Mon Sep 17 00:00:00 2001
From: MattIPv4 <me@mattcowley.co.uk>
Date: Fri, 19 Jan 2024 16:26:21 +0000
Subject: [PATCH 12/18] url: reduce diff for changes

---
 lib/internal/url.js | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/lib/internal/url.js b/lib/internal/url.js
index 2b7113a6480b03..81900c28fff5ec 100644
--- a/lib/internal/url.js
+++ b/lib/internal/url.js
@@ -477,7 +477,9 @@ class URLSearchParams {
     value = StringPrototypeToWellFormed(`${value}`);
     ArrayPrototypePush(this.#searchParams, name, value);
 
-    if (this.#context) setURLSearchParamsModified(this.#context);
+    if (this.#context) {
+      setURLSearchParamsModified(this.#context);
+    }
   }
 
   delete(name, value = undefined) {
@@ -510,7 +512,9 @@ class URLSearchParams {
       }
     }
 
-    if (this.#context) setURLSearchParamsModified(this.#context);
+    if (this.#context) {
+      setURLSearchParamsModified(this.#context);
+    }
   }
 
   get(name) {
@@ -613,7 +617,9 @@ class URLSearchParams {
       ArrayPrototypePush(list, name, value);
     }
 
-    if (this.#context) setURLSearchParamsModified(this.#context);
+    if (this.#context) {
+      setURLSearchParamsModified(this.#context);
+    }
   }
 
   sort() {
@@ -660,7 +666,9 @@ class URLSearchParams {
       }
     }
 
-    if (this.#context) setURLSearchParamsModified(this.#context);
+    if (this.#context) {
+      setURLSearchParamsModified(this.#context);
+    }
   }
 
   // https://heycam.github.io/webidl/#es-iterators
@@ -849,6 +857,8 @@ class URL {
     const previousSearch = updateSearchParams && (this.#searchParamsModified ?
       this.#getSearchFromParams() : this.#getSearchFromContext());
 
+    this.#context.href = href;
+
     const {
       0: protocol_end,
       1: username_end,
@@ -861,7 +871,6 @@ class URL {
       8: scheme_type,
     } = bindingUrl.urlComponents;
 
-    this.#context.href = href;
     this.#context.protocol_end = protocol_end;
     this.#context.username_end = username_end;
     this.#context.host_start = host_start;
@@ -885,6 +894,11 @@ class URL {
     }
   }
 
+  toString() {
+    this.#checkSearchParams();
+    return this.#context.href;
+  }
+
   get href() {
     this.#checkSearchParams();
     return this.#context.href;
@@ -1072,11 +1086,6 @@ class URL {
     }
   }
 
-  toString() {
-    this.#checkSearchParams();
-    return this.#context.href;
-  }
-
   toJSON() {
     this.#checkSearchParams();
     return this.#context.href;

From 319eaa0b5c2ef0d5f2386964ab66c11b6178bb59 Mon Sep 17 00:00:00 2001
From: MattIPv4 <me@mattcowley.co.uk>
Date: Fri, 19 Jan 2024 17:38:30 +0000
Subject: [PATCH 13/18] url: more comments to explain lazy searchParams
 updating

---
 lib/internal/url.js | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/internal/url.js b/lib/internal/url.js
index 81900c28fff5ec..510053ac259538 100644
--- a/lib/internal/url.js
+++ b/lib/internal/url.js
@@ -776,10 +776,11 @@ class URL {
 
   static {
     setURLSearchParamsModified = (obj) => {
-      // If URLSearchParams gets updated, use that as the source-of-truth for search.
+      // When URLSearchParams changes, we lazily update URL on the next read/write for performance.
       obj.#searchParamsModified = true;
 
-      // If URL has an existing search, remove it without cascading back to URLSearchParams
+      // If URL has an existing search, remove it without cascading back to URLSearchParams.
+      // Do this to avoid any internal confusion about whether URLSearchParams or URL is up-to-date.
       if (obj.#context.hasSearch) {
         obj.#updateContext(bindingUrl.update(obj.#context.href, updateActions.kSearch, ''), false);
       }
@@ -845,7 +846,8 @@ class URL {
   }
 
   #checkSearchParams() {
-    // If URLSearchParams has been modified, reflect that back into URL.
+    // If URLSearchParams has been modified, reflect that back into URL, and do not cascade back.
+    // This is done lazily to greatly improve performance when URLSearchParams is updated repeatedly.
     if (this.#searchParamsModified) {
       const href = bindingUrl.update(this.#context.href, updateActions.kSearch, this.#getSearchFromParams());
       this.#updateContext(href, false);

From 0c44fb68d0b2b38ac1d6e98e856a72f22e17111b Mon Sep 17 00:00:00 2001
From: Matt Cowley <me@mattcowley.co.uk>
Date: Fri, 19 Jan 2024 18:09:06 +0000
Subject: [PATCH 14/18] url: cleanup from review

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
---
 lib/internal/url.js | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/lib/internal/url.js b/lib/internal/url.js
index 510053ac259538..e84d5063198b23 100644
--- a/lib/internal/url.js
+++ b/lib/internal/url.js
@@ -840,9 +840,8 @@ class URL {
   }
 
   #getSearchFromParams() {
-    if (!this.#searchParams) return '';
-    if (!this.#searchParams.size) return '';
-    return `?${this.#searchParams.toString()}`;
+    if (!this.#searchParams?.size) return '';
+    return `?${this.#searchParams}`;
   }
 
   #checkSearchParams() {
@@ -885,8 +884,8 @@ class URL {
 
     // If URLSearchParams exists, and the search has changed, update it.
     // Otherwise, check if URLSearchParams has been modified.
-    const currentSearch = updateSearchParams && this.#getSearchFromContext();
     if (updateSearchParams) {
+      const currentSearch = this.#getSearchFromContext();
       if (previousSearch !== currentSearch) {
         setURLSearchParams(this.#searchParams, currentSearch);
         this.#searchParamsModified = false;

From 065c45cba196a95623eb264a186e318847c888b9 Mon Sep 17 00:00:00 2001
From: MattIPv4 <me@mattcowley.co.uk>
Date: Fri, 19 Jan 2024 21:11:04 +0000
Subject: [PATCH 15/18] url: update variable naming, add more comments, include
 hash in benchmark

---
 benchmark/url/url-searchparams-update.js | 16 ++++++----
 lib/internal/url.js                      | 37 +++++++++++++++---------
 2 files changed, 35 insertions(+), 18 deletions(-)

diff --git a/benchmark/url/url-searchparams-update.js b/benchmark/url/url-searchparams-update.js
index 7047df3cf0092d..082d476a5d2250 100644
--- a/benchmark/url/url-searchparams-update.js
+++ b/benchmark/url/url-searchparams-update.js
@@ -1,19 +1,25 @@
 'use strict';
 const common = require('../common.js');
+const assert = require('assert');
 
 const bench = common.createBenchmark(main, {
   searchParams: ['true', 'false'],
-  property: ['pathname', 'search'],
+  property: ['pathname', 'search', 'hash'],
   n: [1e6],
 });
 
+function getMethod(url, property) {
+  if (property === 'pathname') return (x) => url.pathname = `/${x}`;
+  if (property === 'search') return (x) => url.search = `?${x}`;
+  if (property === 'hash') return (x) => url.hash = `#${x}`;
+  throw new Error(`Unsupported property "${property}"`);
+}
+
 function main({ searchParams, property, n }) {
   const url = new URL('https://nodejs.org');
-  if (searchParams === 'true') url.searchParams; // eslint-disable-line no-unused-expressions
+  if (searchParams === 'true') assert(url.searchParams);
 
-  const method = property === 'pathname' ?
-    (x) => url.pathname = `/${x}` :
-    (x) => url.search = `?${x}`;
+  const method = getMethod(url, property);
 
   bench.start();
   for (let i = 0; i < n; i++) {
diff --git a/lib/internal/url.js b/lib/internal/url.js
index e84d5063198b23..147bc0aa3ab756 100644
--- a/lib/internal/url.js
+++ b/lib/internal/url.js
@@ -844,9 +844,10 @@ class URL {
     return `?${this.#searchParams}`;
   }
 
-  #checkSearchParams() {
-    // If URLSearchParams has been modified, reflect that back into URL, and do not cascade back.
-    // This is done lazily to greatly improve performance when URLSearchParams is updated repeatedly.
+  #ensureSearchParamsUpdated() {
+    // URL is updated lazily to greatly improve performance when URLSearchParams is updated repeatedly.
+    // If URLSearchParams has been modified, reflect that back into URL.
+    // Tell #updateContext not to update URLSearchParams, as that's the value we're passing in.
     if (this.#searchParamsModified) {
       const href = bindingUrl.update(this.#context.href, updateActions.kSearch, this.#getSearchFromParams());
       this.#updateContext(href, false);
@@ -854,8 +855,14 @@ class URL {
     }
   }
 
-  #updateContext(href, updateSearchParams = !!this.#searchParams) {
-    const previousSearch = updateSearchParams && (this.#searchParamsModified ?
+  /**
+   * Update the internal context state for URL.
+   * @param {string} href New href string from `bindingUrl.update`.
+   * @param {boolean} [shouldUpdateSearchParams] If URL and URLSearchParams should be synced
+   * (happens by default if URLSearchParams has been created).
+   */
+  #updateContext(href, shouldUpdateSearchParams = !!this.#searchParams) {
+    const previousSearch = shouldUpdateSearchParams && (this.#searchParamsModified ?
       this.#getSearchFromParams() : this.#getSearchFromContext());
 
     this.#context.href = href;
@@ -882,26 +889,28 @@ class URL {
     this.#context.hash_start = hash_start;
     this.#context.scheme_type = scheme_type;
 
-    // If URLSearchParams exists, and the search has changed, update it.
-    // Otherwise, check if URLSearchParams has been modified.
-    if (updateSearchParams) {
+    // If the search string has updated, URL becomes the source of truth, and we update URLSearchParams.
+    // Otherwise, if we have a URLSearchParams, ensure that URL is up-to-date with any modification to it.
+    if (shouldUpdateSearchParams) {
       const currentSearch = this.#getSearchFromContext();
       if (previousSearch !== currentSearch) {
         setURLSearchParams(this.#searchParams, currentSearch);
         this.#searchParamsModified = false;
       } else {
-        this.#checkSearchParams();
+        this.#ensureSearchParamsUpdated();
       }
     }
   }
 
   toString() {
-    this.#checkSearchParams();
+    // Updates to URLSearchParams are lazily propagated to URL, so we need to check we're in sync.
+    this.#ensureSearchParamsUpdated();
     return this.#context.href;
   }
 
   get href() {
-    this.#checkSearchParams();
+    // Updates to URLSearchParams are lazily propagated to URL, so we need to check we're in sync.
+    this.#ensureSearchParamsUpdated();
     return this.#context.href;
   }
 
@@ -1051,7 +1060,8 @@ class URL {
   }
 
   get search() {
-    this.#checkSearchParams();
+    // Updates to URLSearchParams are lazily propagated to URL, so we need to check we're in sync.
+    this.#ensureSearchParamsUpdated();
     return this.#getSearchFromContext();
   }
 
@@ -1088,7 +1098,8 @@ class URL {
   }
 
   toJSON() {
-    this.#checkSearchParams();
+    // Updates to URLSearchParams are lazily propagated to URL, so we need to check we're in sync.
+    this.#ensureSearchParamsUpdated();
     return this.#context.href;
   }
 

From 0d0fef879bf6fa4153854064509312a2e027c5f2 Mon Sep 17 00:00:00 2001
From: MattIPv4 <me@mattcowley.co.uk>
Date: Fri, 19 Jan 2024 23:13:10 +0000
Subject: [PATCH 16/18] url: add tests to ensure lazy URLSearchParams is
 exercised

---
 test/parallel/test-whatwg-url-custom-searchparams.js | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/test/parallel/test-whatwg-url-custom-searchparams.js b/test/parallel/test-whatwg-url-custom-searchparams.js
index 75fa1779bdeb45..2edf2bb072711a 100644
--- a/test/parallel/test-whatwg-url-custom-searchparams.js
+++ b/test/parallel/test-whatwg-url-custom-searchparams.js
@@ -43,6 +43,18 @@ assert.strictEqual(sp.toString(), serialized);
 
 assert.strictEqual(m.search, `?${serialized}`);
 
+sp.delete('a');
+values.forEach((i) => sp.append('a', i));
+assert.strictEqual(m.href, `http://example.org/?${serialized}`);
+
+sp.delete('a');
+values.forEach((i) => sp.append('a', i));
+assert.strictEqual(m.toString(), `http://example.org/?${serialized}`);
+
+sp.delete('a');
+values.forEach((i) => sp.append('a', i));
+assert.strictEqual(m.toJSON(), `http://example.org/?${serialized}`);
+
 assert.strictEqual(sp[Symbol.iterator], sp.entries);
 
 let key, val;

From e9c4da240054342cbf4ccef216ac4bc2174cbd60 Mon Sep 17 00:00:00 2001
From: MattIPv4 <me@mattcowley.co.uk>
Date: Sun, 21 Jan 2024 10:44:24 +0000
Subject: [PATCH 17/18] url: fix updating url hash etc. after searchParams
 update, add test

---
 lib/internal/url.js                           | 43 ++++++++++---------
 .../test-whatwg-url-custom-searchparams.js    | 24 +++++++++++
 2 files changed, 46 insertions(+), 21 deletions(-)

diff --git a/lib/internal/url.js b/lib/internal/url.js
index 147bc0aa3ab756..0e69ff52b5edef 100644
--- a/lib/internal/url.js
+++ b/lib/internal/url.js
@@ -782,7 +782,7 @@ class URL {
       // If URL has an existing search, remove it without cascading back to URLSearchParams.
       // Do this to avoid any internal confusion about whether URLSearchParams or URL is up-to-date.
       if (obj.#context.hasSearch) {
-        obj.#updateContext(bindingUrl.update(obj.#context.href, updateActions.kSearch, ''), false);
+        obj.#updateContext(bindingUrl.update(obj.#context.href, updateActions.kSearch, ''));
       }
     };
   }
@@ -846,24 +846,21 @@ class URL {
 
   #ensureSearchParamsUpdated() {
     // URL is updated lazily to greatly improve performance when URLSearchParams is updated repeatedly.
-    // If URLSearchParams has been modified, reflect that back into URL.
-    // Tell #updateContext not to update URLSearchParams, as that's the value we're passing in.
+    // If URLSearchParams has been modified, reflect that back into URL, without cascading back.
     if (this.#searchParamsModified) {
-      const href = bindingUrl.update(this.#context.href, updateActions.kSearch, this.#getSearchFromParams());
-      this.#updateContext(href, false);
       this.#searchParamsModified = false;
+      this.#updateContext(bindingUrl.update(this.#context.href, updateActions.kSearch, this.#getSearchFromParams()));
     }
   }
 
   /**
    * Update the internal context state for URL.
    * @param {string} href New href string from `bindingUrl.update`.
-   * @param {boolean} [shouldUpdateSearchParams] If URL and URLSearchParams should be synced
-   * (happens by default if URLSearchParams has been created).
+   * @param {boolean} [shouldUpdateSearchParams] If the update has potential to update search params (href/search).
    */
-  #updateContext(href, shouldUpdateSearchParams = !!this.#searchParams) {
-    const previousSearch = shouldUpdateSearchParams && (this.#searchParamsModified ?
-      this.#getSearchFromParams() : this.#getSearchFromContext());
+  #updateContext(href, shouldUpdateSearchParams = false) {
+    const previousSearch = shouldUpdateSearchParams && this.#searchParams &&
+      (this.#searchParamsModified ? this.#getSearchFromParams() : this.#getSearchFromContext());
 
     this.#context.href = href;
 
@@ -889,16 +886,20 @@ class URL {
     this.#context.hash_start = hash_start;
     this.#context.scheme_type = scheme_type;
 
-    // If the search string has updated, URL becomes the source of truth, and we update URLSearchParams.
-    // Otherwise, if we have a URLSearchParams, ensure that URL is up-to-date with any modification to it.
-    if (shouldUpdateSearchParams) {
-      const currentSearch = this.#getSearchFromContext();
-      if (previousSearch !== currentSearch) {
-        setURLSearchParams(this.#searchParams, currentSearch);
-        this.#searchParamsModified = false;
-      } else {
-        this.#ensureSearchParamsUpdated();
+    if (this.#searchParams) {
+      // If the search string has updated, URL becomes the source of truth, and we update URLSearchParams.
+      // Only do this when we're expecting it to have changed, otherwise a change to hash etc.
+      // would incorrectly compare the URLSearchParams state to the empty URL search state.
+      if (shouldUpdateSearchParams) {
+        const currentSearch = this.#getSearchFromContext();
+        if (previousSearch !== currentSearch) {
+          setURLSearchParams(this.#searchParams, currentSearch);
+          this.#searchParamsModified = false;
+        }
       }
+
+      // If we have a URLSearchParams, ensure that URL is up-to-date with any modification to it.
+      this.#ensureSearchParamsUpdated();
     }
   }
 
@@ -918,7 +919,7 @@ class URL {
     value = `${value}`;
     const href = bindingUrl.update(this.#context.href, updateActions.kHref, value);
     if (!href) { throw new ERR_INVALID_URL(value); }
-    this.#updateContext(href);
+    this.#updateContext(href, true);
   }
 
   // readonly
@@ -1068,7 +1069,7 @@ class URL {
   set search(value) {
     const href = bindingUrl.update(this.#context.href, updateActions.kSearch, StringPrototypeToWellFormed(`${value}`));
     if (href) {
-      this.#updateContext(href);
+      this.#updateContext(href, true);
     }
   }
 
diff --git a/test/parallel/test-whatwg-url-custom-searchparams.js b/test/parallel/test-whatwg-url-custom-searchparams.js
index 2edf2bb072711a..faec86e017a2ec 100644
--- a/test/parallel/test-whatwg-url-custom-searchparams.js
+++ b/test/parallel/test-whatwg-url-custom-searchparams.js
@@ -55,6 +55,30 @@ sp.delete('a');
 values.forEach((i) => sp.append('a', i));
 assert.strictEqual(m.toJSON(), `http://example.org/?${serialized}`);
 
+sp.delete('a');
+values.forEach((i) => sp.append('a', i));
+m.href = 'http://example.org';
+assert.strictEqual(m.href, 'http://example.org/');
+assert.strictEqual(sp.size, 0);
+
+sp.delete('a');
+values.forEach((i) => sp.append('a', i));
+m.search = '';
+assert.strictEqual(m.href, 'http://example.org/');
+assert.strictEqual(sp.size, 0);
+
+sp.delete('a');
+values.forEach((i) => sp.append('a', i));
+m.pathname = '/test';
+assert.strictEqual(m.href, `http://example.org/test?${serialized}`);
+m.pathname = '';
+
+sp.delete('a');
+values.forEach((i) => sp.append('a', i));
+m.hash = '#test';
+assert.strictEqual(m.href, `http://example.org/?${serialized}#test`);
+m.hash = '';
+
 assert.strictEqual(sp[Symbol.iterator], sp.entries);
 
 let key, val;

From 04f53a74d288510c2aa0decf6eef0059b52f26e7 Mon Sep 17 00:00:00 2001
From: MattIPv4 <me@mattcowley.co.uk>
Date: Sun, 21 Jan 2024 11:00:22 +0000
Subject: [PATCH 18/18] url: update tests for invalid this

---
 test/parallel/test-whatwg-url-invalidthis.js | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/test/parallel/test-whatwg-url-invalidthis.js b/test/parallel/test-whatwg-url-invalidthis.js
index b46b5d8cceb8fa..f4d9f91ada0095 100644
--- a/test/parallel/test-whatwg-url-invalidthis.js
+++ b/test/parallel/test-whatwg-url-invalidthis.js
@@ -11,12 +11,26 @@ const assert = require('assert');
 ].forEach((i) => {
   assert.throws(() => Reflect.apply(URL.prototype[i], [], {}), {
     name: 'TypeError',
-    message: /Cannot read private member/,
+    message: /Receiver must be an instance of class/,
   });
 });
 
 [
   'href',
+  'search',
+].forEach((i) => {
+  assert.throws(() => Reflect.get(URL.prototype, i, {}), {
+    name: 'TypeError',
+    message: /Receiver must be an instance of class/,
+  });
+
+  assert.throws(() => Reflect.set(URL.prototype, i, null, {}), {
+    name: 'TypeError',
+    message: /Cannot read private member/,
+  });
+});
+
+[
   'protocol',
   'username',
   'password',
@@ -24,7 +38,6 @@ const assert = require('assert');
   'hostname',
   'port',
   'pathname',
-  'search',
   'hash',
 ].forEach((i) => {
   assert.throws(() => Reflect.get(URL.prototype, i, {}), {