Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strip inital question mark from query string if present #177

Closed
chrisveness opened this issue Oct 21, 2016 · 18 comments
Closed

Strip inital question mark from query string if present #177

chrisveness opened this issue Oct 21, 2016 · 18 comments

Comments

@chrisveness
Copy link

Could any leading question mark be removed from the query string supplied to qs.parse()?

Currently if the query string supplied to qs.parse() has the initial question mark, it gets included in the property name of the parsed object:

var obj = qs.parse('?a=c'); // => { '?a': 'c' }

This can be removed manually (qs.parse(location.search.slice(1))), but it would be nicer not to have to do that, and I cannot see any case where the question mark would want to be retained.

@ljharb
Copy link
Owner

ljharb commented Oct 21, 2016

That would potentially be a breaking change for anyone relying on that behavior.

@chrisveness
Copy link
Author

True. I can't imagine why someone would want that behaviour, but just because I can't imagine it doesn't mean it's not possible :)

@ljharb ljharb added this to the 7.0.0 milestone Oct 22, 2016
@dougwilson
Copy link
Contributor

When qs.parse is used for, say, a POST body, it is not expected that a leading ? would magically be stripped, as an example of a use-case that does not want automatic stripping.

@ljharb
Copy link
Owner

ljharb commented Nov 17, 2016

@dougwilson Thanks! could you provide an example POST that would result in a leading ?, to solidify your explanation?

@dougwilson
Copy link
Contributor

Any POST body... the body is a literal, so if it starts with a ? character, that character needs to be in the parsed body. This stripping is something that is just a convenience for the query string in a URL, since the ? is used a separator. There is no separator in a POST body, thus stripping it is a very smelly thing to do.

@dougwilson
Copy link
Contributor

To be clearer: I really don't care if this module wants to strip the first ? if it's there, nor even if that is the default behavior, but it would be nice if there was some kind of switch (either opt-in or opt-out). If it's a forced feature, I can work-around it with the following hack:

function parseLiteral (str) {
  if (str[0]) === '?') return qs.parse('?' + str) // double ? to preserve the ?
  else return qs.parse(str)
}

which is a lame thing :(

@ljharb
Copy link
Owner

ljharb commented Nov 17, 2016

In that case I think this should simply be an option to parse to enable ?-stripping, so you wouldn't have to opt-out of anything.

@chrisveness
Copy link
Author

chrisveness commented Nov 17, 2016

I think a POST body is the use-case I hadn't been imagining!

Given @dougwilson's observations, I agree qs shouldn't strip leading question marks by default.

It's then hard to see an option which would be simpler to use then just

var qry = qs.parse(location.search.slice(1));

since location.search etc always incude a leading question mark – so perhaps the best solution would be just to have a mention of this in the readme, to help others in the future.

@ljharb
Copy link
Owner

ljharb commented Nov 17, 2016

That's also fine, although an option that is able to detect and remove the first ? but not the first character still might be useful - ie, that does something like .replace(/^\?/, '') (but cleaner than that).

@DanielRuf
Copy link

Why not an optional argument to control this? This should not break something.

@ljharb
Copy link
Owner

ljharb commented Dec 8, 2016

@DanielRuf that's what my previous comment is suggesting.

@acidjazz
Copy link

acidjazz commented Feb 2, 2017

since this is mainly related w/ parsing the URL itself which turns into a get

qs.parse(location.search, {type: 'get'}); 

or maybe a wrapper?

qs.parseGet(location.search);

@ljharb
Copy link
Owner

ljharb commented Feb 2, 2017

The former is a bit generic, and the latter is a whole extra API to support - I'd probably make the option stripLeadingQuestionMark and default it to false, if that would work for folks.

@DanielRuf
Copy link

The former is a bit generic, and the latter is a whole extra API to support - I'd probably make the option stripLeadingQuestionMark and default it to false, if that would work for folks.

Right. Optional parameters which default to false.

@chrisveness
Copy link
Author

When I asked the original question, I thought it might be a simple request; I now realise how wrong I was.

I thought that if the initial question mark was never required, it might be nice to be able to write

var obj = qs.parse(location.search)

rather than

var obj = qs.parse(location.search.slice(1))

@dougwilson pointed out why this was naive; for my use-case, simply using .slice(1) is probably easier than an option of any kind.

So just to say – if @ljharb sees other use-cases, that's good, but don't do it for me!

I'm quite happy for this issue to be closed, I'll leave it to you whether you want to run with it.

@acidjazz
Copy link

acidjazz commented Feb 6, 2017

I think something like @ljharb suggested would still be a bit more syntactic sugar than a slice.

@ljharb
Copy link
Owner

ljharb commented Jun 13, 2017

This is done in #213.

@DanielRuf
Copy link

Great, thanks for letting us know =)

@ljharb ljharb added the parse label Jun 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants