dogriffiths/ReactCookbook-source

undo.js code

Ijerry-cloud opened this issue · 1 comments

The 'redo' case of the undo.js code doesn't properly handle the redo action. It applies the most recent action to a current state (it was that action that got it to the current state). A suggested fix (code attached) saves the former action as a property in the object of the previous state when the 'undo' case is triggered. Therefore a subsequent redo action would cause the former action to be applied to the former state.

import lodash from 'lodash'

const undo = (reducer) => (state, action) => {
let {
undoHistory = [],
undoActions = [],
...innerState
} = lodash.cloneDeep(state)
switch (action.type) {
case 'undo': {
if (undoActions.length > 0) {
innerState = undoHistory.pop()
innerState.curr_action = undoActions.pop() //stores the action to initiate on redo
}
break
}

case 'redo': {
if (undoActions.length > 0 && innerState.curr_action) { //check wether there was an undo prior to the redo
undoHistory = [...undoHistory, { ...innerState }]
undoActions = [...undoActions, innerState.curr_action]
innerState = reducer(
innerState,
innerState.curr_action
)
innerState.curr_action = null //disables redo
}
break
}

default: {
undoHistory = [...undoHistory, { ...innerState }]
undoActions = [...undoActions, action]
innerState = reducer(innerState, action)
innerState.curr_action = null //disables redo
}
}
return { ...innerState, undoHistory, undoActions }
}

export default undo

Thanks for reporting this. Would it be possible to create a failing test case showing the expected behavior that I could add to the test script?

https://github.com/dogriffiths/ReactCookbook-source/blob/master/ch03-02-undo/src/undo.test.js