JannicBeck / undox

⎌ Redux Implementation of Undo/Redo based on storing actions instead of states.
MIT License
24 stars 5 forks source link

feature request: limit, filter, preserve #31

Open Buggytheclown opened 3 years ago

Buggytheclown commented 3 years ago

First of all, thanks for your work, it helped us a lot. We start using undox in prod.

Features I am missing: 1) limit - some reducers are tricky and take a little time to execute. Undo takes almost 60ms for a 200-action history. The user can perform 20 actions per minute. 2) filter - I don't want to include some actions (for example, open / close a menu) in the history 3) preserve - for example FETCH_SUCCESS action must be in history to replay the history correctly. But I don't want to let the user undo the FETCH_SUCCESS action.

As a work around i wrap the undox reducer. Let me know if you open to Pull requests

import { UndoxTypes, Action, Reducer, UndoxState } from 'undox';
import _flow from 'lodash/flow';

function getHistoryAction<T, S extends UndoxState<T, A>, A extends Action>({
  state,
  offset,
}: {
  state: S;
  offset: number;
}) {
  return state.history[state.index + offset];
}

type PreserveConfig = { preserve?: (action: Action | Action[]) => boolean };
const undoxEnhancerPreserve = <T, S extends UndoxState<T, A>, A extends Action>({
  preserve = () => false,
}: PreserveConfig) => (reducer: Reducer<S, A>) => (state: S, action: A) => {
  if (
    state &&
    action.type === UndoxTypes.UNDO &&
    preserve(getHistoryAction({ state, offset: 0 }))
  ) {
    return state;
  }
  return reducer(state, action);
};

type FilterConfig = { filter?: (action: Action) => boolean };
const undoxEnhancerFilter = <T, S extends UndoxState<T, A>, A extends Action>({
  filter = () => true,
}: FilterConfig) => (reducer: Reducer<S, A>) => (state: S, action: A) => {
  if (!state || Object.values(UndoxTypes).includes(action.type) || filter(action)) {
    return reducer(state, action);
  }

  const newState = reducer(state, action);
  const oldUndoxState = { index: state.index, history: state.history };
  return {
    ...newState,
    ...oldUndoxState,
  };
};

type LimitConfig = { limit?: { maxCount: number; resetStateAction: string } };
const undoxEnhancerLimit = <T, S extends UndoxState<T, A>, A extends Action>({
  limit,
}: LimitConfig) => (reducer: Reducer<S, A>) => (state: S, action: A) => {
  const newState = reducer(state, action);
  if (limit && newState.history.length > limit.maxCount) {
    const limitedActionsHistoryLength = Math.floor(limit.maxCount / 2);

    const removedActionsHistory = newState.history.slice(
      0,
      newState.history.length - limitedActionsHistoryLength,
    );
    const removedActionsHistoryState = (removedActionsHistory.flat() as A[]).reduce(
      reducer,
      undefined,
    );

    const preservedActionsHistory = newState.history.slice(
      newState.history.length - limitedActionsHistoryLength,
    );
    const newHistory = [
      { type: limit.resetStateAction, payload: removedActionsHistoryState?.present },
      ...preservedActionsHistory,
    ];

    return {
      ...newState,
      history: newHistory,
      index: newHistory.length - 1,
    };
  }
  return newState;
};

export const undoxEnhancer = <S, A extends Action>(
  reducer: Reducer<S, A>,
  config: PreserveConfig & FilterConfig & LimitConfig = {},
) => {
  return _flow(
    undoxEnhancerPreserve(config),
    undoxEnhancerFilter(config),
    undoxEnhancerLimit(config),
  )(reducer);
};
  const reducer = undoxEnhancer(undox(counterReducer, init()), {
    limit: { maxCount: 6, resetStateAction: actionTypes.REINIT },
    filter: ({ type }) => HISTORICAL_ACTIONS_TYPES.has(type),
    preserve: ({ type }) => CAN_NOT_UNDO_ACTIONS_TYPES.has(type),
  });
JannicBeck commented 3 years ago

Hey, great to hear! Yes I'm always open for pull requests that improve the library. :)

On the features you suggested:

  1. Limit: I'm all on board on this one!
  2. Filter: Thought about this frequently but not a big fan of it until now. I also ran into this multiple times when I developed my Application. The thing is: If the action should not be undoable like closing opening a menu, this state should not be in the undoable reducer, but in another one IMHO. Like in a separate branch of your state tree. But hey thats just my experience, so there might be a use case in which that might not work and one actually needs filtering (If this is the case I would appreciate any insight why you need that state to be part of the undoable state).
  3. Preserve: Did not have the time to think about this one yet but maybe its similar to filter? You need the result of the success action but don't want it to be undoable so maybe just don't save it in the undoable state?

Some advice for pull requests:

Buggytheclown commented 3 years ago
  1. Limit - nice
  2. Filter - yes, you are right, we should think about state. If any part of it should not be undoable - separate it
  3. Preserve -
    • i want to load some graph from the back-end (FETCH_SUCCESS)
    • i want to be able to undo some actions with this graph (except FETCH_SUCCESS)
    • maybe in the future i will have some kind of "sync" point with the back-end (SYNC_SUCCESS) and i don't want to undo SYNC_SUCCESS or any action before SYNC_SUCCESS

maybe I need the "clear history" feature, but this will be more imperative than "preserve"

JannicBeck commented 3 years ago

Hey @Buggytheclown just wanted to get back to you and give you an update:

  1. Limit - I'm at the final steps of limit implementation, just need to write some more tests :-)
  2. Filter - Does this work for you? What I encountered in our production environment was that I may want to pass additional state to the reducer so the definition would look like this: (state: S, action: A, additionalState: T) => S. Where additionalState could be something like "is menu open". This is not strictly necessary since you can also pass the additionalState in the action, but sometimes its just more convenient from my experience to have it as a third parameter. So if you also encounter this situation I will look into implementing this.
  3. Preserve - I think your solution of wrapping the undox reducer works great in that case! Provided you don't want any actions to be undone before SYNC_SUCCESS.