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.

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

Awesome Endeavour: Async Iterators #1670

Open
alanshaw opened this issue Oct 25, 2018 · 45 comments

Comments

@alanshaw
Copy link
Member

commented Oct 25, 2018

JS IPFS supports two types of stream at the API level, but uses pull streams for internals. If I was working on js-ipfs at the time I'd have made the same decision. Since then, async/await became part of the JS language and the majority of JavaScript runtimes now support async/await, async iterators and for/await/of (i.e. no need to transpile). These tools give us the power to stream data without needing to rely on a library.

Just because there are new language features available doesn't mean we should switch to using them. It's a significant upheaval to change the core interface spec and its implementations (js-ipfs, js-ipfs-api etc.) without good reason.

That said, it has become apparent that there are a growing number of good reasons to do this:

  • Reduction in bundle size - no need to bundle two different stream implementations, and their eco-system helper modules, no need for the async module.
  • Reduce npm install time - fewer dependencies to install.
  • Allows us to remove a bunch of plumbing code that converts Node.js streams to pull streams and vice versa.
  • Reduces API surface area, no addPullStream, addReadableStream.
  • Building an interface-ipfs-core compatible interface becomes a whole lot easier, no dual promise/callback API and no multiple stream implementation variations of the same function. It would also reduce the number of tests in the interface-ipfs-core test suite for the same reasons.
  • Node.js readable streams are now async iterators thanks to #17755!
  • Of note, it is trivial to convert from pull stream to (async) iterator and vice versa.
  • Unhandled throws that cannot be caught will no longer be a problem
  • Better stack traces, stacks no longer clipped at async boundaries, await stack traces better than promise stack traces
  • A modern, up to date and cutting edge API will aid community contributions and adoption.

The rough plan is:

  1. Drop support for dual callback/promise based APIs
  2. Expose only APIs that return promises or iterators for async actions
  3. Use async/await over then/catch when dealing with promises

This will require significant discussion and coordination from the JS teams. We'll need to reach agreement on the best API to expose for each module and manage releases carefully.

Below is a table documenting the multiformats, libp2p, ipld and ipfs modules that will likely need work. I suspect that some of these modules can be removed as they do not expose an async API. Likewise there's probably modules that got missed. If you notice either way then please edit the table or comment below.

If you'd like to own this enhancement task for a module then please comment below (or add yourself to the table if you know what you are doing). Please open a PR against the module asap (does not have to be anywhere near complete!) so we can add it here also and track progress.

  • 🍎 = Not started
  • 🍊 = In progress
  • 🍏 = Complete
Module PR Owner Status Priority
Multiformats
multihashing-async multiformats/js-multihashing-async#37 @hugomrdias 🍏 P0
multistream-select https://github.com/alanshaw/it-multistream-select @alanshaw 🍊 P1
libp2p
libp2p 🍎 P0
libp2p-bootstrap libp2p/js-libp2p-bootstrap#89 @dirkmc 🍏 P3+
libp2p-crypto libp2p/js-libp2p-crypto#131 @alanshaw 🍏 P3+
libp2p-crypto-secp256k1 libp2p/js-libp2p-crypto-secp256k1#9 @alanshaw 🍏 P1
libp2p-delegated-content-routing libp2p/js-libp2p-delegated-content-routing#7 @achingbrain 🍏 P3+
libp2p-delegated-peer-routing libp2p/js-libp2p-delegated-peer-routing#8 @achingbrain 🍏 P3+
libp2p-floodsub @vasco-santos 🍎 P3+
libp2p-pubsub @vasco-santos 🍎 P0
libp2p-kad-dht libp2p/js-libp2p-kad-dht#82 @dirkmc 🍊 P3+
libp2p-keychain libp2p/js-libp2p-keychain#37 @jacobheun 🍊 P3+
libp2p-mdns libp2p/js-libp2p-mdns#78 @dirkmc 🍊 P3+
libp2p-mplex libp2p/js-libp2p-mplex#94 @alanshaw 🍊 P1
libp2p-nat-mngr 🍎 P3+
libp2p-record libp2p/js-libp2p-record#13 @dirkmc 🍏 P1
libp2p-rendezvous 🍎 P3+
libp2p-secio libp2p/js-libp2p-secio#108 @mkg20001 🍊 P3+
libp2p-spdy 🍎 P1
libp2p-tcp libp2p/js-libp2p-tcp#102 @dirkmc 🍊 P3+
libp2p-utp @vasco-santos 🍎 P1
libp2p-webrtc-direct libp2p/js-libp2p-webrtc-direct#30 @vasco-santos 🍊 P1
libp2p-webrtc-star libp2p/js-libp2p-webrtc-star#183 @vasco-santos 🍊 P3+
libp2p-websocket-star @alanshaw 🍎 P3+
libp2p-websocket-star-rendezvous libp2p/js-libp2p-websocket-star-rendezvous#35 @achingbrain 🍏 P3+
libp2p-websockets libp2p/js-libp2p-websockets#82 @alanshaw 🍊 P1
peer-id libp2p/js-peer-id#87 @hacdias 🍏 P3+
peer-info libp2p/js-peer-info#67 @hacdias 🍏 P3+
interface-connection libp2p/interface-connection#29 @vasco-santos 🍊 P0
interface-transport libp2p/interface-transport#44 @dirkmc 🍏 P0
interface-stream-muxer libp2p/interface-stream-muxer#55 @alanshaw 🍊 P3+
IPLD Please read #1670 (comment) before contributing.
ipns ipfs/js-ipns#19 @dirkmc 🍏 P3+
ipld ipld/js-ipld#190 @vmx 🍏 P3+
ipld-bitcoin ipld/js-ipld-bitcoin#48 @vmx 🍏 P3+
ipld-dag-pb ipld/js-ipld-dag-pb#124 @vmx 🍏 P1
ipld-dag-cbor ipld/js-ipld-dag-cbor#107 @vmx 🍏 P1
ipld-ethereum ipld/js-ipld-ethereum#51 @vmx 🍏 P1
ipld-git ipld/js-ipld-git#51 @vmx 🍏 P1
ipld-graph-builder 🍎 P3+
ipld-raw ipld/js-ipld-raw#32 @vmx 🍏 P1
ipld-selector 🍎 P3+
ipld-zcash ipld/js-ipld-zcash#39 @vmx 🍏 P1
IPFS
interface-datastore ipfs/interface-datastore#25 @alanshaw 🍏 P0
datastore-core ipfs/js-datastore-core#17 @zcstarr 🍏 P1
datastore-fs ipfs/js-datastore-fs#22 @zcstarr 🍏 P1
datastore-level ipfs/js-datastore-level#12 @alanshaw 🍏 P2
datastore-pubsub ipfs/js-datastore-pubsub#15 @vasco-santos 🍊 P3+
datastore-s3 ipfs/js-datastore-s3#17 @dirkmc 🍏 P2
interface-ipfs-core https://github.com/ipfs/interface-ipfs-core/tree/feat/async-iterators @alanshaw 🍊 P3+
interop @alanshaw 🍎 P3+
ipfs @alanshaw 🍎 P3+
ipfs-http-client @alanshaw 🍎 P3+
ipfs-bitswap ipfs/js-ipfs-bitswap#202 @nijynot 🍊 P3+
ipfs-blob-store ipfs-shipyard/ipfs-blob-store#26 @niinpatel 🍊 P3+
ipfs-block-service ipfs/js-ipfs-block-service#85 @dirkmc 🍏 P3+
ipfs-http-response @hugomrdias 🍎 P1
ipfs-mfs ipfs/js-ipfs-mfs#49 @achingbrain 🍏 P3+
ipfs-multipart ipfs/js-ipfs-multipart#17 @achingbrain 🍊 P0
ipfs-repo ipfs/js-ipfs-repo#189 @zcstarr 🍏 P3+
ipfs-unixfs-exporter ipfs/js-ipfs-unixfs-exporter#21 @achingbrain 🍏 P3+
ipfs-unixfs-importer ipfs/js-ipfs-unixfs-importer#24 @achingbrain 🍏 P3+

Current progress:

  • 🍏 28/60 (46.6(6) %)
  • 🍊 15/60 (25%)
  • 🍎 14/60 (23.3(3)%)

Dependents

These modules use IPFS and fall under the ipfs/ipfs-shipyard umbrella so should also be updated.

