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

arrayformat as comma with empty array or objects #310

Closed
davidlandais opened this issue Apr 25, 2019 · 11 comments · Fixed by #350
Closed

arrayformat as comma with empty array or objects #310

davidlandais opened this issue Apr 25, 2019 · 11 comments · Fixed by #350

Comments

@davidlandais
Copy link

davidlandais commented Apr 25, 2019

const options = {
  encode: false,
  strictNullHandling: true,
  arrayFormat: 'comma',
  allowDots: true,
  addQueryPrefix: true
};

const params = {
  foo: [],
  bar: [null],
  baz: ['abc'],
};

// with arrayFormat: 'comma'
qs.stringify(params, options);
// = ?foo=&bar=&baz=abc

// with arrayFormat: 'brackets'
// = ?bar[]&baz[]=abc

stringify will produce ?foo=&bar=&baz=abc. But using brackets, indices, repeat as array format will not concat foo cause it's an empty array. comma is the only one which keep foo. Is it a bug or a conception choice ?

In lib/stringify.js, line 67, maybe you could test if it's an array with values ?

@ljharb
Copy link
Owner

ljharb commented Apr 25, 2019

Are you suggesting that comma shouldn't include foo, or that the other three should?

@davidlandais
Copy link
Author

Yes, i'm suggesting that comma shouldn't include foo :) All three other does not include it and i expected comma don't too. What do you think ?

@ljharb
Copy link
Owner

ljharb commented Apr 26, 2019

That seems reasonable to me.

@davidlandais
Copy link
Author

davidlandais commented May 2, 2019

Did you started or planing to workin on ? Cause if you need, when i got time, i will make you a pull request

@ljharb
Copy link
Owner

ljharb commented May 2, 2019

@davidlandais nope, help wanted means it's up for grabs :-) a PR would be great.

@kruchone
Copy link

kruchone commented May 9, 2019

I would like to offer my 2c on the opposite; I believe we should not strip the key out of the original object.
I am not sure qs should be assuming the intention of the empty array is to completely omit the query string entry entirely. I get that this is probably to-spec in HTTP but the reality is not all HTTP services are to-spec and this could cause problems if the intention of the caller is to actually send the blank query parameter. We are effectively stripping context out of the object when we remove the empty kv pairs.
I would argue that the caller could simply strip empty values before stringify even gets called if it really doesn't want them in the resulting string.

@ljharb
Copy link
Owner

ljharb commented May 10, 2019

@kruchone how to handle empty values seems like something that should be resolved in #226 - until then, i think it's important for all the arrayFormat options to behave the same.

@daggerjames
Copy link
Contributor

It's a bug, I'll fix that later

@issuefiler
Copy link

Suggestion

There’s a use case where you have to convey the presence of an array, empty or not, and I find [null] rather counterintuitive for that purpose. I think there should be an option to specify the way qs handles empty arrays.

@ljharb
Copy link
Owner

ljharb commented Jan 15, 2020

@issuefiler what is one use case for that?

daggerjames added a commit to daggerjames/qs that referenced this issue Jan 15, 2020
daggerjames added a commit to daggerjames/qs that referenced this issue Feb 3, 2020
@jprosevear
Copy link

@issuefiler what is one use case for that?

An index REST api endpoint that takes filters that are query params in an 'in' style:
status=[1, 2, 3]

An empty array would filter to no elements but no query params (ie status=[] where qs would stringify as empty) would get all.

daggerjames added a commit to daggerjames/qs that referenced this issue May 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants