Adapt to change in superagent lib

[bmancini42]

To address issue #67

[bmancini42]

The functionality of the function in question seems to be the same, just a name change as far as I can tell.

[bmancini42]

Actually, the checks failing seem to indicate this does not work

[DevSide]

Thanks. It may be just because Travis installs superagent 3.5 and your fix is not backward compatible. Try with the latest version in package.json. If it works then we should create major version with a breaking change note.

[bmancini42]

I don't think it's just Travis ... I think it's not a drop-in replacement. I am playing around with it to see if I can get it to work. But yes, it does seem like a major version type thing. Possibly it could gracefully revert if there's a way to detect which superagent API version is being used.

[bmancini42]

Logic in the "old" _appendQueryString was processing the QS when it ran: https://github.com/visionmedia/superagent/blob/616cbb3507a7c81cc168891860ea060f3a040fd7/lib/node/index.js#L698

This code seems to be gone, period, after their refactor. Not sure what might be, or not, handling that now.

[bmancini42]

@DevSide So, I have the tests working now, but not sure if this is the right solution. As mentioned above, when superagent switched from _appendQueryString to _finalizeQueryString in ladjs/superagent#1200, they removed code that was processing the "server" .query arg. Maybe they moved the functionality somewhere else, but I couldn't find it. My solution here was to replicate what they had been doing for the QS.

[bmancini42]

@DevSide I just added a little bit for backwards compatibility. This passed the tests on both 3.5.2 and 3.6.0

[DevSide]

You are right, they actually stringify query in prototype.request server side (https://github.com/visionmedia/superagent/pull/1200/files#diff-c24ce7e3da4c0e4ff811a2b6a76f8bd9R562) and they use _finalizeQueryString in prototype.end client side (https://github.com/visionmedia/superagent/pull/1200/files#diff-50cfa59973c04321b5da0c6da0fdf4feR677).

So what about:

if (this._finalizeQueryString) {
    isNodeServer ? this.request() : this._finalizeQueryString(): 
} else {
  // superagent < 3.6
  [...old code]
}

?

[bmancini42]

Yeah that seems right. If I get a chance today I will play around with it and update the branch / PR

[bmancini42]

@DevSide see latest. Uses Request.prototype.request

[DevSide]

Good ! Last request, could you upgrade superagent to ^3.6.0 in peerDependencies and devDependencies please ?

[bmancini42]

Done!

[jbox5]

Is this PR ready to go? Would love a new version to stop the 'Unsupported' messages and traces spamming my console 👍

[DevSide]

👍

[fdubost]

Thanks for your contribution ! 💪

[fdubost]

Published on npm v3.6.0

[jbox5]

Excellent - thanks for getting it progressed everyone.

[bmancini42]

No problem -- we find superagent-mock very helpful, glad to give back