Questions on the proper use and handling of done callbacks

[oderby]

Preface: trying to use choo on a project, and I'm somewhat confused, on several levels, about the proper usage of the done callback in effects and how it relates to the callback provided in the send method.

Here are my particular questions and areas where I'd appreciate some clarifications:

1. In an effect handler, I understand the need to pass the done cb to subsequent send calls, to maintain the chain and allow errors to propagate upwards. But how should I handle the cb if I want to send 2+ actions from a single effect (e.g. I make an API call which returns data applicable to two separate models)? The following seems incorrect, but I'm not sure what the correct behavior should be:

myeffect: (state, data, send, done) => {
    ... make call to server ...
    send('foo:update', newFoo, done);
    send('bar:update', newBar, done);
}

2. Aside from effects sending actions to reducers, we often send actions from views. Presumably the send provided to views is the same send provided to effects. So that implies that one could provide their own callback to be called once the sent action has completed. My question is if there's is any common/encouraged use for this? I could imagine using it to update the html/UI if an error was encountered (which seems generally useful). But I could also imagine that such usage could present a race with the view being retriggered (because the state changed) and all sorts of unpredictable/undesired behavior occurring...

3. Are there any examples of applications making use of the done callback in a non-trivial manner? It would be great to have something to reference to, and none of the examples in this repository seem to make use of it (there are cases of passing it an error, but as far as I can tell nothing acts on that error).

Thanks!

[jakeburden]

Hey @oderby 👋

Typically what I do when I need to call subsequent send functions is something like this:

myeffect: (state, data, send, done) => {
  send('foo:update', newFoo, function () {
     send('bar:update', newBar, done)
  })
}

I also really like this idea in github-feed where @timwis uses run-series like this:

checkCookie: (send, done) => {
  const token = Cookies.get('token')
  if (token) {
    series([
      (cb) => send('receiveToken', token, cb),
      (cb) => send('fetchUser', cb)
    ], done)
  }
}

I hope this helps 🌴

[oderby]

Yeah, that answers my first question. Thanks!

Still curious about 2 and 3 though...

[aknuds1]

1. @jekrb is right, in that you need to nest callbacks and finish by calling done once everything is asynchronously completed.
2. I don't think the send passed to views or to effects is the same as the one passed to effects. The latter type expects a callback to invoke when it finishes, since effects are asynchronous whereas views and reducers are not. I'm pretty sure this is correct at least, if not I hope e.g. @yoshuawuyts will chime in and set me straight :)
3. What do you hope to learn from examples of non-trivial use of the done callback? It's the same pattern as for Node callbacks. As @jekrb explained already, you will need to nest callbacks until there are no more asynchronous operations pending, at which time you can call done to signal that your effect is finished. Do you mean examples of acting upon errors passed to done??

[oderby]

Oh, yeah. Question 3 assumed that the send used in views took a callback, so they could handle errors. So I was curious if there were canonical examples of why that would be useful and how that'd be used. It seems like it could be something thats better handled by some error-tracking state instead. But if it is as you say and the send in views doesn't take a callback, then I'm curious where in application you could place such error handling logic? I guess you could put it at each error- producing site, but then that seems to defeat the purpose of passing the error back via the callback. On Feb 19, 2017 2:46 PM, "Arve Knudsen" <notifications@github.com> wrote: 1. @jekrb <https://github.com/jekrb> is right, in that you need to nest callbacks and finish by calling done once everything is asynchronously completed. 2. I don't think the send passed to views or to effects is the same as the one passed to effects. The latter type expects a callback to invoke when it finishes, since effects are asynchronous whereas views and reducers are not. I'm pretty sure this is correct at least, if not I hope e.g. @yoshuawuyts <https://github.com/yoshuawuyts> will chime in and set me straight :) 3. What would you learn from examples of non-trivial use of the done callback? It's the same pattern as for Node callbacks. As @jekrb <https://github.com/jekrb> explained, you will need already to nest callbacks until there are no more asynchronous operations pending, at which time you can call done to signal that your effect is finished. Do you mean examples of acting upon errors passed to done?? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#417 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABSd9t7JxPxJDl6JXM0iZtjSNlKZirY_ks5reMZUgaJpZM4MEvZJ> .

