Skip to content
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.

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

support array value in searchParams #1559

Closed
1 task done
rifler opened this issue Dec 17, 2020 · 11 comments
Closed
1 task done

support array value in searchParams #1559

rifler opened this issue Dec 17, 2020 · 11 comments

Comments

@rifler
Copy link
Contributor

rifler commented Dec 17, 2020

What problem are you trying to solve?

easier migration from 9.x to 11.x :)

Describe the feature

in 9.x I can pass an array to query field and it stringified like arr.join(',') (default array toString behaviour). example

got.get('https://example.com', {                                                
    query: {                                                                                                                                                                                                                        
        a: ['b', 'c'],                                                          
    },                                                                          
    hooks: {                                                                    
        beforeRequest: [
            (options) => { console.log(options); }
        ],
    },                                                                          
});

// logs: path: '/?a=b%2Cc',

in 11.x query was renamed to searchParams and validation was added. Now exception is thrown when I try to pass an array:

got.get('https://example.com', {                                                
    searchParams: {                                                                                                                                                                                                                               
        a: ['b', 'c'],                                                          
    },                                                                                                                                                 
});

https://github.com/sindresorhus/got/blob/v11.8.1/source/core/index.ts#L1111

I add && !is.array(value) to that if and it seems to work.

URLSearchParams supports arrays too, in different ways:

// 1
new URLSearchParams({ a: [1, 2] }).toString() // a=1%2C2

// 2
var params = new URLSearchParams();
params.append('a', [1, 2]);
params.toString(); // a=1%2C2

Maybe we can return array to the list of allowed values?

Checklist

  • I have read the documentation and made sure this feature doesn't already exist.
@sindresorhus
Copy link
Owner

The defaults are intentionally strict to prevent mistakes. However, the searchParams option also accepts a URLSearchParams instance, so you can just pass that instead:

{
	searchParams: new URLSearchParams(Object.entries({
		a: ['b', 'c']
	}))
}

@rifler
Copy link
Contributor Author

rifler commented Dec 17, 2020

the searchParams option also accepts a URLSearchParams instance, so you can just pass that instead

Yeah, I can find some workarounds and solve that problem in my code, but it is not easy

The defaults are intentionally strict to prevent mistakes

I don't think array is a mistake. Also it works in 9.x and this behaviour can be ported to 11.x in one line

@sindresorhus
Copy link
Owner

I don't think array is a mistake. Also it works in 9.x and this behaviour can be ported to 11.x in one line

There are multiple common ways to represent arrays in a query string. Comma is just one of them. It's better to be explicit about it than using implicit stringification behavior.

You could also use https://github.com/sindresorhus/query-string and just pass the stringified query parameters to searchParams.

This behavior has been like this for a very long time now and (AFAIK) you're the first one to complain about it, so it doesn't seem like such a big issue.

@rifler
Copy link
Contributor Author

rifler commented Dec 17, 2020

sad

@rifler rifler closed this as completed Dec 17, 2020
@rifler
Copy link
Contributor Author

rifler commented Dec 17, 2020

maybe use query-string lib inside got and allow to pass options to manage it?

@rifler
Copy link
Contributor Author

rifler commented Dec 17, 2020

It's better to be explicit about it than using implicit stringification behavior.

On the other hand no one can forbid user to do searchParams: new URLSearchParams(anyPlainObject) and behaviour become inconsistent.
I think if official URLSearchParams allows something, that other supported format should too

@rifler rifler reopened this Dec 17, 2020
@sindresorhus
Copy link
Owner

maybe use query-string lib inside got and allow to pass options to manage it?

No. We don't want to have to proxy all those options through Got. It's an edge-case anyway. The default works for most and when it doesn't, there are many ways to get different behavior as I described in my previous comment.

@rifler
Copy link
Contributor Author

rifler commented Dec 17, 2020

We don't want to have to proxy all those options through Got

as I understand documentation, there is only one option options, not many

there are many ways to get different behavior as I described in my previous comment.

It is not easy in large codebases

it doesn't seem like such a big issue.

What if it is a survival bias? Maybe many users don't create issues and just starting to use other library silently or never updates to new major versions?

@sindresorhus
Copy link
Owner

I'll leave this issue open so the other maintainer and users can chime in, but I've shared my opinion about this.

@szmarczak
Copy link
Collaborator

I agree 100% with @sindresorhus

@Giotino
Copy link
Collaborator

Giotino commented Dec 21, 2020

A small note on the Got 9 behavior.

URLSearchParams follows the WHATWG spec that defines it as key-value iterable (where key and value are both strings). I think that the "array feature" that is using "," as separator is an undefined behavior due to its implementation, it's not within the spec.

https://url.spec.whatwg.org/#interface-urlsearchparams

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants