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

Storing closures inside the state and async side effects #40

Open
frankiesardo opened this issue Jul 30, 2020 · 3 comments
Open

Storing closures inside the state and async side effects #40

frankiesardo opened this issue Jul 30, 2020 · 3 comments

Comments

@frankiesardo
Copy link

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 frankiesardo changed the title Storing closures inside the state and multiple side effects Storing closures inside the state and async side effects Jul 30, 2020
@jamesplease
Copy link
Collaborator

jamesplease commented Jul 30, 2020

👋 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.

@conorhastings
Copy link
Owner

conorhastings commented Jul 30, 2020

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 😜

@frankiesardo
Copy link
Author

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 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants