Js-ipfs: Remove ready event from basic usage

Created on 7 Dec 2018  ·  7Comments  ·  Source: ipfs/js-ipfs

Now that we've started going down the "promises everywhere" rabbit hole it should be possible to remove waiting for the ready event from the default usage.

There's a few different ways we could go about this. Off the top of my head I think it would be nice to have a property on the object that is a promise that is resolved when ready.

let ipfs = new IPFS()
await ipfs.ready

And we could create a convenience method called create() that did this automatically.

let ipfs = await IPFS.create()

Thoughts?

P3 dieasy help wanted statuready

All 7 comments

I'd personally love to see this happen! 👍I think adding/changing what is suggested here would improve the usability and readability a ton (=2 loc).

We had a discussion about it in the past in https://github.com/ipfs/js-ipfs/issues/1083 (see the different proposal for the API), but as you said @mikeal, things have changed now with using promises everywhere. Thanks for bringing it up again! ❤️

@mikeal As a frequent user of js-ipfs I also think something like this is really necessary — thanks for bringing this up!

I would like, though, to make a distinction between creating and starting, and would like to make .start() a method.
(The creation can only be called once, but starting would be idempotent.)

Because in some user-land packages we can be using IPFS node instances that we haven't created ourselves, we don't really know the start state, and simply doing something like this to ensure the node is started would help a lot on reducing the complexity of some processes:

// a) starts or,
// b) if starting, returns when started or
// c) if started, returns immediately
await ipfs.start() 

Also, I think we should decide between calling the state started versus ready. I'm fine with either, but await ipfs.start() is more intuituitive to me than await ipfs.ready().

Also, I think const ipfs = await IPFS.create(options) should, by default, start the node (unless options.start is false).

Nonetheless, the user could still ensure it's started by doing await start().

@pgte I ran into this just the other day calling await ipfs.start() when it was already started seeing the exception :)

I agree that it's confusing to have these two states, ready and started. My main question is, why is the ready state necessary? I understand that there are async things we want to do upon creation but why don't we just begin all of the API methods with await ipfs.ready and hide this state change from the user and effectively just queue up any method calls that are waiting for the ready state?

@mikeal that would be very helpful, (much alike a lot of node.js database clients where it queues operations or queries while not started).

One potential problem with this is that multiple operations or queries could fail if starting fails. But I guess users could always wait for started before starting any operation or query...

much alike a lot of node.js database clients where it queues operations or queries while not started

I don't think I would have had this idea had I not written this so many times in the level ecosystem :)

But I guess users could always wait for started before starting any operation or query...

If .start() has been called then we should wait for it to finish, if it hasn't been called we should throw if the operation can't complete without the node being started.

Looking through the code on this and we'll need to do this in stages.

  1. PR for a ready promise instantiated in the main constructor.
  2. Update API's to wait for ready promise
    2.1. Update depenedencies that implement top level APIs, like mfs.
    2.2. Update internal APIs.

For step 2, we'll want to have a discussion about whether or not we want to do this before the big async function refactor or after.

I am happy to look at this over the holidays! :)

Was this page helpful?
0 / 5 - 0 ratings