Module PR Owner Status Priority
awesome-ipfs 🍎
benchmark-js.ipfs.io 🍎
cid-utils-website 🍎
demo-ipfs-todo 🍎
ipfs-companion @lidel 🍎
ipfs-desktop 🍎
ipfs-fuse @alanshaw 🍎
ipfs-geoip ipfs/ipfs-geoip#67 @nijynot 🍊 P0
ipfs-iiif-db 🍎
ipfs-level 🍎
ipfs-locations 🍎
ipfs-performance-profiling 🍎
ipfs-pubsub-peer-monitor 🍎
ipfs-pubsub-room 🍎
ipfs-pubsub-room-demo 🍎
ipfs-redux-bundle 🍎
ipfs-registry-mirror 🍎
ipfs-postmsg-proxy @alanshaw 🍎
ipfs-pubsub-1on1 🍎
ipfs-service-worker @vasco-santos 🍎
ipfs-share-files 🍎
ipfs-stats 🍎
ipfs-webui 🍎
ipfsd-ctl ipfs/js-ipfsd-ctl#353 @achingbrain 🍏
ipld-explorer 🍎
ipld-explorer-cli @alanshaw 🍎
ipld-explorer-components 🍎
ipscend 🍎
npm-on-ipfs @achingbrain 🍎
peer-crdt-textarea-binding 🍎
peer-flipchart 🍎
peer-pad-core 🍎
peer-star-app 🍎
peer-star-network-vis 🍎
peer-star-network-vis-react 🍎
peer-star-peer-color 🍎
peer-star-react 🍎
peerpad-peer-crdt 🍎
service-worker-gateway @vasco-santos 🍎
tevere 🍎
window.ipfs-fallback @alanshaw 🍎
y-ipfs-connector 🍎
@daviddias

This comment has been minimized.

Copy link
Member

commented Oct 26, 2018

This is a "Everyone is Welcome" effort and a excellent opportunity for everyone to get their feet wet on the codebase, improve documentation, testing and refresh the code.

Hoping that we can count with help from the IPFS GUI, IPFS in Web Browsers, Community WG and Dynamic Data & Capabilities. This will make our JS APIs better for all the modern/new JS developers!

@ipfs/javascript-team Unite 👋🏽⚡️

@dignifiedquire

This comment has been minimized.

Copy link
Member

commented Oct 26, 2018

this is awesome and makes me so happy to see 🎉

@hugomrdias

This comment has been minimized.

Copy link
Collaborator

commented Oct 26, 2018

add me to multihashing-async, ipfsd-ctl, ipfs-multipart

@daviddias

This comment has been minimized.

Copy link
Member

commented Oct 26, 2018

@hugomrdias you got it! :)

@alanshaw alanshaw referenced this issue Oct 27, 2018
2 of 2 tasks complete

alanshaw added a commit to alanshaw/js-libp2p-crypto-secp256k1 that referenced this issue Oct 28, 2018

feat: use async/await
This PR changes this module to remove callbacks and use async/await. The API is unchanged aside from the obvious removal of the `callback` parameter.

depends on `multihashing-async` PR TODO

refs ipfs/js-ipfs#1670

License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
@alanshaw alanshaw referenced this issue Oct 28, 2018
1 of 1 task complete
@vasco-santos

This comment has been minimized.

Copy link
Member

commented Oct 29, 2018

Updated the table with my assignements

@hacdias

This comment has been minimized.

Copy link
Member

commented Oct 30, 2018

@alanshaw I'm taking a stab at libp2p/js-peer-id#87 😄

Update: I added myself to peer-info too.

@hacdias hacdias referenced this issue Oct 30, 2018
1 of 1 task complete
@alanshaw

This comment has been minimized.

Copy link
Member Author

commented Oct 30, 2018

Thoughts and views on libp2p/interface-peer-discovery#2 please!

@hacdias

This comment has been minimized.

Copy link
Member

commented Oct 30, 2018

@alanshaw I was thinking at taking a look at ipld-* and I noticed interface-ipld-format is missing from the table. Despite not being code, it is also required to be updated. I'll take a look at that one, does it make sense?

@mikeal

This comment has been minimized.

Copy link
Member

commented Oct 30, 2018

RE: IPLD

We (@vmx and I) aren't too happy with interface-ipld-format and have been talking about a re-write for a some time now. Originally we had intended to use async functions as a test case "before advocating that all of js-ipfs moves to it" but you're beating us to it now :)

Perhaps we should accelerate the timeline for this refactor rather than trying to update the current interface?

@daviddias

This comment has been minimized.

Copy link
Member

commented Oct 31, 2018

@mikeal it would be great if you, @vmx and @hacdias could work together on that refactor/redesign. I is a great opportunity for @hacdias to learn the IPLD way and learn from you two :)

@hacdias

This comment has been minimized.

Copy link
Member

commented Oct 31, 2018

Yeah, that would be awesome. @mikeal @vmx as you can see by my previous comment, I already created a PR on interface-ipld-format. Maybe we could continue the discussion there!

@vmx vmx referenced this issue Oct 31, 2018
@mikeal

This comment has been minimized.

Copy link
Member

commented Oct 31, 2018

it would be great if you, @vmx and @hacdias could work together on that refactor/redesign

Sounds great, but I do want to make sure I'm prioritizing things in the right order (there's a lot on my plate from being out on vacation and now I'm sick). Is this refactor already blocking downstream refactors?

