DoctorMcKay / node-steam-tradeoffer-manager

Simple and sane Steam trade offer management
https://dev.doctormckay.com/forum/9-node-steam-tradeoffer-manager/
MIT License
503 stars 129 forks source link

Add Promise support #185

Open serge1peshcoff opened 7 years ago

serge1peshcoff commented 7 years ago

If a user wants to load user's inventory in a function, he/she has to do this:

function sendOffer(steamId, itemIds) {
  this.manager.loadUserInventory(steamId, 730, 2, true (loadInvErr, inv) => {
    // blah blah blah
  }
}  

Eventually, if you have a lot of callbacks, it can grow up to a mess, like in these screenshot: callback hell illustration

If this library had a Promise support, you can do it this way (that's how I imagine it):

function sendOffer(steamId, itemIds) {
  this.manager.loadUserInventory(steamId, 730, 2, true)
    .then(result => {
    // doing something with result
    }
    .catch(err => { 
      // handling errors here
    }
} 

Or (that's the way I prefer to use it) even use async/await with it (I think it's awesome, though it's not officially supported by Node.JS/ECMA standard and I have to use babel to compile it):

async function sendOffer(steamId, itemIds) {
  try {
    const result = await this.manager.loadUserInventory(steamId, 730, 2, true)
    // doing something with the result
  } catch (e) {
    // handling errors
  }
}  

We don't even need to use custom libraries (like q, bluebird etc.), because Node.JS handles Promises natively now.

My suggestion is: every time a function with callback (e.g. offer.getReceivedItems) is called, check the last argument, and if it's not defined, then return a Promise wrapper, if it's defined, just call the callback directly.

ignlg commented 7 years ago

IMO, the advantages of using bluebird are endless, beginning with a great error trace support instead of an UnhandledPromiseRejectionWarning from nowhere (shame on you, node!). And take a look at its method Promise.promisify, it's a miracle when you need to port a library.

tacyarg commented 7 years ago

@serge1peshcoff

As stated before by @DoctorMcKay the reason hes not supporting this is because the node version hes based on is v4 which is LTS. Making a wrapper for the calls you need using bluebird is very straightforward and takes no more than a hour or two. try: http://bluebirdjs.com/docs/api/promise.fromcallback.html

p.s. Writing functional code will keep you away from callback hell, but either way you should never have to go more than a layer or two deep. You must be doing something really wrong to go 13 layers deep...

If you need some help with improving the quality of your code you can check out this book: https://www.amazon.com/Functional-JavaScript-Introducing-Programming-Underscore-js/dp/1449360726/ref=sr_1_1?ie=UTF8&qid=1496809246&sr=8-1&keywords=Functional+JavaScript

DoctorMcKay commented 7 years ago

Since v4 is out of active support, I'm (slowly) starting to migrate my stuff to Node v6 (i.e. dropping support for 4). This will be a major module release when it does happen.

Baterka commented 6 years ago

Is there any progress Mr. McKay? I learned ES6 promises and now I can not stop using them 💃

scholtzm commented 6 years ago

Work has already started. See v3 branch for details.

Baterka commented 6 years ago

Nice! Is it complete already? Or can't be used in production for now.

Aareksio commented 6 years ago

Good chance, native promise support in not coming stable anytime soon, @Baterka, for now, promisify on your own, there's only a few methods.

nolddor commented 3 years ago

+1, it would be great to support promises as in steam-user.

srg-n commented 1 year ago

Any updates on this? Looks like v3 has been on hold for quite some time.