request/request-promise

New Updated Code Breaks Promise Functions

devotox opened this issue · 7 comments

This was meant to be a combination of both request and bluebird.

But now with the API changes you will no longer be able to use any of bluebird's methods other than .then and .catch.Especially one as useful as .bind(this).

Question: Will you be somehow proxying methods to Bluebird in an upcoming release or is this the way you will go from now on just adding prototyp methods directly to the request object?

Thanks for your instant feedback!

Indeed, the Bluebird promise is hidden inside and 0.3.0 only exposes .then and .catch. To expose additional methods like .bind it only need 3 extra lines of code internally.

So which option do you prefer judging from how you use Request-Promise?

  1. Expose .bind as well
    • Pro: You don't need to change your existing code.
    • Contra: Since this will certainly be not the last method we need to expose as well, the request call object will get polluted more and more which increases the risk of naming conflicts. (Maybe the Request team decides to introduce a .bind method themselves...)
  2. Expose a .promise method that returns the Bluebird promise which is currently hidden internally
    • Pro: The request call object won't be polluted too much and you can instantly make use of the full Bluebird API
    • Contra: You need to change your existing code. (All rp(...).bind(...) calls convert to rp(...).promise().bind(...).)

I think the second way would be the best and most manageable and I would
happily update my code for it

On Nov 11, 2014 3:35 PM, "Nicolai Kamenzky" notifications@github.com
wrote:

Thanks for your instant feedback!

Indeed, the Bluebird promise is hidden inside and 0.3.0 only exposes .then
and .catch. To expose additional methods like .bind it only need 3 extra
lines of code internally.

So which option do you prefer judging from how you use Request-Promise?

  1. Expose .bind as well
    • Pro: You don't need to change your existing code.
    • Contra: Since this will certainly be not the last method we need
      to expose as well, the request call object will get polluted more and more
      which increases the risk of naming conflicts. (Maybe the Request team
      decides to introduce a .bind method themselves...)
      1. Expose a .promise method that returns the Bluebird promise which
        is currently hidden internally
    • Pro: The request call object won't be polluted too much and you
      can instantly make use of the full Bluebird API
    • Contra: You need to change your existing code. (All
      rp(...).bind(...) calls convert to rp(...).promise().bind(...).)


Reply to this email directly or view it on GitHub
#27 (comment)
.

Just to be sure: Only the first method in the chain needs to be .then or .catch. Further chaining can use all Bluebird methods.

rp(...).bind(...); // Currently not supported

rp(...).then(...).bind(...); // Already supported because .then() returns a full-fledged Bluebird promise

Yes this is true and technically you could use the .then and just pass the data through and then have a bluebird instance but the solution also aligned previously would be instantly robust with the .promise() function

Indeed, thanks for your perspective. I will release 0.3.1 later today with .promise() included.

Excellent and thank you for a very useful library

I just published version 0.3.1. Have fun with it. :)