Fix searchParams setter dropping the value when a URL is set#2454
Merged
sindresorhus merged 2 commits intoJun 21, 2026
Merged
Conversation
Owner
|
One thing to fix before merging: direct assignment on a bare const parameters = new URLSearchParams("foo=bar");
const options = new Options();
options.searchParams = parameters;
parameters.set("foo", "changed");
console.log(options.searchParams.toString()); // foo=changedThat seems accidently inconsistent with the constructor / extend path, which clones |
Contributor
Author
|
Good catch, now cloning the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Assigning to
options.searchParamsdirectly on anOptionsinstance silently drops the new value. The query string the request is sent with is unchanged.This happens outside the option-merging that the constructor uses internally — most notably in a
beforeRequesthook, where the docs explicitly encourage mutatingoptions:The
searchParamsdocumentation states the value "will override the query string inurl", so the request above should be sent with?foo=bar.The same applies to a fresh
Optionsinstance with no URL yet:Cause
In the
searchParamssetter, the incoming value is normalized into a localupdatedURLSearchParams. The merging branch consumesupdated, but both non-merging branches wrote/storedsearchParameters(the existing params) instead ofupdated, so the new value was never applied:The normal
got(url, {searchParams})path was unaffected because it goes through the merging branch, which is why this wasn't caught earlier.Fix
Use
updatedin the non-merging branches so the assignment overrides the query string as documented. The merging branch is unchanged.Added two regression tests covering overriding
searchParamsfrom abeforeRequesthook with both an object and a string. Both fail before the change and pass after. The existingarguments,normalize-arguments,hooks, andpaginationsuites remain green (tsc --noEmitandxoare also clean).