querystring.stringify behavior for undefined, null, empty array[], should be improved

[danielb2]

Currently we see

$ node
> require('querystring').
    stringify({ blah: undefined, fump: null, foo: '', lonelyAmpersand: [] });
'blah=&fump=&foo=&'

Empty array appears to be broken, and should, I believe, return: lonelyAmpersand=

I think at least undefined should exclude the item from showing up at all as both null and empty string cover that already. IOW, I would have expected stringify({ blah: undefined }) to return ''

Thoughts?

[cjihrig]

I disagree that empty array is broken. It should display lonelyAmpersand=value for every value in the array. Since the array is empty, nothing is displayed.

I do agree with you that undefined values should be omitted. Of course, this is just personal opinion, and changing the behavior could break existing code.

[danielb2]

@cjihrig Yes, it does that (display for every value in the array). The case I'm referring to is where the array is empty. See my example which has lonelyAmpersand: [].

Here's an example which may illustrate it clearer:

require('querystring').stringify({ a: [], b: [], c: [], d: [], n: [] });
'&&&&'

[cjihrig]

Ah yes, the ampersand definitely shouldn't be displayed if nothing else is.

[danielb2]

@indutny any comment on the other behavior ?

[indutny]

@danielb2 I don't think that the rest are the bugs.

[danielb2]

@indutny it seems suspicious that even keys that are undefined should be showing up.

Are you sure? The behavior does makes sense for null, but not undefined as well

[arb]

I've often thought that stringify({ blah: undefined }) should return '' as well. If not the default, maybe provide an option to strip undefined values out.

If you really want to pass a query string that equals the value "undefined" you shouldn't rely on the side effect of stringify changing the value undefined into the word "undefined".

There is already precedent for this behavior with JSON.stringify().

[cjihrig]

A stripUndefined option (false by default) would be useful and maintain backwards compatibility 👍

[mattlgy]

I agree with undefined values being omitted.

This seems wrong to me:

var qs = require('querystring')

var obj = {
  foo : undefined
};

obj.bar === undefined;
obj.foo === undefined;

obj = qs.parse(qs.stringify(obj));

obj.bar === undefined;
// however...
obj.foo !== undefined;

Also differing from JSON.stringify()'s behavior in regards to undefined seems funky to me.

[danielb2]

@indutny ping?

[chrisdickinson]

My 2c: we should aim to make querystring.stringify(querystring.parse(value)) === value so that we don't discard information from the original string. To that end, we should differentiate /path/?query from /path/?query=. The former should be represented as {query: undefined}, and the latter as {query: ''}. In @mattliegey's example, obj.foo should be undefined, but foo in obj should be true; whereas bar in obj would remain false.