Use query-string instead of qs

[natew]

Just noticed that history is 60k before minification!

[natew]

I do see that requiring just the mode you want can reduce it, checking that out.

[mjackson]

We could also probably save some space in our production build by putting some of our warnings inside if (__DEV__), for example.

[taion]

We should use https://github.com/facebook/fbjs/blob/master/scripts/babel/dev-expression.js (or an equivalent) for that, IMO.

[mjackson]

@taion That looks interesting, but I've never used it. How does it work?

[taion]

It's what React itself uses. It's a Babel plugin that does the following 3 things:

• Replaces __DEV__ with a NODE_ENV check: https://github.com/facebook/fbjs/blob/master/scripts/babel/dev-expression.js#L36
• Minifies away invariant error messages: https://github.com/facebook/fbjs/blob/master/scripts/babel/dev-expression.js#L55-L61
• Strips warning calls in production: https://github.com/facebook/fbjs/blob/master/scripts/babel/dev-expression.js#L101-L103

The caveat is that when you build an application from the CJS modules, you'll have to use DefinePlugin or envify, but it turns out you already have to do so when using upstream React anyway, so no real change.

[taion]

@mjackson #124

[timdorr]

With #124, it's dropped the minified size. I think that's the best we can do without dropping qs entirely.

[nhunzaker]

There is some interesting conversation about the future of qs here: ljharb/qs#140

In the comments, @HenrikJoreteg mentions that he has started using https://github.com/sindresorhus/query-string for its lightness.

qs is extremely mature, and query-string doesn't support nesting. I know switching query string parsers is a big deal, but I wanted to bring it up as an interesting thought.

[taion]

As it stands right now, the size of the UMD bundle is not an accurate gauge of the size of the library, since in practice you'd only want to use a single history (instead of all of them).

Ultimately what we want is a way to not force users to bundle the default handlers, but that's as much an issue going from React Router to history (e.g. not always bundling createHashHistory).

I'm going to close this for now because I don't think there's anything to actively track here.

[natew]

+1 for moving to query-string.

@taion that is true in but doens't change the fact that qs is still the largest part of the bundle and the bundle itself is pretty big for what should be a small part of your app.

Personally, I just am using history as a cross browser polyfill for pushState because history.js seems outdated. This seems like a reasonable use case, and I think advocates for using the lightest possible qs parser.

[taion]

I understand what you're trying to do, but for your use case, using a custom bundle that uses only the type of history that you are actually using is going to save you a lot more space than our dropping qs for query-string.

It definitely seems worth considering using query-string instead, but that will have to be deferred to a 2.0 release.

[JedWatson]

I just jumped in here to suggest switching to query-string as well (after reading the "future of qs" issue linked above) and found this.

Size aside, the maintainers of qs have also chosen to ship ES6 code for the main entry in package.json as of 6.0.0, which means that if (in a project) you're excluding node_modules from babel you're going to have issues and need a weird edge case to not exclude qs.

At the moment history depends on qs@^4.0.0 so it's not an issue here, but that also means if you want to work with qs in your own project you have to pin it to an old version for consistency and to avoid duplication.

TL;DR qs looks like it's going to be a bag of hurt in userland before long, +1 for moving away from it (and would be nice to not implicitly recommend it too)

[taion]

A few points here:

• I don't really care about Node < 4.0.0, and they can address their distribution issues pretty easily just by adding a separate "browser" entry point that points at transpiled code. It's not a bad approach, and perhaps one we should consider down the road for packages like this one as well. I'm very happy with packages intended primarily for Node using some ES6 in "main", because they can transpile for "browser". If they're not willing to do the latter, then we have problems.
• I like the simplicity of query-string, but the argument in https://github.com/sindresorhus/query-string#nesting makes an awful strawman argument by pointing at open issues on a repo that moved over to qs over a year ago.
• Our UMD distribution has significantly bigger problems if you intend to use it as a minimum-size polyfill than its inclusion of qs - specifically, it includes all the histories, whereas in any practical situation you'd only want to use one. This remains a legitimate point in favor of using query-string, but not a strong one - I do care about users of our bundle, but it's a secondary concern.
• I'd definitely want to use qs in the query support example if we switch to query-string, because that's a practical thing that users might want to do!

The big open question to me is this -

Because we always define default query string serialization and deserialization per https://github.com/rackt/history/blob/v1.13.1/modules/useQueries.js#L5-L11, your built package will always include any dependencies we need for the default handler. Is there any good way to get around this while still offering a reasonable, user-friendly default?

The same question comes up for createHashHistory from React Router as well, and I don't know how to resolve it there either.

[taion]

Well, never mind, looks like qs intends on dropping all browser support for the moment, so I guess we have to move to query-string.

[mjackson]

I'm 👍 for moving to query-string. I think the hapi team is pretty squarely focused on node. It's been a struggle to get qs to support browsers properly and I've had to contribute a few PRs there.

[mjackson]

I just am using history as a cross browser polyfill

I'd recommend against this, @natew. You can definitely build a cross-browser polyfill on top of history, but our goal here is to just create something that works, not a proper polyfill.

[mjackson]

your built package will always include any dependencies we need for the default handler. Is there any good way to get around this while still offering a reasonable, user-friendly default?

I don't think so, @taion. The best we can do is to make it as light as possible.