setState updater function is impure ?

[littlee]

https://github.com/mweststrate/immer-gifts/blob/3a828879662992ef9a9abca239df2d28fd9b3b24/src/index.js#L34-L41

I notice that in the code example above, the updater function passed to setState is impure, and when rendering app inside <React.StrictMode /> during development, react will run the updater twice https://github.com/facebook/react/issues/12856#issuecomment-390206425

so can we implement undo feature in a better pattern?

[shimikano]

I ran into the same issue when playing around with the egghead tutorial (which is great BTW). The patches are sent to the server twice and the inversePatches are pushed onto the undoStack twice.

I tried to move these side-effects outside the setState callback, something like this:

 const dispatch = useCallback((action, undoable = true) => { 
   const [nextState, patches, inversePatches] = patchGeneratingGiftsReducer(state, action);
   
   send(patches) // always send patches 
   if (undoable) undoStack.current.push(inversePatches) // store patches if this is undoable 
     
   setState(nextState); 
 }, [state]) // additional dependency 

This solves the double side-effect issue. However, it adds another dependency to the dispatch memoization (namely, state), which in turn adds a dependency to the handleReserve callback, which in turn causes all items to be re-rendered again even if only one item is reserved.

I was wondering whether you found a solution, @littlee.

[littlee]

@shimikano I eventually find my own solution, it may be a little complicated, hope you can understand the code

import { useReducer } from 'react';
import produce, {
  applyPatches,
  enablePatches,
  produceWithPatches
} from 'immer';

enablePatches();

const ignoreActions = ['UNDO', 'REDO'];

function isIgnoreAction(action) {
  return ignoreActions.indexOf(action.type) !== -1;
}

function patchUndoRedo(state, action, patches, inversePatches) {
  return produce(state, (draft) => {
    if (draft.undoPointer > 0) {
      draft.undoStack.splice(draft.undoStack.length - draft.undoPointer);
      draft.undoPointer = 0;
    }
    if (patches.length || inversePatches.length) {
      draft.undoStack.push({
        type: action.type,
        patches,
        inversePatches
      });
    }
  });
}

function editorReducer(state, action) {
  if (!isIgnoreAction(action)) {
    const [nextState, patches, inversePatches] = produceWithPatches(
      state,
      (state) => {
        switch (action.type) {
          case 'CLEAR_HISTORY': {
            state.undoPointer = 0;
            state.undoStack = [];
            break;
          }

          case 'CHANGE_SOMETHING': {
            state.something++;
            break;
          }
          default:
        }
      }
    );
    return patchUndoRedo(nextState, action, patches, inversePatches);
  } else {
    switch (action.type) {
      case 'UNDO': {
        const stackIndex = state.undoStack.length - 1 - state.undoPointer;
        const canUndo = state.undoStack[stackIndex];
        if (canUndo) {
          const nextState = applyPatches(
            state,
            state.undoStack[stackIndex].inversePatches
          );
          return {
            ...nextState,
            undoPointer: state.undoPointer + 1
          };
        }
        return state;
      }
      case 'REDO': {
        const stackIndex = state.undoStack.length - 1 - (state.undoPointer - 1);
        const canRedo = state.undoStack[stackIndex];
        if (canRedo) {
          const nextState = applyPatches(
            state,
            state.undoStack[stackIndex].patches
          );
          return {
            ...nextState,
            undoPointer: state.undoPointer - 1
          };
        }
        return state;
      }
      default:
        return state;
    }
  }
}

const App = (props) => {
  const [state, dispatch] = useReducer(editorReducer, {
    something: 1,
    undoPointer: 0,
    undoStack: []
  });
  return (
    <div>
      <button
        onClick={() => {
          dispatch({
            type: 'CHANGE_SOMETHING'
          });
        }}
      >
        change
      </button>
      <button
        onClick={() => {
          dispatch({
            type: 'UNDO'
          });
        }}
      >
        undo
      </button>
      <button
        onClick={() => {
          dispatch({
            type: 'REDO'
          });
        }}
      >
        redo
      </button>
      <pre>{JSON.stringify(state, null, 2)}</pre>
    </div>
  );
};

[shimikano]

@littlee Thank you for sharing your code 👍 Basically, the main difference is the fact that the undo stack is part of the state.

For what it's worth, in order to stay close to the original code, I tried avoiding the additional state dependency to the callback in my previous comment by accessing it through a ref as described in the React hook FAQ.

[littlee]

@shimikano I store the undo stack in the state, just because I want to display it to the users, like a history panel in Photoshop, if your boss didn't ask you to implement something like this, store the undo stack in a ref is also good enough.