Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature request: limit, filter, preserve #31

Open
Buggytheclown opened this issue Jan 15, 2021 · 3 comments
Open

feature request: limit, filter, preserve #31

Buggytheclown opened this issue Jan 15, 2021 · 3 comments

Comments

@Buggytheclown
Copy link

Buggytheclown commented Jan 15, 2021

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
Copy link
Owner

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:

  • New features will need a thorough test suite with edge cases and happy cases.
  • I don't want to add any external library to this like lodash.

@Buggytheclown
Copy link
Author

  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
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants