Correctly handle query strings that contain =
#169
Conversation
index.js
Outdated
|
|
||
| for (const param of input.split('&')) { | ||
| let [key, value] = param.replace(/\+/g, ' ').split('='); | ||
| let [key, value] = param.replace(/\+/g, ' ').split(/\=(.+)/); // eslint-disable-line no-useless-escape |
There was a problem hiding this comment.
ESLint is correct. You don't have to escape =.
There was a problem hiding this comment.
@sindresorhus - yup I know, but then I get a different ESLint error no-div-regex Yup I will add a test. Also, it seems to fail other tests now, so I have to look into it
There was a problem hiding this comment.
Ah ok. Better to ignore the no-div-regex rule.
|
Can you add a test? |
|
I added tests and replaced the regex with a |
index.js
Outdated
|
|
||
| for (const param of input.split('&')) { | ||
| let [key, value] = param.replace(/\+/g, ' ').split('='); | ||
| let [key, value] = param.replace(/\+/g, ' ').replace('=', '<**>').split('<**>'); |
There was a problem hiding this comment.
Just ran into the same issue. This is the alternative I used for a fix https://github.com/cam-perry/query-string/commit/b65a53b55ae7a8825d62af757a587d365ba7e3b7:
const i = param.indexOf("=");
let key = param.slice(0, i);
let value = param.slice(i+1);
|
We're also experiencing this issue. When will the fix be merged? |
index.js
Outdated
|
|
||
| for (const param of input.split('&')) { | ||
| let [key, value] = param.replace(/\+/g, ' ').split('='); | ||
| let [key, value] = param.replace(/\+/g, ' ').replace('=', '<**>').split('<**>'); |
There was a problem hiding this comment.
The previous regex was better. This is risky as it will fail if the query string actually contains <**>, even if unlikely.
|
You can use my https://github.com/sindresorhus/split-on-first module. |
Some of our GraphQL IDs had `=` in their value, therefore splitting on `=` creates issues - unless we split by the 1st occurence.
For example: `?testCode_In=VGVzdENvZGVOb2RlOjQ=` will be parsed as: `{testCode_In: "VGVzdENvZGVOb2RlOjQ"}`. This leads to confusion and inaccurate search terms.
|
@sindresorhus - Nice. I used your package like you suggested. This should take care of the GraphQL ids now 👍 |
|
@sindresorhus - is there anything else that is needed in order to merge? |
sindresorhus
changed the title
=
4 participants
Some of our GraphQL IDs had
=in their value, therefore splitting on=creates issues - unless we split by the 1st occurrence.For example:
?testCode_In=VGVzdENvZGVOb2RlOjQ=will be parsed as:{testCode_In: "VGVzdENvZGVOb2RlOjQ"}. This leads to confusion and inaccurate search terms.This has resolved the issue for us.