Skip to content

Commit a0634d7

Browse files
sankalp1999targos
authored andcommitted
url: add value argument to has and delete methods
The change aims to add value argument to two methods of URLSearchParams class i.e the has method and the delete method. For has method, if value argument is provided, then use it to check for presence. For delete method, if value argument provided, use it to delete. Fixes: #47883 PR-URL: #47885 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Debadree Chatterjee <debadree333@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
1 parent 1aec718 commit a0634d7

15 files changed

+707
-607
lines changed

benchmark/common.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -360,8 +360,9 @@ function getUrlData(withBase) {
360360
for (const item of data) {
361361
if (item.failure || !item.input) continue;
362362
if (withBase) {
363-
result.push([item.input, item.base]);
364-
} else if (item.base !== 'about:blank') {
363+
// item.base might be null. It should be converted into `undefined`.
364+
result.push([item.input, item.base ?? undefined]);
365+
} else if (item.base !== null) {
365366
result.push(item.base);
366367
}
367368
}

doc/api/url.md

+30-4
Original file line numberDiff line numberDiff line change
@@ -859,11 +859,22 @@ new URLSearchParams([
859859

860860
Append a new name-value pair to the query string.
861861

862-
#### `urlSearchParams.delete(name)`
862+
#### `urlSearchParams.delete(name[, value])`
863+
864+
<!-- YAML
865+
changes:
866+
- version: REPLACEME
867+
pr-url: https://github.com/nodejs/node/pull/47885
868+
description: Add support for optional `value` argument.
869+
-->
863870

864871
* `name` {string}
872+
* `value` {string}
873+
874+
If `value` is provided, removes all name-value pairs
875+
where name is `name` and value is `value`..
865876

866-
Remove all name-value pairs whose name is `name`.
877+
If `value` is not provided, removes all name-value pairs whose name is `name`.
867878

868879
#### `urlSearchParams.entries()`
869880

@@ -918,12 +929,27 @@ are no such pairs, `null` is returned.
918929
Returns the values of all name-value pairs whose name is `name`. If there are
919930
no such pairs, an empty array is returned.
920931

921-
#### `urlSearchParams.has(name)`
932+
#### `urlSearchParams.has(name[, value])`
933+
934+
<!-- YAML
935+
changes:
936+
- version: REPLACEME
937+
pr-url: https://github.com/nodejs/node/pull/47885
938+
description: Add support for optional `value` argument.
939+
-->
922940

923941
* `name` {string}
942+
* `value` {string}
924943
* Returns: {boolean}
925944

926-
Returns `true` if there is at least one name-value pair whose name is `name`.
945+
Checks if the `URLSearchParams` object contains key-value pair(s) based on
946+
`name` and an optional `value` argument.
947+
948+
If `value` is provided, returns `true` when name-value pair with
949+
same `name` and `value` exists.
950+
951+
If `value` is not provided, returns `true` if there is at least one name-value
952+
pair whose name is `name`.
927953

928954
#### `urlSearchParams.keys()`
929955

lib/internal/url.js

+28-9
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ class URLSearchParams {
435435
}
436436
}
437437

438-
delete(name) {
438+
delete(name, value = undefined) {
439439
if (typeof this !== 'object' || this === null || !(#searchParams in this))
440440
throw new ERR_INVALID_THIS('URLSearchParams');
441441

@@ -445,12 +445,23 @@ class URLSearchParams {
445445

446446
const list = this.#searchParams;
447447
name = toUSVString(name);
448-
for (let i = 0; i < list.length;) {
449-
const cur = list[i];
450-
if (cur === name) {
451-
list.splice(i, 2);
452-
} else {
453-
i += 2;
448+
449+
if (value !== undefined) {
450+
value = toUSVString(value);
451+
for (let i = 0; i < list.length;) {
452+
if (list[i] === name && list[i + 1] === value) {
453+
list.splice(i, 2);
454+
} else {
455+
i += 2;
456+
}
457+
}
458+
} else {
459+
for (let i = 0; i < list.length;) {
460+
if (list[i] === name) {
461+
list.splice(i, 2);
462+
} else {
463+
i += 2;
464+
}
454465
}
455466
}
456467
if (this.#context) {
@@ -495,7 +506,7 @@ class URLSearchParams {
495506
return values;
496507
}
497508

498-
has(name) {
509+
has(name, value = undefined) {
499510
if (typeof this !== 'object' || this === null || !(#searchParams in this))
500511
throw new ERR_INVALID_THIS('URLSearchParams');
501512

@@ -505,11 +516,19 @@ class URLSearchParams {
505516

506517
const list = this.#searchParams;
507518
name = toUSVString(name);
519+
520+
if (value !== undefined) {
521+
value = toUSVString(value);
522+
}
523+
508524
for (let i = 0; i < list.length; i += 2) {
509525
if (list[i] === name) {
510-
return true;
526+
if (value === undefined || list[i + 1] === value) {
527+
return true;
528+
}
511529
}
512530
}
531+
513532
return false;
514533
}
515534

test/fixtures/wpt/README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ Last update:
2727
- resource-timing: https://github.com/web-platform-tests/wpt/tree/22d38586d0/resource-timing
2828
- resources: https://github.com/web-platform-tests/wpt/tree/919874f84f/resources
2929
- streams: https://github.com/web-platform-tests/wpt/tree/51750bc8d7/streams
30-
- url: https://github.com/web-platform-tests/wpt/tree/7c5c3cc125/url
30+
- url: https://github.com/web-platform-tests/wpt/tree/c4726447f3/url
3131
- user-timing: https://github.com/web-platform-tests/wpt/tree/df24fb604e/user-timing
3232
- wasm/jsapi: https://github.com/web-platform-tests/wpt/tree/cde25e7e3c/wasm/jsapi
3333
- wasm/webapi: https://github.com/web-platform-tests/wpt/tree/fd1b23eeaa/wasm/webapi

test/fixtures/wpt/url/README.md

+24-26
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,29 @@
11
## urltestdata.json
22

3-
These tests are for browsers, but the data for
4-
`a-element.html`, `url-constructor.html`, `a-element-xhtml.xhtml`, and `failure.html`
5-
is in `resources/urltestdata.json` and can be re-used by non-browser implementations.
6-
This file contains a JSON array of comments as strings and test cases as objects.
7-
The keys for each test case are:
8-
9-
* `base`: an absolute URL as a string whose [parsing] without a base of its own must succeed.
10-
This key is always present,
11-
and may have a value like `"about:blank"` when `input` is an absolute URL.
12-
* `input`: an URL as a string to be [parsed][parsing] with `base` as its base URL.
13-
* Either:
14-
* `failure` with the value `true`, indicating that parsing `input` should return failure,
15-
* or `href`, `origin`, `protocol`, `username`, `password`, `host`, `hostname`, `port`,
16-
`pathname`, `search`, and `hash` with string values;
17-
indicating that parsing `input` should return an URL record
18-
and that the getters of each corresponding attribute in that URL’s [API]
19-
should return the corresponding value.
20-
21-
The `origin` key may be missing.
22-
In that case, the API’s `origin` attribute is not tested.
23-
24-
In addition to testing that parsing `input` against `base` gives the result, a test harness for the
25-
`URL` constructor (or similar APIs) should additionally test the following pattern: if `failure` is
26-
true, parsing `about:blank` against `input` must give failure. This tests that the logic for
27-
converting base URLs into strings properly fails the whole parsing algorithm if the base URL cannot
28-
be parsed.
3+
`resources/urltestdata.json` contains URL parsing tests suitable for any URL parser implementation.
4+
5+
It's used as a source of tests by `a-element.html`, `failure.html`, `url-constructor.any.js`, and
6+
other test files in this directory.
7+
8+
The format of `resources/urltestdata.json` is a JSON array of comments as strings and test cases as
9+
objects. The keys for each test case are:
10+
11+
* `input`: a string to be parsed as URL.
12+
* `base`: null or a serialized URL (i.e., does not fail parsing).
13+
* Then either
14+
15+
* `failure` whose value is `true`, indicating that parsing `input` relative to `base` returns
16+
failure
17+
* `relativeTo` whose value is "`non-opaque-path-base`" (input does parse against a non-null base
18+
URL without an opaque path) or "`any-base`" (input parses against any non-null base URL), or is
19+
omitted in its entirety (input never parses successfully)
20+
21+
or `href`, `origin`, `protocol`, `username`, `password`, `host`, `hostname`, `port`,
22+
`pathname`, `search`, and `hash` with string values; indicating that parsing `input` should return
23+
an URL record and that the getters of each corresponding attribute in that URL’s [API] should
24+
return the corresponding value.
25+
26+
The `origin` key may be missing. In that case, the API’s `origin` attribute is not tested.
2927

3028
## setters_tests.json
3129

test/fixtures/wpt/url/failure.html

+6-5
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,15 @@
99
promise_test(() => fetch("resources/urltestdata.json").then(res => res.json()).then(runTests), "Loading data…")
1010

1111
function runTests(testData) {
12-
for(const test of testData) {
13-
if (typeof test === "string" || !test.failure || test.base !== "about:blank") {
14-
continue
12+
for (const test of testData) {
13+
if (typeof test === "string" || !test.failure || test.base !== null) {
14+
continue;
1515
}
1616

1717
const name = test.input + " should throw"
1818

19-
self.test(() => { // URL's constructor's first argument is tested by url-constructor.html
19+
self.test(() => {
20+
// URL's constructor's first argument is tested by url-constructor.html
2021
// If a URL fails to parse with any valid base, it must also fail to parse with no base, i.e.
2122
// when used as a base URL itself.
2223
assert_throws_js(TypeError, () => new URL("about:blank", test.input));
@@ -30,7 +31,7 @@
3031
// The following use cases resolve the URL input relative to the current
3132
// document's URL. If this test input could be construed as a valid URL
3233
// when resolved against a base URL, skip these cases.
33-
if (!test.inputCanBeRelative) {
34+
if (test.relativeTo === undefined) {
3435
self.test(() => {
3536
const client = new XMLHttpRequest()
3637
assert_throws_dom("SyntaxError", () => client.open("GET", test.input))

test/fixtures/wpt/url/historical.any.js

+7
Original file line numberDiff line numberDiff line change
@@ -36,4 +36,11 @@ test(() => {
3636
assert_throws_dom("DataCloneError", () => self.structuredClone(new URLSearchParams()));
3737
}, "URLSearchParams: no structured serialize/deserialize support");
3838

39+
test(() => {
40+
const url = new URL("about:blank");
41+
url.toString = () => { throw 1 };
42+
assert_throws_exactly(1, () => new URL(url), "url argument");
43+
assert_throws_exactly(1, () => new URL("about:blank", url), "base argument");
44+
}, "Constructor only takes strings");
45+
3946
done();

test/fixtures/wpt/url/resources/a-element-origin.js

+18-13
Original file line numberDiff line numberDiff line change
@@ -5,23 +5,28 @@ function setBase(base) {
55
}
66

77
function bURL(url, base) {
8-
base = base || "about:blank"
9-
setBase(base)
10-
var a = document.createElement("a")
11-
a.setAttribute("href", url)
12-
return a
8+
setBase(base);
9+
const a = document.createElement("a");
10+
a.setAttribute("href", url);
11+
return a;
1312
}
1413

15-
function runURLTests(urltests) {
16-
for(var i = 0, l = urltests.length; i < l; i++) {
17-
var expected = urltests[i]
18-
if (typeof expected === "string" || !("origin" in expected)) continue
19-
// skip without base because you cannot unset the baseURL of a document
20-
if (expected.base === null) continue;
14+
function runURLTests(urlTests) {
15+
for (const expected of urlTests) {
16+
// Skip comments and tests without "origin" expectation
17+
if (typeof expected === "string" || !("origin" in expected))
18+
continue;
19+
20+
// Fragments are relative against "about:blank" (this might always be redundant due to requiring "origin" in expected)
21+
if (expected.base === null && expected.input.startsWith("#"))
22+
continue;
23+
24+
// We cannot use a null base for HTML tests
25+
const base = expected.base === null ? "about:blank" : expected.base;
2126

2227
test(function() {
23-
var url = bURL(expected.input, expected.base)
28+
var url = bURL(expected.input, base)
2429
assert_equals(url.origin, expected.origin, "origin")
25-
}, "Parsing origin: <" + expected.input + "> against <" + expected.base + ">")
30+
}, "Parsing origin: <" + expected.input + "> against <" + base + ">")
2631
}
2732
}

test/fixtures/wpt/url/resources/a-element.js

+19-14
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,28 @@
11
promise_test(() => fetch("resources/urltestdata.json").then(res => res.json()).then(runURLTests), "Loading data…");
22

33
function setBase(base) {
4-
document.getElementById("base").href = base
4+
document.getElementById("base").href = base;
55
}
66

77
function bURL(url, base) {
8-
base = base || "about:blank"
9-
setBase(base)
10-
var a = document.createElement("a")
11-
a.setAttribute("href", url)
12-
return a
8+
setBase(base);
9+
const a = document.createElement("a");
10+
a.setAttribute("href", url);
11+
return a;
1312
}
1413

15-
function runURLTests(urltests) {
16-
for(var i = 0, l = urltests.length; i < l; i++) {
17-
var expected = urltests[i]
18-
if (typeof expected === "string") continue // skip comments
19-
// skip without base because you cannot unset the baseURL of a document
20-
if (expected.base === null) continue;
14+
function runURLTests(urlTests) {
15+
for (const expected of urlTests) {
16+
// Skip comments
17+
if (typeof expected === "string")
18+
continue;
19+
20+
// Fragments are relative against "about:blank"
21+
if (expected.relativeTo === "any-base")
22+
continue;
23+
24+
// We cannot use a null base for HTML tests
25+
const base = expected.base === null ? "about:blank" : expected.base;
2126

2227
function getKey(expected) {
2328
if (expected.protocol) {
@@ -30,7 +35,7 @@ function runURLTests(urltests) {
3035
}
3136

3237
subsetTestByKey(getKey(expected), test, function() {
33-
var url = bURL(expected.input, expected.base)
38+
var url = bURL(expected.input, base)
3439
if(expected.failure) {
3540
if(url.protocol !== ':') {
3641
assert_unreached("Expected URL to fail parsing")
@@ -49,6 +54,6 @@ function runURLTests(urltests) {
4954
assert_equals(url.pathname, expected.pathname, "pathname")
5055
assert_equals(url.search, expected.search, "search")
5156
assert_equals(url.hash, expected.hash, "hash")
52-
}, "Parsing: <" + expected.input + "> against <" + expected.base + ">")
57+
}, "Parsing: <" + expected.input + "> against <" + base + ">")
5358
}
5459
}

0 commit comments

Comments
 (0)