redux-undo icon indicating copy to clipboard operation
redux-undo copied to clipboard

Undo/redo side-effects

Open davidroeca opened this issue 7 years ago • 9 comments

As mentioned in https://github.com/omnidan/redux-undo/issues/147 users could benefit from use cases supported in https://github.com/powtoon/redux-undo-redo

An example: An action happens, and an api call is made that creates a resource. That action gets undone, and the user wants to take reverse-action on the api call to delete the resource. This functionality is currently not supported.

This functionality could add significant complexity to the library, but if it can be encapsulated/made simple, I'd argue that the library would benefit from it.

This could be implemented with an optional config outlining specific action types. It may also require additional state to be saved alongside past, present, and future, such as all the actions with documented side-effects that can be undone. So maybe in sync with past and future, there may be an actionPast and actionFuture with all documented actions with side effects that took place between states.

Just an idea, so I'd be open to other implementation suggestions as well.

davidroeca avatar Mar 16 '17 15:03 davidroeca

Hey I was wondering on the state of this enhancement? Or if the UNDO action can be made to return the state?

tsu-la avatar Aug 21 '17 14:08 tsu-la

Hi @tsu-la, I made a first crack at this and didn't like it, but the library has matured a little bit since then.

The challenge is that the design here sort of runs counter to the original design of redux-undo, since for the most part it computes once and saves intermediate states. This enhancement would compute forward and backward and not necessarily rely on intermediate states.

I guess we could encapsulate the complexity in a top-level key such as actionHistory.

I also thought about writing some middleware to handle this, which makes more intuitive sense to me, but may be overkill

davidroeca avatar Aug 21 '17 15:08 davidroeca

To me this feature is make-or-break.

Has there been any development here? Further thoughts?

jeremyblalock avatar Mar 24 '18 00:03 jeremyblalock

This feature would definitely be a huge improvement, the use cases are not just server calls but basically all possible side effects managed by middle-ware. An example of this is changing state in the web audio API. The web audio API updates are done in middle-ware and in order to undo them you need a reverse action when undo is called.

Idicious avatar May 22 '18 10:05 Idicious

There's no need to support this use case in the library. If you need to do an API call on UNDO/REDO you can simplify write a middleware that do something for those action. You have the state so you can compare present with past/future and do the appropriate call. Do I misunderstand the problem?

ramiel avatar Feb 24 '19 22:02 ramiel

@ramiel The usecase is when you have side effects on specific actions. These are generally put into middlewares. With only the undo / redo actions how do you know which middlewares to run on an undo? Running them all isn't an option as you have no idea what action is being undone / redone and you almost always need the payload from the action.

Idicious avatar Feb 25 '19 06:02 Idicious

That's definitely true.

ramiel avatar Feb 25 '19 10:02 ramiel

I've copied the code and added this instead of grouping etc in reducer.js.

    if (action && action.meta && action.meta.isSideEffectOfUserAction) {
          const groupedState = newHistory(history.past, res, history.future)
          debug.log('groupBy grouped the action with the previous action')
          debug.end(groupedState)
          return groupedState
        }

        // If the action wasn't filtered or grouped, insert normally
        history = insert(history, res, config.limit)

RichardLindhout avatar Mar 18 '20 16:03 RichardLindhout

Could we use a higher order reducer to handle the side-effect?

withRedoUndoSideEffect(undoable(orignalReducer))

At the same time we could maintain a side effect stack to provide context the side effect

{
  past: [...pastStatesHere...],
  present: {...currentStateHere...},
  future: [...futureStatesHere...],
  pastSideEffects:[],
  futureSideEffects: []
}

tobeczm avatar Oct 15 '21 09:10 tobeczm