Js-ipfs: Adopt async/await and promises instead of callbacks

[fsdiogo]

I'm opening this issue so we can have a discussion about adopting async/await and promises instead of callbacks across the js repos.

You can check https://github.com/ipfs/notes/issues/289 to see where this came up.

• We could start to adopt async/await and promises, as the JavaScript ecosystem is evolving in that direction
  
  
  
  • This is a never ending discussion, but in my opinion it makes the code more readable and new developers will probably have a better understanding of the source code
  
  

As Node 6 is almost entering maintenance phase, I think we're right on time to at least start chatting about this.

I'm all for async/await, to be completely honest I really don't like the callback style 😅

My idea is not to make a huge refactor to abolish callbacks right away, at least not right now, but in future PRs with new features we could start to shift to the promises style.

What do you guys think?

[vasco-santos]

I agree that adopting async/await would improve considerably the readability of the code.

Moreover, we would be able to handle both sync and async errors within the same block, which would also turn the error handling easier.

[dryajov]

I generally like the idea of using async/await (and generators, generics, types, etc.. modern JS FTW ;)). My only concern at this point is that we'd be mixing two completely different styles, which could get ugly pretty quickly. I think we have to be careful and come up with a plan similar to the one being undertaken with https://github.com/ipfs/js-ipfs/issues/1260.

I would evaluate every new feature that is immediately available to us on each platform and where/how we can use it to improve our current implementation.

We also have to take into consideration libraries such as pull-stream that we rely on extensively and how to mix it with the more async/await'y stuff. There is an issue in the pull-stream repo that discussed how to bring it to modern js.

Finally, I'll play the devil's advocate and mention that http://caolan.github.io/async/ has a very complete set of async primitives that alleviate most of the hurdles associated with callbacks.

[pgte]

As much as I was against promises and pro callbacks, I'm fully converted now to async/await. In comparison, callbacks are clunky and harder to read.

Pull-streams use callbacks in some places (like in pull.asyncMap) but there are alternatives like pull-promise-map we could evaluate to use while pull-stream decides.

[daviddias]

Please make a complete proposal with things like:

• Pros/Cons
• Studies that show on how async/await improved the dev process and the readability of the code
• Performance Impact

Essentially, justify the time investment with something stronger than an opinion. Have in mind that there are far more important tasks at hand.

My idea is not to make a huge refactor to abolish callbacks right away, at least not right now, but in future PRs with new features we could start to shift to the promises style.

Either commit to A or B, inconsistency leads to confusion.

[justinmchase]

Personally, I think the callback style is better. Using the async library there is nothing you can't do and if you use await there are still certain kinds of control flow which can be really tricky, such as more complex async tasks like parallelism and queue/cargo situations. You still end up needing to use callbacks or wrapping a bunch of stuff in promisify functions to get await to work and then you just end up with confusing mixed code and indirection. There's also a perf cost.

The callback style code isn't really that hard to read or understand, it seems like developers who are used to doing things linearly or imperatively struggle for a while but the real problem is just learning to think async not the code or the syntax.

Overall the async / await syntax was probably an unneeded addition to javascript and makes the code more confusing ultimately. I already saw someone in this thread say it will be easier to mix sync and async functions, which in my opinion is a problem. It would be better to expose those sync functions as bugs that should be resolved rather than make it easier to not notice them.

[mikeal]

So, what I'd like a little guidance on is "what is the policy for new things." I don't think it's realistic, or worth it, to dedicate cycles to re-writing lots of working code that uses callbacks. But being that all the reference implementations are written using callbacks it implies that people should not write new modules using promises and async/await.

Many of the top level API's for modules support both callbacks and promises, but some do not. Is there a policy regarding this? Would it be fine to write a new module that only exposed a promise interface?

[daviddias]

There are some discussions and notes from discussions we had 1+ years ago but the summary of those are:

• We expose Promises, Callbacks, Readable Streams and Pull Streams at the top level, i.e. js-ipfs
• All internals use callbacks and/or pull-streams
• js-libp2p & js-ipld although being the top level of libp2p and IPLD respectively, they still have been considered internals and so we don't expose multiple APIs.

More context:

• https://github.com/ipfs/js-ipfs/issues/362
• https://github.com/ipfs/js-ipfs/issues/557

[mikeal]

@diasdavid thanks for the pointers, i'll take a look and maybe we can revisit this all in Berlin :)

[alanshaw]

This endeavour can now be tracked here https://github.com/ipfs/js-ipfs/issues/1670