Got: Arrays in query properties

[mrmlnc]

Problem

The documentation says that query property can be represented as an object.

If the property is an array, the request path is built incorrectly.

const res = await got('https://site.com', {
  query: {
    some: ['a', 'b']
  }
});

Current behaviour

request => https://site.com/?some=a%2Cb

Expected behaviour

request => https://site.com/?some=a&some=b

Problem research

In the got@8 we use querystring.

https://github.com/sindresorhus/got/blob/ad7b361dcb2490c3864b845b979b756f13f7d89b/index.js#L552

In the got@9 we use URLSearchParams.

https://github.com/sindresorhus/got/blob/8d2e91171509bc2aec71c4483978cfd421ebcf1d/source/normalize-arguments.js#L118

// Current behaviour
new URL.URLSearchParams({ some: ['a', 'b'] }).toString() // -> some=a%2Cb

// Correct way
new URL.URLSearchParams([["some", 'a'],["some", 'b']]).toString() // -> some=a&some=b

If I understand the description correctly, I cannot pass an object to this constructor:

https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams/URLSearchParams

[szmarczak]

Yes, you can. The MDN shows:

var params4 = new URLSearchParams({"foo" : 1 , "bar" : 2});

Node API: https://nodejs.org/api/url.html#url_constructor_new_urlsearchparams_obj

@sindresorhus Maybe we should add querystring option (function)?

[sindresorhus]

There are multiple common ways to represent arrays in query strings. I don't think Got should be opinionated about which one to use.

I suggest we update the docs to say that it should be an object with key/value as string, and then show an example of using a custom URLSearchParams instance:

const res = await got('https://site.com', {
    query: new URLSearchParams([['some', 'a'],['some', 'b']])
});

And in the next major version of Got, we can validate the input and throw a user-friendly error if the object has non-string values.

Thoughts?

[sindresorhus]

@sindresorhus Maybe we should add querystring option (function)?

For what purpose? Show an example of usage.

[szmarczak]

For what purpose? Show an example of usage.

OP:

const res = await got('https://site.com', {
  query: {
    some: ['a', 'b']
  }
});

[sindresorhus]

@szmarczak No, I mean, show an example of what your proposed "querystring option (function)" would look like.

[szmarczak]

const res = await got('https://site.com', {
  querystring: require('query-string/qs/querystring'),
  query: {
    some: ['a', 'b']
  }
});

Or we could transform the object (there's no need to implement the querystring option):

{
  some: ['a', 'b'],
  foo: 'bar',
  n: 3
}

[
  ['some', 'a'],
  ['some', 'b'],
  ['foo', 'bar'],
  ['n', 3]
]

[sindresorhus]

@szmarczak That just feels like needless bloat. We already support both URLSearchParams and string, both of which can be used to achieve it:

const res = await got('https://site.com', {
    query: new URLSearchParams([['some', 'a'],['some', 'b']])
});

or

const res = await got('https://site.com', {
    query: require('query-string').stringify({some: ['a', 'b']})
});

And I would never introduce two similarly named options with different purpose. That's just bad API design.

[sindresorhus]

querystring: require('query-string/qs/querystring'),

This wouldn't always work anyway as users wouldn't be able to specify any options for those packages.

[vitkarpov]

There are multiple common ways to represent arrays in query strings.

Yes, but none of them are some=a,b, aren't they?

We already support both URLSearchParams and string, both of which can be used to achieve it

That's exactly how it could be done now and it's fine. It's just confusing that the "query as an object" leads to an invalid query string eventually.

[szmarczak]

This wouldn't always work anyway as users wouldn't be able to specify any options for those packages.

Yeah, and as you pointed out users can do query: require(...)... so probably it is worthless.

Yes, but none of them are some=a,b, aren't they?

No.

That's exactly how it could be done now and it's fine. It's just confusing that the "query as an object" leads to an invalid query string eventually.

+1. query: {some: ['a', 'b']} is more convenient than query: [['some', 'a'], ['some', 'b']] .

[sindresorhus]

Yes, but none of them are some=a,b, aren't they?

I have seen that in the wild, but not as common. I even had a PR for it that was never merged: https://github.com/sindresorhus/query-string/pull/150/files

It's just confusing that the "query as an object" leads to an invalid query string eventually.

I totally agree.

[szmarczak]

I guess we need to update the README then?

[sindresorhus]

Yup. I’ll do a PR later.