@vasco-santos vasco-santos referenced this issue May 31, 2019
0 of 3 tasks complete

vmx added a commit to dirkmc/js-ipfs-block-service that referenced this issue Jun 18, 2019

chore: callbacks -> async / await
This is part of the Awesome Endeavour: Async Iterators
ipfs/js-ipfs#1670

BREAKING CHANGE: All places in the API that used callbacks are now replaced with async/await

vmx added a commit to ipfs/js-ipfs-block-service that referenced this issue Jun 18, 2019

chore: callbacks -> async / await
This is part of the Awesome Endeavour: Async Iterators
ipfs/js-ipfs#1670

BREAKING CHANGE: All places in the API that used callbacks are now replaced with async/await
@vmx vmx referenced this issue Jun 18, 2019

achingbrain added a commit to ipfs/aegir that referenced this issue Jul 5, 2019

feat: support async hooks
A common use of hooks in aegir is to start and stop an ipfs node for
browser tests to run against.

Since they all use ipfsd-ctl and that is moving to async/await in
ipfs/js-ipfsd-ctl#353 as part of ipfs/js-ipfs#1670, it would be
nice to not have to mix async and callbacks in the hooks.

This PR adds support for async hooks while not breaking existing
callback based ones.

hugomrdias added a commit to ipfs/aegir that referenced this issue Jul 5, 2019

feat: support async hooks (#390)
A common use of hooks in aegir is to start and stop an ipfs node for
browser tests to run against.

Since they all use ipfsd-ctl and that is moving to async/await in
ipfs/js-ipfsd-ctl#353 as part of ipfs/js-ipfs#1670, it would be
nice to not have to mix async and callbacks in the hooks.

This PR adds support for async hooks while not breaking existing
callback based ones.

achingbrain added a commit to libp2p/js-libp2p-crypto-secp256k1 that referenced this issue Jul 5, 2019

feat: use async/await
This PR changes this module to remove callbacks and use async/await. The API is unchanged aside from the obvious removal of the `callback` parameter.

depends on `multihashing-async` PR TODO

refs ipfs/js-ipfs#1670

License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>

achingbrain added a commit to libp2p/js-libp2p-crypto-secp256k1 that referenced this issue Jul 10, 2019

feat: use async/await
This PR changes this module to remove callbacks and use async/await. The API is unchanged aside from the obvious removal of the `callback` parameter.

depends on `multihashing-async` PR TODO

refs ipfs/js-ipfs#1670

License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>

jacobheun added a commit to libp2p/js-libp2p-crypto-secp256k1 that referenced this issue Jul 10, 2019

feat: use async await (#18)
BREAKING CHANGE: Callback support has been dropped in favor of async/await.

* feat: use async/await

This PR changes this module to remove callbacks and use async/await. The API is unchanged aside from the obvious removal of the `callback` parameter.

refs ipfs/js-ipfs#1670

* fix: use latest multihashing-async as it is all promises now
@alanshaw alanshaw referenced this issue Jul 17, 2019
52 of 54 tasks complete
@daviddias

This comment has been minimized.

Copy link
Member

commented Jul 20, 2019

Hi y'all 👋🏽 AFAIU, this transition would something that would make everyone more productive and happy. Is there anything we can do to accelerate it? For example, could we have one week in which the team does nothing else other than shipping this refactor?

@alanshaw

This comment has been minimized.

Copy link
Member Author

commented Jul 23, 2019

Yes, that would push it along a fair bit I reckon! It's taking so long because it's not top of anyone's priority list.

@dirkmc

This comment has been minimized.

Copy link
Contributor

commented Jul 23, 2019

Totally agree 👍

When I started making async/await PRs I took advantage to fix bugs and make improvements to the code, but actually I think it just makes it more confusing for reviewers and slows the process down.

I noticed that @achingbrain followed a strategy of purely changing from callbacks to async/await. He would open issues for any bugs or improvements so they could be worked on separately, I think that's the best approach.

@alanshaw alanshaw referenced this issue Jul 25, 2019
0 of 1 task complete
@jacobheun jacobheun referenced this issue Jul 29, 2019
19 of 19 tasks complete

alanshaw added a commit that referenced this issue Aug 2, 2019

docs: update examples (#2319)
Since I was going through these anyway to ensure they work with [0.37](#2192) I've updated the examples to use the new [`IPFS.create` constructor](https://github.com/ipfs/js-ipfs#ipfs-constructor) and switched to using the promised API so that we onboard new users to using promises and minimise the disruption caused when #1670 bubbles up to here.

License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>

@achingbrain achingbrain self-assigned this Aug 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.