[aknuds1]

In synchronous functions (e.g. views) you don't need any special mechanism for signaling errors; you can either throw exceptions or use whatever other method.

On the other hand, thinking it over again, I'm actually not sure how a send in a view/reducer will communicate an error back. I guess maybe you should set a state property.

Edited due to initial typos, sorry.

[jorgebucaran]

@aknuds1 @jekrb Hi! 👋

Question: Why do you need to call done even if there is no error? From the docs:

When an effect or subscription is done executing, or encounters an error, it should call the final done(err, res) callback.

[aknuds1]

@jbucaran As you see from the documentation, you should call done when an effect or subscription has finished executing, if it encounters an error or not. Since effects and subscriptions are asynchronous, you have to call done signal that they have finished.

Iff there is an error pass it to done, otherwise pass null for the error.

[jorgebucaran]

@aknuds1 Yup, yup. I can see calling it makes sense if there is an error; what I can't wrap my head around is calling it when there is none.

My reasoning is: effects don't cause the view to re-render, so go fetch some data from some server and when you have the data just trigger a reducer, but if no further actions will be sent, then why call done?

[aknuds1]

@jbucaran It can be difficult to say exactly why you need to call done technically since it depends on Choo's implementation at any given time, but I think so long as the documentation states that you have to do it just go with it.

Also, you might want to call something else after an effect has finished, for example another effect, and this won't happen unless your first effect calls done. Bottom line, so long as an API is designed a certain way, it's best to follow said design rather than making assumptions about what you can and cannot do, because it can end up biting you or the programmer maintaining your code.

[jorgebucaran]

Also, you might want to call something else after an effect has finished, for example another effect, and this won't happen unless your first effect calls done.

Absolutely. After some async task, update the view, etc. That's what most effects will be doing anyway.

Now, obviously, there can be effects that don't need to call further actions (at least the reducer kind). I can't think of a convincing example off the top of my head, but that's a situation in which calling done feels quirky.

So long as the documentation states that you have to do it just go with it.

Haha good one! 👋

[aknuds1]

I don't think signalling that an async function has finished is ever quirky. It's simply less error prone is it not? :) It's better not assuming you won't need to know when an async function has finished, since as you know code changes all the time.

[jorgebucaran]

It's simply less error prone is it not?

🤔 I've been known to be wrong, but I'm not so sure about this one this time.

Allow me to regress. Inside an effect, inside your async task you know when things are done or not, which is to say, you totally know when you'd need to call further actions if you wish to update the model (and re-render the view).

Since choo's effects don't cause the view to be rerendered, I see no advantage for them to know when they are "done", unless the framework had some out of the box glue that let you chain effects or something like that (but it doesn't).

[aknuds1]

@jbucaran Your effect shouldn't know if it's being called by another asynchronous function though, should it? The caller would depend on knowing when the effect finishes.

[jorgebucaran]

@aknuds1 Gotcha! I've been totally under the impression folks were using promises or async functions for coordinating this kind of stuff.

[aknuds1]

@jbucaran Wondering if I'm missing something. Wouldn't you need for effect 1 to call done in order to be able to call effect 2 once it finishes?

[jorgebucaran]

@aknuds1 If your effect returns a promise or is an async function (which does it under the hood), then no. Otherwise, you need a callback mechanism like done, of course.

[aknuds1]

@jbucaran But you call the effect via send which does not return a promise (and does not expect a promise), but rather communicates via a done callback?

I mean, I use promises too, but they wrap send.

[jorgebucaran]

Ah, you are totally right! 🙇

I was betrayed by my own bias. In a library I made, actions are "wrapped" so that effects return whatever value the associated function returns. So, I can chain promises or use async functions.

It still bugged me that choo required a done callback; I thought am I doing something wrong? But after:

@aknuds1 But you call the effect via send which does not return a Promise, but rather communicates via a done callback?

It all makes sense now.

[aknuds1]

Yeah, it's Node style, not based around promises :)

[jorgebucaran]

Inside joke? Promise support has been in Node.js since 0.12.18 and it's pretty decent now. async/await just landed as well.

[aknuds1]

Well, classical Node style then.

[yoshuawuyts]

Closing because choo@5 will introduce a new API. See #425 for the merged PR. Thanks!