remove/opt-out of qs module

[jonathanong]

looking through qs's issues, it seems like there isn't even a consensus on how nested query string parsing should be. we should either remove it, opt out of it, or create semantics everyone could agree on and redo the qs module

[dougwilson]

This is about the req.query, is that right?

[jonathanong]

yeah

[jonathanong]

maybe do an option like app.querystring = require('qs') or something

[dougwilson]

OK, just checking :) req.query should really be a getter anyway, instead of some weird automatic middleware. Also, querystring module in core sucks because it's also unpredictable: sometimes a value is a string and sometimes it's an array.

[jonathanong]

really? i thought it was always a string. i thought qs was giving me all the different ttypes

[dougwilson]

no, even core is broken. ?foo=bar&foo=baz will give ['bar', 'baz'] in code. This is fine, but you still have to test if it is an array or not. A proper interface to query values would be req.query.get('foo') which would give first occurrence and req.query.all('foo') will give you an array of all (and if only a single value, just a one element array). This would thus guarantee the type you're getting.

[dougwilson]

It's also what like all the other languages like to do.

[jonathanong]

ohhh. i guess i'd be down for a module like that, except it should have a convenience method to return an object. the object should probably return only strings as default and optionally do the nested/array/type inferencing magic

[dougwilson]

Right. I can see the convenience method taking an option to return all values as strings only (so first value when there are multiple), all as arrays, or the mixed format. Then people will know if they asked for the mixed, they'll know what kind of checking they need to be doing or whatever.

[defunctzombie]

We should be providing a query string module but optionally allowing the user to bring their own (maybe via setting when creating the app or something)

We can do what @jonathanong suggested or optionally think about

var app = express({ qs: my_qs });

I know we haven't passed options to express like this before but maybe it could work.

[dougwilson]

Right. I see many different things going on here:

1. req.query should be a getter instead of added by middleware. This would be nice in that you only have to parse the query if you want it, instead of always without a choice.
2. We need to have some better default parsing, probably something where users don't get into trouble with just slapping crap around (because they didn't realize a value could be an array, object, or what-have-you).
3. Let people override this, especially important if it becomes a getter.

We have setting stuff already with app.set('trust proxy', val), app.set('view engine', val), etc. I think all the settings should be in the same location, so we can either add the qs parser there, or move all that stuff into the express constructor (but not have them mixed where some are one way and some are another way).

[dougwilson]

There is now a commit on the 4.7 branch that implements this. A user can opt-out of parsing with app.disable('query parser'), change it to not use qs with app.set('query parser', 'simple'), or provide their very own whatever they want parser with app.set('query parser', fn).

[hacksparrow]

Updated the 5.x doc accordingly.