fix(url): String/template/concat of URL & URLSearchParams give href/query (Node parity)#5108
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughTwo coercion functions gain early URL/URLSearchParams detection: ChangesURL/URLSearchParams Coercion Fix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
…andle cutoff (CodeRabbit #5108)
…ral) for URL coercion guard (CodeRabbit #5108 / addr-class audit)
…andle cutoff (CodeRabbit #5108)
…ral) for URL coercion guard (CodeRabbit #5108 / addr-class audit)
76c8017 to
4dd77d2
Compare
…uery (Node parity)
ToString *coercion* of a WHATWG `URL` / `URLSearchParams` returned
`[object Object]` — `String(url)`, `` `${url}` ``, and `"" + url` all
diverged from explicit `url.toString()` (which already returns the href).
Their native `toString`s (href / the query string) aren't discoverable as
object fields, so the generic OrdinaryToPrimitive resolves the inherited
`Object.prototype.toString` and yields `[object Object]`.
Two coercion entry points needed the URL/URLSearchParams arm, mirroring
the existing Date special-cases:
- `js_jsvalue_to_string` (backs `String(x)` / explicit coercion): detect
before OrdinaryToPrimitive and return the href / query string. The
pointer is normalized to `POINTER_TAG` first because the operand can
arrive as a raw heap pointer (upper-16 == 0).
- `to_primitive_default_for_add` (backs `+` and template literals, via
`js_dynamic_string_or_number_add`): same arm before
`ordinary_to_primitive_number_for_add`, which would otherwise return the
inherited `Object.prototype.toString`'s `[object Object]`.
Reuses the existing `js_url_href_if_url` / `try_read_as_search_params`
detectors and `js_url_search_params_to_string`.
Verified vs Node v26.3.0: explicit / `String()` / template / concat of a
`URL` all yield the href, and of a `URLSearchParams` the query string;
custom `toString`/`valueOf`/plain-object/class coercion unchanged
(`[object Object]` where Node gives it); the full URL probe is
byte-identical. value/to_string + dynamic + concat unit tests pass
(the lone `builtin_prototype_methods_reject_dynamic_new` flake passes in
isolation — pre-existing parallel-test interference, not this change).
…andle cutoff (CodeRabbit #5108)
…ral) for URL coercion guard (CodeRabbit #5108 / addr-class audit)
4dd77d2 to
a474806
Compare
Problem
ToString coercion of a WHATWG
URL/URLSearchParamsreturned[object Object], diverging from expliciturl.toString()(which already works):URL/URLSearchParamscarry nativetoStrings (the href / the query string) that aren't discoverable as object fields, so the generic OrdinaryToPrimitive resolves the inheritedObject.prototype.toString→[object Object].Fix
Two coercion entry points needed a URL/URLSearchParams arm, mirroring the existing
Datespecial-cases:js_jsvalue_to_string(backsString(x)/ explicit coercion): detect URL/USP before OrdinaryToPrimitive and return the href / query string. The pointer is normalized toPOINTER_TAGfirst since the operand can arrive as a raw heap pointer (upper-16 == 0).to_primitive_default_for_add(backs+and template literals viajs_dynamic_string_or_number_add): same arm beforeordinary_to_primitive_number_for_add, which would otherwise return the inheritedObject.prototype.toString's[object Object].Reuses existing
js_url_href_if_url/try_read_as_search_paramsdetectors andjs_url_search_params_to_string.Verification (vs Node v26.3.0)
String()/ template / concat of aURL→ href; of aURLSearchParams→ query string. ✓toString/valueOf/plain-object/class coercion unchanged (CUSTOM/T/C/pt, and[object Object]forvalueOf-only & plain). ✓perry-runtimeto_string + dynamic + concat unit tests pass. (builtin_prototype_methods_reject_dynamic_newpasses in isolation; its failure under the paralleldynamicfilter is pre-existing test-interference, not this change.)Found via a
node --experimental-strip-typesdifferential sweep; relates to the parity roadmap (#4315). No version bump / changelog per maintainer instruction.Summary by CodeRabbit
String(url)now stringifiesURLobjects to theirhrefvalue.URLSearchParamsobjects now stringify to their serialized query-string form.URL/URLSearchParamsto better match nativeURL/URLSearchParamsbehavior.