mattkrick / redux-optimistic-ui

a reducer enhancer to enable type-agnostic optimistic updates
MIT License
693 stars 36 forks source link

Handling offline #41

Open WillSquire opened 6 years ago

WillSquire commented 6 years ago

Reading through the documentation I can't work out if this has a pooling mechanism for requests and executes in roder or not. I.e. if the connection goes offline, does it wait and retry in the request order or simply fail?

mattkrick commented 6 years ago

good thinkin!

this is just a dumb client cache. Handling offline stuff should be handled in your data transport layer. if you mix the 2 you get all kinds of sadness down the road.

BenBach commented 6 years ago

We are using a separate request saga with an offline queue for our requests. This works great together. However, we run into problems when the user is offline for longer than a few minutes.

What happens is that a lot of optimistic BEGIN actions are fired and new requests actions are queued in the offline queue. Then we get the following errors message:

@@optimist: Possible memory leak detected.
                  Verify all actions result in a commit or revert and
                  don't use optimistic-UI for long-running server fetches

Is there a good way to handle these kinds of scenarios.

Thank you very much in advance. :-)

mattkrick commented 6 years ago

you can remove the error by doing something like optimistic(reducer, {maxHistory: Infinity}), but that doesn't fix the root cause.

making over 100 actions while offline seems like a lot. are you certain they are all dependents of the return value? For example, let's say you use redux for your remote fetches as well as a button hover state. the button hover state is not dependent on the return value of the fetch. you should wrap your fetch reducer with this & leave the button reducer as a vanilla reducer.

WillSquire commented 6 years ago

Thanks for getting back @mattkrick. Not sure about the data transport layer part, could you contextualise please?

With @BenBach 's case I believe 100 actions is possible without their being a memory leak. I know in my scenario it certainly was (product management system). To try to mitigate it though I debounced network requests to 'batch' the changes. In general I found only the U in CRUD gave me pain. My updates were partial/PATCH (i.e. only rewrote changed fields), but if it were a complete overwrite/PUT then it would be easier. Also I didn't use this library in the end, I used redux-offline instead which might be why there is pain around partial updates as it doesn't really store the history, only the actions. So reverting to previous action values in a chain of actions doesn't work when the chain breaks... it needs 'network' data to be seperate from 'local' changes. No libraries do this well enough for me yet.

mattkrick commented 6 years ago

Agreed! No one does it well. I had to PR relay to make it work.

FWIW, Here's the pattern I use to manage local/optimistic/domain state: https://github.com/facebook/relay/issues/2481#issuecomment-403649559

WillSquire commented 6 years ago

@mattkrick Yes, that's the way to go about it. Personally I think if that 'network' data and 'local' data were kept separate and the history were encapsulated in both, then it would be a nice improvement to this library (at least in my eyes). Using your already implemented ensureState, this can be what merges the too (will need deep comparisons though).

BenBach commented 6 years ago

Thanks for all the answers. @mattkrick I have the problem right now that even though I wrapped just one sub reducer with the optimistic reducer enhancer it also add other actions to the history which are from totally different parts of the app and not related to this specific action. Is this intended?

Any help highly appreciated :-)

mattkrick commented 6 years ago

@WillSquire i used to do that (local state in redux, domain state in relay) but it's a huge pain. you end up with no single source of truth. sometimes a mutation needs to also mutate the local state (eg animations after a value is returned).

@BenBach that doesn't sound right, check your middleware