chaplinjs/chaplin

utils.queryParams.stringify should support a `replacer` function argument

Closed this issue · 2 comments

Currently, the stringify method supports only one way to transform object values that are arrays. For instance, running the following

var params = {states: ['open', 'closed']};
utils.queryParams.stringify(params);

produces states=open&states=closed which is sensible but somewhat limiting if an endpoint (such as a server) is expecting something like states=open,closed.

JSON.stringify takes a function as a second argument (replacer) which allows the user to pass in a method to aid in the transforming process. This would allow for a different stringification process, like the one described above if utils.queryParams.stringify had a similar signature.

On a related note, a similar argument could be made for utils.queryParams.parse to support a reviver argument in a similar way as to how JSON.parse does so.

I like the idea, however I dislike the current implementation of utils.queryParams. Chaplin (or any other framework) should not include query strings parsers (or any other), because there is one in browser and because they are too complex to handle all cases with split('?').map(decodeURIComponent). I see couple of choices here:

  1. We use URLSearchParams, implement replacer and reduce 30 lines of unneeded code. Downside: we need to add polyfill to dependencies.
  2. We bundle Node's querystring module and reduce 35 lines of code. Downsides: we can not implement replacer easily, we repeat browser code and our bundle will grow 300+ lines of code.
  3. I implement replacer myself, reduce 15 lines of code, will not handle all edge cases and repeat some of browser's code.

@paulmillr, any thoughts?

3 is fine probably