Skip to content

Commit f07cabe

Browse files
authored
fix(ext/url): align URLSearchParams with Node for node:url compat (#34119)
## Summary Brings the global `URLSearchParams` (also re-exported by `node:url`) in line with Node.js where node-compat tests assert behaviors the WHATWG spec doesn't pin down or that browsers and Node diverge on. Unblocks **14 previously-failing whatwg-url node-compat tests**. ### Changes 1. **`new URLSearchParams(null)` / `new URLSearchParams(undefined)`** now produce empty params instead of stringifying `null` to `"null"`. Browsers/spec stringify but Node treats null/undefined as "no arg" — and WPT has no test for the `null` case, so this only diverges from the letter of the spec. The constructor is rewritten to dispatch the union overloads (string / iterable-of-pairs / record) directly so it can surface Node-style `ERR_INVALID_TUPLE` and `ERR_ARG_NOT_ITERABLE` errors that the previous "Item N does not have length 2 exactly" wording masked. 2. **Iterator `.next()` error message.** The iterators produced by `mixinPairIterable` (URLSearchParams keys/values/entries) now throw `TypeError [ERR_INVALID_THIS]: Value of "this" must be of type URLSearchParamsIterator` when invoked on a non-iterator `this`, matching the message Node tests assert. 3. **`Symbol.for("nodejs.util.inspect.custom")` methods** added to `URLSearchParams.prototype` and `URL.prototype` so Node's property-descriptor sanity test sees them. The methods delegate to the existing `Deno.privateCustomInspect` path, so the spec-side formatting is unchanged. 4. **Method shape.** `entries`/`keys`/`values`/`forEach` are now defined with object-method shorthand so they no longer carry an own `prototype` property, matching Node's class-method shape. The iterator prototype gets a `Deno.privateCustomInspect` method so `util.inspect(sp.keys())` returns `URLSearchParams Iterator { 'a', 'b' }` instead of `'Object [URLSearchParams Iterator] {}'`. 5. **`forEach` callback `this`** defaults to `undefined` (per WebIDL spec and Node), not `globalThis`. A missing/non-function callback now throws `TypeError [ERR_INVALID_ARG_TYPE]` to match Node. ### Tests enabled in `tests/node_compat/config.jsonc` - `parallel/test-whatwg-url-custom-searchparams-constructor.js` - `parallel/test-whatwg-url-custom-searchparams-delete.js` - `parallel/test-whatwg-url-custom-searchparams-entries.js` - `parallel/test-whatwg-url-custom-searchparams-foreach.js` - `parallel/test-whatwg-url-custom-searchparams-get.js` - `parallel/test-whatwg-url-custom-searchparams-getall.js` - `parallel/test-whatwg-url-custom-searchparams-has.js` - `parallel/test-whatwg-url-custom-searchparams-inspect.js` - `parallel/test-whatwg-url-custom-searchparams-keys.js` - `parallel/test-whatwg-url-custom-searchparams-set.js` - `parallel/test-whatwg-url-custom-searchparams-stringifier.js` - `parallel/test-whatwg-url-custom-searchparams-values.js` - `parallel/test-whatwg-url-custom-searchparams.js` - `parallel/test-whatwg-url-properties.js` ## Test plan - [x] `cargo build --bin deno` - [x] All 14 newly-enabled node-compat tests pass end-to-end - [x] Previously-enabled `test-whatwg-url-custom-{inspect,searchparams-append,searchparams-sort}.js` still pass - [x] `deno fmt` clean - [x] `tools/lint.js --js` clean on changed files - [x] Spot-check that other webidl pair-iterable consumers (FormData/Headers) still see `this === undefined` (correct per spec) in forEach Closes denoland/orchid#99 --------- Co-authored-by: divybot <divybot@users.noreply.github.com>
1 parent 881cf73 commit f07cabe

3 files changed

Lines changed: 252 additions & 69 deletions

File tree

ext/web/00_url.js

Lines changed: 153 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,17 @@ const {
1616
op_url_stringify_search_params,
1717
} = core.ops;
1818
const {
19-
ArrayIsArray,
19+
ArrayFrom,
20+
ArrayPrototypeJoin,
2021
ArrayPrototypeMap,
2122
ArrayPrototypePush,
2223
ArrayPrototypeSome,
2324
ArrayPrototypeSort,
2425
ArrayPrototypeSplice,
26+
ObjectGetOwnPropertyDescriptor,
2527
ObjectKeys,
2628
ObjectPrototypeIsPrototypeOf,
29+
ReflectOwnKeys,
2730
SafeArrayIterator,
2831
StringPrototypeSlice,
2932
StringPrototypeStartsWith,
@@ -117,48 +120,118 @@ class URLSearchParams {
117120
/**
118121
* @param {string | [string][] | Record<string, string>} init
119122
*/
120-
constructor(init = "") {
121-
const prefix = "Failed to construct 'URL'";
122-
init = webidl.converters
123-
["sequence<sequence<USVString>> or record<USVString, USVString> or USVString"](
124-
init,
125-
prefix,
126-
"Argument 1",
127-
);
123+
constructor(init = undefined) {
128124
this[webidl.brand] = webidl.brand;
129-
if (!init) {
130-
// if there is no query string, return early
125+
// Node treats `null` and `undefined` as a missing argument (empty params).
126+
// The WHATWG spec would stringify them via the union conversion, but
127+
// browsers actually never observe this case in practice; matching Node
128+
// here unblocks node:url compat without affecting WPT.
129+
if (init === null || init === undefined) {
131130
this[_list] = [];
132131
return;
133132
}
134133

135-
if (typeof init === "string") {
136-
// Overload: USVString
137-
// If init is a string and starts with U+003F (?),
138-
// remove the first code point from init.
139-
if (init[0] == "?") {
140-
init = StringPrototypeSlice(init, 1);
134+
if (typeof init === "object" || typeof init === "function") {
135+
// Object overloads: either iterable of pairs or a record.
136+
const method = init[SymbolIterator];
137+
if (method !== undefined && method !== null) {
138+
if (typeof method !== "function") {
139+
const err = new TypeError("Query pairs must be iterable");
140+
err.code = "ERR_ARG_NOT_ITERABLE";
141+
throw err;
142+
}
143+
// Sequence<sequence<USVString>>
144+
const pairs = [];
145+
// deno-lint-ignore prefer-primordials
146+
const iter = method.call(init);
147+
if (iter == null || typeof iter.next !== "function") {
148+
const err = new TypeError(
149+
"Each query pair must be an iterable [name, value] tuple",
150+
);
151+
err.code = "ERR_INVALID_TUPLE";
152+
throw err;
153+
}
154+
while (true) {
155+
// deno-lint-ignore prefer-primordials
156+
const res = iter.next();
157+
if (res == null) {
158+
const err = new TypeError(
159+
"Each query pair must be an iterable [name, value] tuple",
160+
);
161+
err.code = "ERR_INVALID_TUPLE";
162+
throw err;
163+
}
164+
if (res.done === true) break;
165+
const pair = res.value;
166+
if (
167+
(typeof pair !== "object" && typeof pair !== "function") ||
168+
pair === null ||
169+
typeof pair[SymbolIterator] !== "function"
170+
) {
171+
const err = new TypeError(
172+
"Each query pair must be an iterable [name, value] tuple",
173+
);
174+
err.code = "ERR_INVALID_TUPLE";
175+
throw err;
176+
}
177+
const entry = [];
178+
for (const v of new SafeArrayIterator(ArrayFrom(pair))) {
179+
ArrayPrototypePush(
180+
entry,
181+
webidl.converters.USVString(v, undefined, undefined),
182+
);
183+
}
184+
if (entry.length !== 2) {
185+
const err = new TypeError(
186+
"Each query pair must be an iterable [name, value] tuple",
187+
);
188+
err.code = "ERR_INVALID_TUPLE";
189+
throw err;
190+
}
191+
ArrayPrototypePush(pairs, entry);
192+
}
193+
this[_list] = pairs;
194+
return;
141195
}
142-
this[_list] = op_url_parse_search_params(init);
143-
} else if (ArrayIsArray(init)) {
144-
// Overload: sequence<sequence<USVString>>
145-
this[_list] = ArrayPrototypeMap(init, (pair, i) => {
146-
if (pair.length !== 2) {
147-
throw new TypeError(
148-
`${prefix}: Item ${
149-
i + 0
150-
} in the parameter list does have length 2 exactly`,
196+
// Record<USVString, USVString>. We iterate own enumerable keys
197+
// (including Symbol keys so USVString coercion throws on them, like
198+
// Node does) and dedupe by the USVString-coerced name so that two keys
199+
// collapsing to U+FFFD overwrite each other instead of appearing twice
200+
// in the iterator output.
201+
const result = { __proto__: null };
202+
const allKeys = ReflectOwnKeys(init);
203+
for (let i = 0; i < allKeys.length; i++) {
204+
const key = allKeys[i];
205+
const desc = ObjectGetOwnPropertyDescriptor(init, key);
206+
if (desc !== undefined && desc.enumerable === true) {
207+
const name = webidl.converters.USVString(key, undefined, undefined);
208+
const value = webidl.converters.USVString(
209+
init[key],
210+
undefined,
211+
undefined,
151212
);
213+
result[name] = value;
152214
}
153-
return [pair[0], pair[1]];
154-
});
155-
} else {
156-
// Overload: record<USVString, USVString>
157-
this[_list] = ArrayPrototypeMap(
158-
ObjectKeys(init),
159-
(key) => [key, init[key]],
160-
);
215+
}
216+
const list = [];
217+
const resultKeys = ObjectKeys(result);
218+
for (let i = 0; i < resultKeys.length; i++) {
219+
ArrayPrototypePush(list, [resultKeys[i], result[resultKeys[i]]]);
220+
}
221+
this[_list] = list;
222+
return;
161223
}
224+
225+
// USVString overload.
226+
let str = webidl.converters.USVString(init, undefined, undefined);
227+
if (str.length === 0) {
228+
this[_list] = [];
229+
return;
230+
}
231+
if (str[0] === "?") {
232+
str = StringPrototypeSlice(str, 1);
233+
}
234+
this[_list] = op_url_parse_search_params(str);
162235
}
163236

164237
#updateUrlSearch() {
@@ -338,6 +411,42 @@ class URLSearchParams {
338411
webidl.assertBranded(this, URLSearchParamsPrototype, "URLSearchParams");
339412
return this[_list].length;
340413
}
414+
415+
// Node exposes this as a method on URLSearchParams.prototype so that
416+
// `Object.getOwnPropertyDescriptor(URLSearchParams.prototype,
417+
// Symbol.for("nodejs.util.inspect.custom"))` is not undefined. We delegate
418+
// to Deno's `privateCustomInspect` to preserve the Map-like formatting
419+
// (`URLSearchParams { 'a' => 'b' }`) that node compat tests expect.
420+
[SymbolFor("nodejs.util.inspect.custom")](_depth, inspectOptions, inspect) {
421+
webidl.assertBranded(this, URLSearchParamsPrototype, "URLSearchParams");
422+
return this[SymbolFor("Deno.privateCustomInspect")](
423+
inspect,
424+
inspectOptions,
425+
);
426+
}
427+
428+
[SymbolFor("Deno.privateCustomInspect")](inspect, inspectOptions) {
429+
webidl.assertBranded(this, URLSearchParamsPrototype, "URLSearchParams");
430+
if (
431+
typeof inspectOptions?.depth === "number" && inspectOptions.depth < 0
432+
) {
433+
return "[Object]";
434+
}
435+
const entries = this[_list];
436+
if (entries.length === 0) return "URLSearchParams {}";
437+
const pairs = ArrayPrototypeMap(
438+
entries,
439+
(e) =>
440+
`${inspect(e[0], inspectOptions)} => ${inspect(e[1], inspectOptions)}`,
441+
);
442+
const inlined = ArrayPrototypeJoin(pairs, ", ");
443+
const breakLength = inspectOptions?.breakLength;
444+
const oneLine = `URLSearchParams { ${inlined} }`;
445+
if (typeof breakLength === "number" && oneLine.length > breakLength) {
446+
return `URLSearchParams {\n ${ArrayPrototypeJoin(pairs, ",\n ")} }`;
447+
}
448+
return oneLine;
449+
}
341450
}
342451

343452
webidl.mixinPairIterable("URLSearchParams", URLSearchParams, _list, 0, 1);
@@ -490,6 +599,16 @@ class URL {
490599
);
491600
}
492601

602+
// See URLSearchParams: Node exposes this as a method so that the descriptor
603+
// lookup is not undefined. Deno's own inspector still prefers
604+
// Deno.privateCustomInspect, so this is effectively the same code path.
605+
[SymbolFor("nodejs.util.inspect.custom")](_depth, inspectOptions, inspect) {
606+
return this[SymbolFor("Deno.privateCustomInspect")](
607+
inspect,
608+
inspectOptions,
609+
);
610+
}
611+
493612
#updateSearchParams() {
494613
if (this.#queryObject !== null) {
495614
const params = this.#queryObject[_list];

0 commit comments

Comments
 (0)