Query string hook

[jamesplease]

I love this library and its commitment to simplicity. I always reach for it on new projects. One thing I always need to add in is query string support. I'm glad this lib doesn't include this out of the box, because there are so many ways to stringify query string objects.

Myself, and many devs I work with, aren't satisfied with a system where the query string is built and then appended to the URI before passing it into xhr. In other words, this API:

const query = qs.stringify({ ... });
xhr.get(`/things?${query}`)

A declarative approach would be more preferable. However, it's not trivial to wrap xhr to add in support for a qs option due to the numerous argument signatures it accepts. It's not impossible, by any means, but this is what I currently do:

export default function request(uri, options, cb) {
  let params = {};
  // This handles the `xhr(options, cb)` syntax
  if (typeof uri === 'object') {
    params = uri;
  }
  // This handles the `xhr(uri, options, cb)` syntax
  else if (typeof uri === 'string' && typeof options === 'object') {
    params = options;
    params.uri = uri;
  }
  // This handles the `xhr(uri, cb)` syntax
  else {
    params.uri = uri;
  }

  // This adds support for the `qs` option
  const urlString = params.uri ? params.uri : params.url;
  params.uri = buildUrl(urlString, params);

  let callback;
  if (typeof options === 'function') {
    callback = options;
  } else if (typeof cb === 'function') {
    callback = cb;
  }

  return xhr(params, callback);
}

A simpler alternative would be a hook within this library that would work like follows:

• If the hook is present, the value of the qs option will be passed to it. The return value of the hook is appended to the URI.
• If the hook is not present, nothing happens
• By default, there is no hook

The example code I wrote at the start of this issue would then become:

import xhr from 'xhr';
import qs from 'your-favorite-qs-lib';

xhr.queryStringStringify = qs.stringify;

xhr.get('/things', {qs: { ... });`

I'd be happy to whip up a PR if there's interest. Otherwise, I understand the desire to keep this lib slim. No worries, either way!

Thanks for reading!

[reqshark]

sounds like a dup of #72, we don't want to bloat xhr more than it already is with qs stuff

moreover,

mapping key/values for uri encoding looks different than JSON so I wouldnt want to say stringify

[jamesplease]

sounds like a dup of #72

@reqshark , although it's similar to #72, I don't think this is a duplicate. Here I am pointing out that using the qs module (or any other query string building lib) in a declarative manner is not simple, which is at odds with xhrs otherwise declarative approach to building XHRs.

we don't want to bloat xhr more than it already is with qs stuff

I understand. This is intended to be the most lightweight approach to handling query strings in an agnostic, declarative way. It'd only be a few lines of code added to this lib.

mapping key/values for uri encoding looks different than JSON so I wouldnt want to say stringify

Alright, that's fine. We could use whatever want for the API name. It doesn't have to be stringify, although many popular libs do use that nomenclature, which is why I used it in this proposal.

---

My argument is that the very teeny increase in filesize is worth the benefits. Namely:

1. Users can easily specify query strings in a declarative manner
2. The README emphasizes how you can use xhr as a client-side replacement of request, but that's only partially true. It'd be a hard sell to use this lib for universal code, as the Node code would absolutely use the qs option, and right now there's no simple way to get support for that using this lib.

[naugtur]

url: url+"?"+qs(query) seemed good enough for me, but we could consider adding a pluggable thingy to support the qs option.

I'm afraid people would find examples of it online and miss the fact that they need to provide an implementation though, so we'd have to throw a descriptive error if that happens.

[jamesplease]

I'm afraid people would find examples of it online and miss the fact that they need to provide an implementation though, so we'd have to throw a descriptive error if that happens.

That makes sense to me.

@naugtur, if you'd like me to whip up a PR to get a sense of what this would look like, let me know and I can put it together. I don't think it'd be too much work, so if it doesn't get merged, then that's fine. I'll be sure to add in an error. Speaking of throwing an Error, I'll cross-reference #58 , since that conversation is relevant to this issue.

[naugtur]

@jmeas Sure, just to see what the API could be, we can then iterate and see if we come up with something that's not going to confuse anyone.

[tbranyen]

@reqshark just an FYI, stringify while used by the JSON interface, has nothing to do with JSON. The function name is incredibly fitting especially given the Node parity: https://nodejs.org/api/querystring.html#querystring_querystring_stringify_obj_sep_eq_options

Edit: Whoops referenced the wrong dev in the comment, since updated, sorry about that @naugtur

[jamesplease]

Resolved in #160 to be released in v3. Closing.