[New] Prefix option for parse function #213
Conversation
…ptions, respectively
dlwalsh
changed the title
|
I also contemplated making |
lib/parse.js
Outdated
| strictNullHandling: false | ||
| }; | ||
|
|
||
| var parseValues = function parseQueryStringValues(str, options) { | ||
| var obj = {}; | ||
| var parts = str.split(options.delimiter, options.parameterLimit === Infinity ? undefined : options.parameterLimit); | ||
| var cleanStr = str.indexOf(options.prefix) === 0 ? str.substr(options.prefix.length) : str; |
There was a problem hiding this comment.
please never use substr; it's either nonstandard or deprecated. Always use slice.
|
No particular use case for a generic prefix, other than my inclination towards a flexible API. I'm happy to change it to a boolean option that would simply trim the leading |
|
I agree that your API is properly flexible :-) I think We might as well add an |
|
I've updated parse to use the I've tried to add |
lib/parse.js
Outdated
| @@ -18,7 +18,9 @@ var defaults = { | |||
|
|
|||
| var parseValues = function parseQueryStringValues(str, options) { | |||
| var obj = {}; | |||
| var parts = str.split(options.delimiter, options.parameterLimit === Infinity ? undefined : options.parameterLimit); | |||
| var cleanStr = options.ignoreQueryPrefix && str.slice(0, 1) === '?' ? str.slice(1) : str; | |||
There was a problem hiding this comment.
str.charAt(0) === '?' might be cleaner than the slice(0, 1) here - alternatively, options.ignoreQueryPrefix ? str.replace(/^\?/, '') : str?
Also, please parenthesize the ternary conditional since there's multiple items.
|
I've modified parse as requested. I've also added |
|
What version of |
|
^would like to know this as well. |
|
Whatever the next one is. This is a minor, so it'll be in the next release that a proper ^ range covers. |
| t.test('adds query prefix', function (st) { | ||
| st.equal(qs.stringify({ a: 'b' }, { addQueryPrefix: true }), '?a=b'); | ||
| st.end(); | ||
| }); |
There was a problem hiding this comment.
Hi, regarding the new addQueryPrefix option, which is wonderful news, I noticed the following situation:
> qs.stringify({ a: 1, b: 2}, { addQueryPrefix: true })
'?a=1&b=2'
> qs.stringify({}, { addQueryPrefix: true})
'?'
In my use case I don't want to keep the lonely ? if I'm stringifying an empty object.
What do you think about it?
There was a problem hiding this comment.
That seems like a reasonable modification - I haven't released the feature yet, so can you get a PR up asap for that?
The primary motivation for this change is that react-router v4 returns the
location.searchproperty with a leading question mark. Whilst it's trivial to remove this, it's nicer still to have the library handle it.Fixes #177.