mattkrick / redux-optimistic-ui

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

Add support for the Saga pattern by enabling multiple actions in a transaction #11

Open vjancik opened 8 years ago

vjancik commented 8 years ago

NOT FINISHED - DISCUSSION

I want to use this library in combination with Redux Saga. And with Redux Saga, you chain actions to create more complicated UI modifications. These chained actions can be synchronous or asynchronous.

And I want some of these actions to be reversible, and optimistic. To do that I need to be able to revert multiple actions that are part of the same transaction.

With my changes, this is possible. All actions with the same meta.optimistic.id get filtered out of the history on a revert.

This should be useful for all kinds of asynchronous action libraries where you can chain actions. E.g. redux-saga, redux-thunk, redux-observable

And of course, all functionality before these changes stays unaffected.

mattkrick commented 8 years ago

Why not give each action its own id? Then just revert the action instead of the entire transaction. Assigning an id on a per-transaction basis makes for an all-or-nothing approach, right? If you have a test case in mind that'd probably help me understand the problem better.

vjancik commented 8 years ago

Because then I have to keep track of all the IDs of all the actions in that particular transaction. And revert them all one by one.

Think of it like a DB transaction. You have a sequence of DB queries, in a transaction, and if any one of them should fail, all should fail. And you roll back the changes.

The equivalent of what you're proposing would be to roll back each of the queries on by one.

My particular use case is that I am composing my actions from smaller actions, to minimize code repetition inside the reducers. So e.g. REQUEST_DATA, other than just fetching the data, also triggers SET_LOADING, and then CLEAR_LOADING when it finishes, then as a result of the data pull, I call another action action which loads the data that this action fetched. Now if I want to optimistically set some variable before I confirm it with the server (with a subsequent REQUEST_DATA) I also have to rollback SET_LOADING, if it fails, or else my loading counter will be off.

I could make SET_LOADING reducer logic part of the REQUEST_DATA, but I use it in other places, so I'd prefer to compose them rather than duplicate them (the SET_LOADING reducer logic).

mattkrick commented 8 years ago

With that pattern, won't you have an entity integrity problem? As i understand it, you have a single loading boolean somewhere in your state that is turned true when something requests data & false when that data comes back. That means you can't make 2 requests for data. Alternatively, I'd suggest that loading is a computed property. Your loading flag is true when getState().history.size > 0.

If you have multiple loading flags in your state, then each will be managed by its own reducer & this isn't a problem.

PS, if you're doing async work with your own server, I'd suggest you try cashay, which is like relay, but for redux: https://github.com/mattkrick/cashay. it makes life MUCH easier.

vjancik commented 8 years ago

No I have a loading counter, 0 <> +Inf, and a global Loading indicator, which is indicating something is loading if the counter is positive, then I have a per-entity loading boolean, to indicate if that particular entity is currently being loaded.

Thank you for the suggestions! I'll check out cashay.

So what do you think of these changes? I think they are a nice inexpensive improvement to the current functionality.

mattkrick commented 8 years ago

yeah, it looks minimally invasive & i don't see a huge performance hit to include the changes. I'm not sure I understand it 100%, but if you write up a couple unit tests, I can dig in a little more. In the meantime I'll get a TravisCI on this repo

clayne11 commented 7 years ago

This sounds awesome. I would love to have multiple actions in a transaction.

vjancik commented 7 years ago

@clayne11 It's not that much code. If you'd like to take over, please go ahead! Right now I unfortunately don't have time to finish this work :(