use-reducer-with-side-effects icon indicating copy to clipboard operation
use-reducer-with-side-effects copied to clipboard

Storing closures inside the state and async side effects

Open frankiesardo opened this issue 4 years ago • 3 comments

Hello @conorhastings

Thanks for this library. Unfortunately I discovered it too late as I finished working on my spin on the same topic: https://github.com/frankiesardo/use-reducer-with-effects but I'm glad to see other people share a similar philosophy. The library I authored differ slightly in the sense that the side effects are represented as data instead of closures but I think they're conceptually very similar otherwise. I'd be happy to hear your thoughts!

I have two questions regarding your implementation:

  1. Your sideEffect is stored in the state object as a function / closure. Does this have any implications re: equality checks and serialisability of the state object?
  2. I execute the sideEffects as soon as they are returned by the reducer: https://github.com/frankiesardo/use-reducer-with-effects/blob/master/src/index.js#L6-L15 do you think that's wrong? I see you have them in an async block. I also don't bother keeping a queue of side effects since as soon as a side effect is returned by the reducer, the useEffect is triggered and the side effect is removed from the state object. Do you think that can cause problems?

In any case, thanks for taking the time and BTW your README is extremely clear and well put together, well done for that 👍

frankiesardo avatar Jul 30 '20 06:07 frankiesardo

:wave: yo @frankiesardo ! I'm glad others have independently discovered the wonders of the useReducerWithSideEffects pattern. This lib has made some of the most complex code that I need to write so much simpler. I checked out your lib: it looks really nice!

I know you asked for @conorhastings ' thoughts. I'd love to hear from him too; I'm a heavy user of this lib so I hope you don't mind if I share my thoughts 😅

  1. Does this have any implications re: equality checks and serialisability of the state object?

Do you mean equality checks internally (within the lib) or externally (from the perspective of the user?). It's been awhile since I've dug into the implementation, but the last time I checked the lib is working as expected without any strange equality check bugs.

For consumers, the "full" state object that contains both the side effects and the "regular state" is not a public API (users only receive the "state" part of things), so they never receive the object storing the array of side effects. This avoids the issue of users needing to worry about the side effects that may be stored. In other words, the same caveats that apply when using a regular useReducer are the same things you must worry about when using this lib.

On serializability: side effects within the state only last for a brief moment. The very moment after they are introduced, they are removed (by being executed and removed from the state). They may have their own internal state, but that's outside of the scope of what this library concerns itself with. So side effects are not serialized, and it would also be quite tricky to serialize them in a way that made sense for a general use case. The way I think about this library is that the same serializability concerns you might have when using regular useReducer apply here, too.

I execute the sideEffects as soon as they are returned by the reducer...do you think that's wrong?

tbqh I never understood the async part of this lib. My quick guess is that making it async or not async would have the exact same result. This lib doesn't really seem to be utilizing the async-ness of the execution of the side effects in any way that I can perceive, but it's possible I'm missing something. Maybe @conorhastings can comment more on this?

2. I also don't bother keeping a queue of side effects since as soon as a side effect is returned by the reducer, the useEffect is triggered and the side effect is removed from the state object. Do you think that can cause problems?

imo as long as the library's side effect execution behavior is deterministic + documented then it's OK. I'd need to think more about if the order of execution could differ in your lib vs this lib, though.

jamesplease avatar Jul 30 '20 16:07 jamesplease

In terms of async, which specific part are you speaking about? The basic reason is due to the fact that side effects can be asynchronous using await allows the side effects to run in a guaranteeed correct order

I think james covered this rest, he's put a lot of work into the lib 😜

conorhastings avatar Jul 30 '20 16:07 conorhastings

Hey @jamesplease @conorhastings thank you for taking the time to review my lib and discuss your implementation, you mentioned some very helpful stuff.

For consumers, the "full" state object that contains both the side effects and the "regular state" is not a public API (users only receive the "state" part of things), so they never receive the object storing the array of side effects.

On serializability: side effects within the state only last for a brief moment. The very moment after they are introduced, they are removed (by being executed and removed from the state)

Both this points are very clear and I see now how that works now. I was initially put off by the idea of storing functions inside a reducer but yours it's a well reasoned case.

tbqh I never understood the async part of this lib. My quick guess is that making it async or not async would have the exact same result.

The basic reason is due to the fact that side effects can be asynchronous using await allows the side effects to run in a guaranteed correct order

In my implementation I fire off all the effects saved in memory at the same time, since they should all be async. This is probably something I should document so 👍 for pointing that out.

I'm going to subscribe for updates for this library as I'd love to be kept in the loop if a deeper understanding of Hooks changes the way we implemented our solutions, but for now keep up the good work 👏

frankiesardo avatar Jul 31 '20 08:07 frankiesardo