Send undefined query

[gohiei]

const qs = { a: undefined, b: 1 };
request.get('my-url').query(qs).end();

// version 4.1.0
send my-url?b=1

// version 5.0.2
send my-url?a=undefined&b=1

I tested it on chrome 73.0.3683.103

[jasonblanchard]

Can we get a confirmation on whether or not this is a bug or expected behavior?

This can have all sorts of downstream consequences, so we have pinned this package to 4.x for now. I'm trying to decide if this is the responsibility of the caller, in which case we'll need to address it across our services, or if we should wait for a fix.

One data point is that this new behavior does not have parity with JSON.stringify:

const qs = { a: undefined, b: 1 };
JSON.stringify(qs); // "{"b":1}"

Note that the result has the key whose value is undefined omitted rather than set to the string literal "undefined" (tested in Chrome and Node v10.9.0).

[leeyeh]

The current behaviour in Node and browsers is different. +1 for retaining backward compatible(omit undefined fields).

[leeyeh]

The node implementation uses qs to encode queryString. qs omits undefined properties entirely: https://github.com/ljharb/qs/blame/b84acbab2c414658f15339c9e61cf9941d9d391f/README.md#L392-L396

qs.stringify({ a: null, b: undefined }) //=> 'a='

The browser implementation(v4) behaves exactly the same with qs:

function pushEncodedKeyValuePair(pairs, key, val) {
  if (val != null) {
    // push value to paires
  } else if (val === null) {
    pairs.push(encodeURIComponent(key));
  }
}

v5 changes the first condition:

function pushEncodedKeyValuePair(pairs, key, val) {
- if (val != null) {
+ if (val !== null) {
    // push value to paires
  } else if (val === null) {
    pairs.push(encodeURIComponent(key));
  }
}

It looks like a unintentional change(to conform with eslint maybe) to me.

---

👇 I created a PR to fix this by omitting undefined queries explicitly.

[wavded]

This is preventing me from upgrading to v5, sticking with v4 for now.