Skip to content
This repository has been archived by the owner on Sep 6, 2024. It is now read-only.

promises support #59

Closed
capaj opened this issue Aug 20, 2017 · 14 comments
Closed

promises support #59

capaj opened this issue Aug 20, 2017 · 14 comments

Comments

@capaj
Copy link

capaj commented Aug 20, 2017

I prefer to write my JS with async/await and the readme only shows callback style. So I have to ask-is there no planned support for promises?
I already created a similar issue here:
iotaledger/iota.crypto.js#4
but it would be great to have it in both.

@chrisdukakis
Copy link
Contributor

Support for promises is planned and will be added soon.
Promises are not needed for iota.crypto.js, all methods are synchronous.

@capaj
Copy link
Author

capaj commented Aug 20, 2017

@chrisdukakis makes sense. Thanks for a quick response.

@jimthedev
Copy link

In the mean time you can use https://github.com/jimthedev/iotap which tries to promisify the existing api. It is quite small (a single fn) and PRs are of course welcome.

@brunoamancio
Copy link

brunoamancio commented Sep 25, 2017

See #89

@jkoudys
Copy link

jkoudys commented Apr 7, 2018

If we're introducing promises, how about taking it a step further with full async/await support?

@capaj
Copy link
Author

capaj commented Apr 7, 2018

@jkoudys if it's returning a promise you can await it. There's no extra step a library needs to make to allow async/await.

@jkoudys
Copy link

jkoudys commented Apr 8, 2018

@capaj I was referring to using async/await in the project code itself, instead of building promises the old fashioned way. Since there's no transpiling happening yet when building for /dist that's not feasible until we introduce babel.

@anyong
Copy link
Contributor

anyong commented Apr 8, 2018

@capaj @jkoudys first of all, thanks for keeping this conversation going!

Promise support is available in the next branch right now, but we decided to go with Bluebird since we like the API it provides, and using async/await precludes changing the Promise implementation, which means we don't get things like .asCallback. I hope this is satisfactory for now...

@capaj
Copy link
Author

capaj commented Apr 8, 2018

@anyong I am not sure why you'd say that bluebird is incompatible with async/await. async/await doesn't care whether the promise is native or bluebird. If it has methods then and catch it works every time.

@anyong
Copy link
Contributor

anyong commented Apr 8, 2018

@capaj if you try to import a custom promise (and name it Promise) in a TypeScript file with async/await, you get the following error:

[ts] Duplicate identifier 'Promise'. Compiler reserves name 'Promise' in top level scope of a module containing async functions.

There are various issues around the TypeScript repo with possible fixes:

microsoft/TypeScript#8331
microsoft/TypeScript#12323
microsoft/TypeScript#12374

The considerations for sticking with Promise were:

  • Single promise implementation throughout the code
  • Easily support callbacks for legacy compatibility
  • Not implementing workaround / special case to support custom promise with async/await

You will be able to await all async methods from iota.lib.js in your own code now, and we could definitely explore using async/await in the new codebase since it is supported by TS directly with no additional compilation steps.

@capaj
Copy link
Author

capaj commented Apr 8, 2018

thanks for clarification

@anyong
Copy link
Contributor

anyong commented Apr 8, 2018

@capaj would you be OK with closing this issue now or is there anything else we should discuss here?

@capaj
Copy link
Author

capaj commented Apr 8, 2018

well best practice is to close this when "next" branch is published to npm as the latest version, but I can live with this being closed now 💪

@anyong anyong closed this as completed Apr 8, 2018
@chrisdukakis
Copy link
Contributor

Promises on next or master are here to stay, I promise!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants