Design Proposal: Undo/Redo feature

[shuktika12163]

Problem statement

When editing a flow in designer, users have to manually revert their changes. This is a design proposal for undo/redo feature implementation in LAUX and Power Automate. Users will have undo/redo buttons in the command bar for reverting significant changes in their flow that we would save states for (eg. adding/removing action, adding text in parameter etc.).

Implementation details

Overview

We will store an array of past and future redux states that will track undo/redo states in the redux store. These state saves will be dispatched/triggered once certain actions are performed by user (i.e. on certain redux actions/reducers). Undo/redo buttons will be disabled based on past/future arrays being empty.

Functions available outside LAUX for host services to use:

1. onUndoClick - callback when user clicks undo for host's undo button onClick: When user clicks undo, the current state will be stored as the first element in future array and will be replaced by the last element in past array.
2. onRedoClick - callback when user clicks redo for host's redo button onClick: When user clicks redo, the current state will be stored at the end of past array and will be replaced by the first element in future array.
3. saveUndoState - function to dispatch/trigger a state save for undo/redo tracking, in case host services want to save state in cases not tracked by LAUX. When an undo state save is to be triggered, first save the current state as last element in the array, reset the future array to empty, then the action should be performed.

Option 1: Use redux-undo library

Undoable is an enhancer from redux-undo library that can be applied to the root reducer or specific slices. The enhancer changes the state to the format of past, present and future to track undo/redo states. For example, if applied to the RootState, the state would change to:

{
  past: Array<RootState>,
  present: RootState,
  future: Array<RootState>
}

We could choose to apply the enhancer to certain slices instead of the root state. They would also put the individual slice states in the format mentioned above. Unsure how this would work with undo/redo button clicks and if it could put the slices out of sync (will have to try a proof of concept for this).

Pros:

1. Re-using existing library. Undoable already provides options to save states on certain actions/reducers (by providing includeActions and excludeActions), group by options to group actions to save 1 state after a sequence of actions, limit on number of saved states etc. so we will not have to reimplement these. It also has existing functions to connect to undo and redo buttons, enable/disable them.
2. This enhancer is already being used by data mapper project too.

Cons:

1. The biggest con of this approach is that the existing state will change and it'll be under "present". This will be a big breaking change. We have a lot of places that use RootState or other slice states that will need to be updated to state.present and we run the risk of missing some. This will also break hosts that rely on RootState object.
2. Less flexibility for what we want to store in undo/redo state (part of a slice), cannot compress or store differences in states. It automatically saves the entire state for the slice we apply the function to.

Documentation:

• https://redux-undo.js.org/
• https://redux.js.org/usage/implementing-undo-history

Option 2: Implement undo/redo in LAUX from scratch

Storing undo/redo state history
We have two options for storing history for undo/redo:

1. Create an undo slice that stores past and future arrays of all the root state variables that we want to save the state for. I'm unsure if it'll be tricky or not possible to get access to RootState in the undo slice to save state (maybe creating a thunk function outside slice would work?). Undo/redo clicks would be dispatched actions that update the rootstate.

{
  ...rootState,
  undo: {
    past: Array<Partial<RootState>>,
    future: Array<Partial<RootState>>,
  }
}

2. Add past and future arrays to individual slices for which we want to save undo history. It might get a little tricky to keep different slice arrays in sync and to figure out if any slice's state changed at all to trigger a state save, but it should be doable. Undo/redo clicks could be extra reducers added to the individual slices.

For example, if we want to store only workflow state and operations state for undo history, state would change to something like:

{
  ...rootState,
  workflow: {
    ...workflowState,
    past: Array<Partial<WorkflowState>>,
    future: Array<Partial<WorkflowState>>
  },
  operations: {
    ...operationsState,
    past: Array<Partial<OperationsState>>,
    future: Array<Partial<OperationsState>>
  }
}

Triggering state save for undo history
We want to save state for undo history when the flow has changed from a user action, i.e. when the flow can potentially become dirty if it wasn't already and has changed from previous undo history save. We can add a debounce rate to not save state too often if the actions can occur too close together. Workflow slice already has a matcher that attempts to set the flow as dirty on certain actions. We can probably use the same list of actions to trigger undo history state save on:

LogicAppsUX/libs/designer/src/lib/core/state/workflow/workflowSlice.ts

Line 482 in ea8387a

builder.addMatcher(

We have three options to trigger a state save for undo history (i.e. update the past and future arrays):

1. Use extraReducers: builder.addMatcher to trigger a undo history state save on matching only certain actions (eg. add node, delete node etc.). We would usually want to save state before the action is performed, so I'm unsure if this will work since the matcher will be called after action we match on is performed so we might not have the previous/beginning state from before action is performed when the matcher is called. However, if it can work, this would be an easy way of keeping all the actions we care about for undo history in one place and easy to update in future. This will also be similar to builder.addMatcher in workflow slice for updating isDirty variable.

builder.addMatcher(
  isAnyOf(
    addNode,
    moveNode,
    ...
  ),
  (state) => {
    // Open question: How can this be performed before actions like addNode etc. are done?
    // Get the present state variables we care about and append it to the past array for this state
    const partialPresent = { state.a, state.b, ... }
    state.past = [ ...state.past, partialPresent ]
  }
);

2. Create a new action saveUndoState using createAction and use extraReducers: builder.addCase to trigger state save on actions. This will be similar to resetWorkflowState action. The saveUndoState action would be dispatched at the beginning of the actions we want to trigger undo history save on (eg. add node, delete node etc.). I'm unsure how we can dispatch or trigger a call to saveUndoState for actions specified in slices like addNode.

// Create the action
export const saveUndoState = createAction('saveUndoState');

// Add case for action
builder.addCase(saveUndoState, (state) => {
    // Get the present state variables we care about and append it to the past array for this state
    const partialPresent = { state.a, state.b, ... }
    state.past = [ ...state.past, partialPresent ]
  }
);

// Dispatch the action
// Open question: How can we dispatch this from actions created in slices?
dispatch(saveUndoState(rootState));

3. Create a new action saveUndoState using createAction and dispatch it when we deep compare changes in workflow in DataProviderInner. This is not as nice as the previous two options but if the open questions for those cannot be figured out, we might have to go with this option. Also, unsure if this will cover all cases for user changes.

// Create the action
export const saveUndoState = createAction('saveUndoState');

// Dispatch the action in DataProviderInner:
const DataProviderInner: React.FC<BJSWorkflowProviderProps> = ({ workflow, children, runInstance, customCode, appSettings }) => {
  const dispatch = useDispatch<AppDispatch>();
  useDeepCompareEffect(() => {
    dispatch(initWorkflowSpec('BJS'));
    ...
   dispatch(saveUndoState(rootState));
  }, [runInstance, workflow, customCode]);

...
};

Preferred Solution

Option 2 of implementing undo/redo in LAUX from scratch is preferred since it gives us more flexibility with implementation and is less likely to break existing code. It also doesn't introduce a breaking change that would impact accessing the state in LAUX and by host services.

Which options to use within option 2 is a bit of an open question that I'm seeking help with:

1. State history: Storing undo/redo state in its own slice would contain the state and not modify other existing states, so it's less likely to affect existing code as compared to adding arrays in individual slices. It will also make it easier to keep all slices' states together and check for changes. However, it might be easier to add case/matcher and save states in individual slices (unsure about rootstate access in undo slice). It might also be easier/more generic to implement undo/redo click actions as extra reducers in individual slices.
2. Triggering state save: I think option 1 or 2 within this would be preferred but I'm unsure of the implementation feasibility of both. For first option, is there a way we can perform the state save specified in matcher first, then the action like addNode? I think option 2 kind of solves the problem in option 1 but introduces another issue. For second option, is there a way we can trigger state save by dispatching it from within slice actions like addNode? Maybe the solution for this is to just dispatch the saveUndoState action wherever these slice actions are called (eg. wherever addNode is dispatched, we dispatch saveUndoState first) - though this might be a problem in future if someone adds a new place that calls addNode but misses the undo state dispatch. I'm wondering if it'll have to be a combination of option 1 and 2 - addMatcher for slice actions like addNode where we store previous state before action and then use it in addMatcher (might have to do something for other slices to stay in sync), and addCase for dispatchable places.

Potential Optimizations

1. We can store compressed/partial states to reduce space usage. I'm planning to add telemetry to log size of various state objects to get some data for size.
2. We should have a limit for undo history - currently planning up to 100 states. In V1, we were able to store compressed flow definitions up to 100 based on our size analysis earlier. Redux state to be saved should be smaller than the flow definitions used for V1.
3. If needed, depending on how frequently state save is getting triggered, we might need to add debounce rate for triggering undo history save.

Open questions

The biggest open question is which approaches are preferred for implementation.

For option 2 (implementing from scratch), which ones are feasible? Re-listing open questions from option 2 in one place here:

Storing undo/redo state history:
Option 1. Is it possible to save part of root state in undo slice? Can we access root state in a slice under the root state?

Triggering state save for undo history:
Option 1: Is there a way we can perform the state save specified in matcher first, then the action like addNode?
Option 2: Is there a way we can trigger state save by dispatching it from within slice actions like addNode?
Option 3: Will it cover all user edits?

[rllyy97]

Hi Shuktika, thanks so much for the detailed write up!
I think it would be best to implement this from scratch just so we have full control over this feature.
Let's create an undo slice and store things the first way you mentioned:

{
  past: Array<Partial<RootState>>,
  future: Array<Partial<RootState>>,
}

I don't think there's a benefit to splitting up the slices into their own arrays, in our set / get actions we can just use the slices we need to and ignore the ones we don't, but every action will use the same slices anyways.

For your first history question, yes we should be able to access the root state but we will have to do so from a thunk. Setting the state data back in place might be a little more involved though. I'm not sure if we can/should modify the state directly in the thunk, we might need to define some set actions in each slice that we want updated that we can dispatch from the thunk. I think we'll need a thunk for storing state, which we can either call manually or have as a side effect of other actions defined in their slices, and thunks for undo/redo/clear that will be called manually. All 4 of these should be exposed outside the project.

Let me know if any of that is confusing or if you want to discuss it further.

[shuktika12163]

Thank you for the review!

I also have questions for this part:
I think we'll need a thunk for storing state, which we can either call manually or have as a side effect of other actions defined in their slices

For dispatching the thunk for storing state on certain actions (i.e. saving a new state for undo history), do you have suggestions on how it can be achieved as a side effect of other actions? I was thinking using builder.addMatcher to match other actions like addNode, but that would run after the action is performed, we instead want to save state before the action is performed (not sure if this can be specified). Also, we probably can't dispatch the thunk from inside one of the slice actions like addNode. I think one of these options would be cleaner if possible (less chances of future changes missing a undo state save) but doesn't look like it can be done?

The only feasible option I can think of currently is dispatching the thunk wherever the other reducer actions are called, eg. dispatch saveUndoState before addNode is called. But this means that any new places calling these slice actions in the future will also need to add the saveUndoState thunk (which could be missed). Is this option ok or is there a better way to add an enhancer/middleware for dispatching the thunk as a side effect of some slice actions?

[rllyy97]

Ah right, I was misunderstanding something, I was thinking we could use the builder.addCase({action}.pending, state ~~~ in extraReducers but I forgot that only allows us to modify the slice itself.
Instead I think the best option would be to define some custom redux middleware to get used in our top level libs\designer\src\lib\core\store.ts, and just run it based on a list of undoable actions.
ie.

function stateHistoryMiddlware({ dispatch, getState }) {
  return next => action => {
    if (undoableActionTypes.includes(action.type) {
      dispatch(stateHistory.storeStateToHistory)
    }
    next(action)
  }
}

The boilerplate for redux middleware might not be exactly that, but I know we have access to the action, dispatch, and next.
This would let us store the state before advancing to the redux action we want as undoable.