Skip to content

Conversation

@dsanders11
Copy link
Contributor

This is a quick and dirty PR more for discussion than immediate merging.

Currently the behavior for serializing application/x-www-form-urlencoded payloads seems to differ between Node and the browser, with Node using indices for array values and the browser not. This PR tries to make the behavior consistent, with the assumption that the behavior in Node is the intended behavior.

So, open question on which environment has the intended behavior currently. I see that in Node indices are turned off for serializing the query string, but not for the payload body when it's application/x-www-form-urlencoded.

@niftylettuce
Copy link
Collaborator

niftylettuce commented Aug 26, 2020

I'm wondering if using a package like https://www.npmjs.com/package/qs would be best, as to ensure consistency regardless of Node/Browser changes, e.g. qs.stringify({ a: ['b', 'c', 'd'] }, { indices: true });

@dsanders11
Copy link
Contributor Author

@niftylettuce, that package is already used in the Node code, isn't it? I wasn't sure if it plays nice in the browser environment. If so, that seems like a simpler way to go as it would reduce some code and keep them consistent, as you said.

@niftylettuce
Copy link
Collaborator

Yeah qs works fine in browsers, superagent is actually built to make dist builds for browsers already so should be fine to use it for both. Would love a PR! Would be super super helpful, I'm so stressed for time

@dsanders11
Copy link
Contributor Author

@niftylettuce, made PR #1591, which just uses qs and should be quick and easy. Thanks for the feedback.

@dsanders11 dsanders11 closed this Aug 30, 2020
@niftylettuce
Copy link
Collaborator

v6.1.0 has been released to npm, thank you @dsanders11

https://github.com/visionmedia/superagent/releases/tag/v6.1.0

@dsanders11 dsanders11 deleted the serialize-with-indices branch September 27, 2020 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants