fix: use qs to serialize form data in browser#1591
fix: use qs to serialize form data in browser#1591niftylettuce merged 1 commit intoforwardemail:masterfrom
Conversation
Unifies behavior for Node and browser with regards to 'application/x-www-form-urlencoded'
|
v6.1.0 has been released to npm, thank you @dsanders11 https://github.com/visionmedia/superagent/releases/tag/v6.1.0 |
|
Are you sure that this can't be fixed without qs? This PR increased package size by 33.4% |
|
This may be a better alternative, I am open to a PR. |
|
As implemented, this also breaks backwards compatibility - see #1611. It would be nice if this change in behaviour was at least documented. |
|
@slobo - not sure if "backwards compatibility" is really the correct description here. Breaking change, sure. But the behavior before was different between browser and Node, which was a bug, so one of them was going to change existing behavior to fix that. While its unfortunate, can't retain backwards compatibility when the previous behavior was the result of a bug. As @niftylettuce said in an earlier comment, a PR for any suggested changes (or documenting the change in behavior) would probably be appreciated. |
|
Thanks @dsanders11 , I appreciate the tough position, and the desire to have browser and Node behave the same. As a suggestion, this may have warranted a 7.0 release - I believe it was introduced in 6.1*. We have been using supreagent for a number of years on the client, and did not expect a minor update 6.0 > 6.1 to break our browser code. I've created a PR to document this somewhere visible: #1614
|
|
@slobo, you're right, I think it was part of v6.1. I think that was @niftylettuce just trying to get the fix into a release in a timely manner. You're right that it probably warranted a v7 to make the breaking nature clear. I think your PR is in the right direction. |
|
Yeah this can be a breaking change and I'll do a major semver bump |
|
Thanks guys! |
|
I think URLSearchParams would have been better |
Unifies behavior for Node and browser with regards to 'application/x-www-form-urlencoded'
Previous PR was #1589, but this is a better fix.