-
-
Notifications
You must be signed in to change notification settings - Fork 732
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
[update] stringify with comma:true to decode comma for an array as a reserved char per RFC3986 #338
base: main
Are you sure you want to change the base?
Conversation
When the format is "RFC3986", this seems correct. However (see https://github.com/ljharb/qs#rfc-3986-and-rfc-1738-space-encoding) when the format is RFC1738, for example, i would not expect it to be encoded. |
will try to do it for RFC3986 only |
4c4f45b
to
8da95de
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've rebased this and tweaked some things, but an easy solution hasn't yet presented itself.
} else { | ||
if (encoder) { | ||
var commaKey = encodeValuesOnly ? prefix : encoder(prefix, defaults.encoder, charset, 'key'); | ||
var commaValue = obj.map(function (item) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can't add a dependency on .map
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is wrong with the .map
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@petersolopov Pre-ES5 browsers won't have it. The PR can be rebased and use utils.maybeMap
instead.
… for an array as a reserved char
8da95de
to
9be0025
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #338 +/- ##
==========================================
- Coverage 99.85% 99.77% -0.08%
==========================================
Files 8 8
Lines 1336 1349 +13
Branches 164 168 +4
==========================================
+ Hits 1334 1346 +12
- Misses 2 3 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
@Om4ar are you no longer interested in completing this PR? Please leave it open even so, so that someone else can complete it. |
pr to correctly stringify a array with formatArray as a comma
keeping the comma character decoded to separate items for this character a reserved one per RFC3986
fixes: #337