charkour / zundo

🍜 undo/redo middleware for zustand. <700 bytes
https://codesandbox.io/s/zundo-2dom9
MIT License
597 stars 19 forks source link

Feature Request: Custom function to apply future and past states #152

Open alexanderhorner opened 7 months ago

alexanderhorner commented 7 months ago

When invoking the undo or redo actions, the current implementation sets the state using the default set function provided by Zustand, which, if I recall correctly, relies on Object.assign. Enhancing this by allowing customization of the set function would be helpful.

A practical application of this could be optimizing the size of future and past states by storing only differences or 'patches'. This is already feasible for shallow state structures but poses challenges for deeper objects. By enabling the set function to be customizable, one could apply changes using a bespoke function before updating the currentState.

charkour commented 7 months ago

Hey @alexanderhorner,

Thanks for the comment! Have you checked out the diff option that is passed to the temporal middleware?

I should update the readme, but this can support diffs or patches.

alexanderhorner commented 7 months ago

Yes, I've tried to implement patches with that, but i didn't manage to get it to work with deeper objects.

Are you sure this supports this?

The example in the documentation saves patches like "path.to.deeper.object.poperty", but this will just create a key named path.to.deeper.object.poperty instead of actually a nested object.

charkour commented 7 months ago

Ah, you're right. The example documentation isn't helpful in your case. I believe achieving your intended use case is possible, but the docs provide an example for a different use case.

Have you looked into doing something like this? https://stackoverflow.com/a/20240290/9931154

I'll update the docs to support this method because I think it's what developers usually want. Thanks!

alexanderhorner commented 7 months ago

I have looked into something like this. The problem I have is I don't know how you would actually implement that, but maybe I just didn't understand something correctly?

The diff method is only usefully when saving past states/pushing new states, right? When applying (so when calling undo) it will just take whatever is in the past state array and call the set() method of zustand, wich uses Object assign I think.

Object.assign(
  { a: { b: 1 }}, 
  { a: { c: 2 }, d: 3 }
);

will just result in

{
  a: {
    c: 2
  },
  d: 3
}
geoffreygarrett commented 7 months ago

I've been playing around with the diff config option and in my case, I need more than just a CHANGE diff, but also REMOVE and CREATE. I believe @alexanderhorner is spot on with needing to allow the user to customize the set function further than that of the default.

The user could then use optics-ts or Ramda as mentioned in the zustand docs.

geoffreygarrett commented 7 months ago

A few hours later, I got a diff-based temporal middleware working, after dropping zundo's files into my project. I didn't have the time to make it non-intrusive and make a PR, and it may be a bit brute-forced (with the help of GPT):

Additional applyDiff and revertDiff in options

I used these libraries for configuring the options:

import * as _ from 'lodash';
import {produce} from 'immer';
import diff from "microdiff";
The applyDiff and revertDiff callbacks ```ts // ... export const useTableStore = create()( devtools( temporal( //... {applyDiff: (diffs) => { return produce((draft) => { diffs.forEach((diff) => { // Determine the parent path and the key/index to operate on const pathParts = diff.path.slice(0, -1); const keyOrIndex = diff.path[diff.path.length - 1]; const parent = _.get(draft, pathParts.join('.')); switch (diff.type) { case 'REMOVE': if (_.isArray(parent)) { // Use _.remove on the parent array _.remove(parent, (item, index) => index === keyOrIndex); } else { // For non-array paths, use _.unset normally _.unset(draft, diff.path.join('.')); } break; case 'CREATE': case 'CHANGE': _.set(draft, diff.path.join('.'), diff.value); // Change the property/item break; } }); }); }, revertDiff: (diffs) => { return produce((draft) => { diffs.forEach((diff) => { const pathParts = diff.path.slice(0, -1); const parentPath = pathParts.join('.'); const parent = _.get(draft, parentPath); switch (diff.type) { case 'CREATE': // Assuming items have a unique 'id' or similar property // Adjust this logic to match how you can uniquely identify items if (_.isArray(parent)) { // Use lodash's _.remove to find and remove the item by its unique identifier _.remove(parent, (item, index) => item.id === diff.value.id); } else { // For non-array paths or when not dealing with list items _.unset(draft, diff.path.join('.')); } break; case 'REMOVE': case 'CHANGE': // Directly set the old value back, assuming path and oldValue are correctly provided _.set(draft, diff.path.join('.'), diff.oldValue); break; } } ); }); }, diff: (pastState, currentState) => { return diff(pastState, currentState); }} // ... ``` I'm not happy about the performance and type-agnostic nature of: ```ts _.remove(parent, (item, index) => item.id === diff.value.id); ``` for `revertDiff` but it works for my data structures for now.

Changes in temporalStateCreator

I say changes, but like I said, it wasn't unintrusive, I rewrote it (might do a PR later if interested, and make it less intrusive). The time-consuming part (for me) is constructing the types to allow for a difference type that isn't tied to i.e. microdiff as a dep.

Rewrites of undo, redo and modification to _handleSet ```ts undo: (steps = 1) => { const { pastDiffs, futureDiffs } = get(); if (pastDiffs.length >= steps) { for (let i = 0; i < steps; i++) { const diffToRevert = pastDiffs.pop(); if (diffToRevert && options?.revertDiff) { // Use the revertDiff callback to generate a state updater function // Note: Adjusted to pass a single diff as an array for consistency const stateUpdater = options.revertDiff(diffToRevert); // Apply the updater function directly userSet(stateUpdater); futureDiffs.unshift(diffToRevert); // Move the reverted diff to futureStates for possible redo } } // Update the diff arrays after all operations set({ pastDiffs, futureDiffs }, false); } else { // No past states to undo } }, redo: (steps = 1) => { const { pastDiffs, futureDiffs } = get(); if (futureDiffs.length >= steps) { for (let i = 0; i < steps; i++) { const diffToApply = futureDiffs.shift(); if (diffToApply && options?.applyDiff) { // Use the applyDiff callback to generate a state updater function // Note: Adjusted to pass a single diff as an array for consistency const stateUpdater = options.applyDiff(diffToApply); // Apply the updater function directly userSet(stateUpdater); pastDiffs.push(diffToApply); // Move the applied diff back to pastStates for possible undo } } // Update the diff arrays after all operations set({ pastDiffs, futureDiffs }, false); } else { // No future states to redo } }, //... _handleSet: (pastState, replace, currentState, deltaState) => { // This naively assumes that only one new state can be added at a time if (options?.limit && get().pastStates.length >= options?.limit) { get().pastStates.shift(); } get()._onSave?.(pastState, currentState); set({ pastStates: get().pastStates.concat(deltaState || pastState), futureStates: [], pastDiffs: get().pastDiffs.concat([deltaState] || pastState), futureDiffs: [], }); }, ```