Skip to content

url: don't update URL immediately on update to URLSearchParams #51520

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nodejs-github-bot merged 18 commits into from
Jan 24, 2024
Merged

url: don't update URL immediately on update to URLSearchParams #51520

Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
2b2d11b
url: remove #context from URLSearchParams
MattIPv4 Jan 19, 2024
129a1a9
url: rely on URLSearchParams if in use for search
MattIPv4 Jan 19, 2024
c2e8526
url: update URLSearchParams directly from URL search setter
MattIPv4 Jan 19, 2024
039380e
url: create href using searchParams without bindingUrl call
MattIPv4 Jan 19, 2024
e015de2
url: ensure ? is present when relying on URLSearchParams
MattIPv4 Jan 19, 2024
2487726
url: don't use public getter for toString/toJSON
MattIPv4 Jan 19, 2024
b0270bc
url: update searchParams tests to expect encoded results
MattIPv4 Jan 19, 2024
d9c5381
url: only use URLSearchParams as store when modified
MattIPv4 Jan 19, 2024
a05675c
url: store search in URL on first access/update after URLSearchParams…
MattIPv4 Jan 19, 2024
310d275
url: add benchmark for param append for URL and URLSearchParams
MattIPv4 Jan 19, 2024
b118a90
url: add benchmark to check impact of searchParams checking
MattIPv4 Jan 19, 2024
916374e
url: reduce diff for changes
MattIPv4 Jan 19, 2024
319eaa0
url: more comments to explain lazy searchParams updating
MattIPv4 Jan 19, 2024
0c44fb6
url: cleanup from review
MattIPv4 Jan 19, 2024
065c45c
url: update variable naming, add more comments, include hash in bench…
MattIPv4 Jan 19, 2024
0d0fef8
url: add tests to ensure lazy URLSearchParams is exercised
MattIPv4 Jan 19, 2024
e9c4da2
url: fix updating url hash etc. after searchParams update, add test
MattIPv4 Jan 21, 2024
04f53a7
url: update tests for invalid this
MattIPv4 Jan 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
url: don't use public getter for toString/toJSON 
This appears to keep the "calling stringifier with this = {} didn't throw TypeError" test happy
  • Loading branch information
@MattIPv4
MattIPv4 committed Jan 19, 2024
commit 2487726934472b00aea84bbcd707380cdccc0db0
22 changes: 13 additions & 9 deletions lib/internal/url.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,16 @@ class URL {
return `?${this.#searchParams.toString()}`;
Comment thread
MattIPv4 marked this conversation as resolved.
Outdated
}

#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;

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down