Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Promise support #185

Open
serge1peshcoff opened this issue Nov 16, 2016 · 9 comments · May be fixed by #295
Open

Add Promise support #185

serge1peshcoff opened this issue Nov 16, 2016 · 9 comments · May be fixed by #295
Labels
enhancement

Comments

@serge1peshcoff
Copy link
Contributor

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
Copy link

ignlg commented Nov 20, 2016 •
edited
Loading

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
Copy link

tacyarg commented Jun 7, 2017 •
edited
Loading

@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
Copy link
Owner

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
Copy link

Baterka commented Feb 12, 2018

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

@scholtzm
Copy link

scholtzm commented Feb 12, 2018 •
edited
Loading

Work has already started. See v3 branch for details.

@Baterka
Copy link

Baterka commented Feb 12, 2018

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

@Aareksio
Copy link
Contributor

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
Copy link

nolddor commented Mar 9, 2021

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

@DoctorMcKay DoctorMcKay linked a pull request Mar 29, 2021 that will close this issue
[WIP] v3 #295
Open
@srg-n
Copy link

srg-n commented Nov 26, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[WIP] v3 DoctorMcKay/node-steam-tradeoffer-manager
9 participants
@ignlg @serge1peshcoff @nolddor @DoctorMcKay @scholtzm @Baterka @srg-n @tacyarg @Aareksio