When using the handleSet method to cooldown State pushes, diff gets the wrong pastState.

[alexanderhorner]

The pastState should be the last state actually pushed to the history (in other words, the last state handleSet was called on).

diff: (pastState, currentState) => {
  const delta = diff(currentState, pastState);
  console.log('delta', delta)

  const newStateFromDiff = delta.reduce(
    (acc, difference) => {
      type Key = keyof typeof currentState
      if (difference.type === 'CHANGE') {
        const pathAsString = difference.path.join('.') as Key
        acc[pathAsString] = difference.value
      }
      return acc
    },
    {} as Partial<typeof currentState>,
  );
  return Object.keys(newStateFromDiff).length > 0 ? newStateFromDiff : null;
},
handleSet: (handleSet) => (state) => {
  if (!state) return

  debouncedHandleSetTimeoutID && clearTimeout(debouncedHandleSetTimeoutID)
  debouncedHandleSetTimeoutID = window.setTimeout(() => {
    handleSet(state)
  }, 300)
}

let debouncedHandleSetTimeoutID: number | null = null

[alexanderhorner]

The state that is passed to handleSet also should be the last actually saved history state. Else if you type 'A', wait a bit, and then type 'BCDE' real quick, the pastState will be 'ABCD'. It should be 'A' though.

[alexanderhorner]

Diff and equality does't make sense if the currentState isn't compared to the last state stored in history. Or am I missing something?

[charkour]

Hey @alexanderhorner, thanks for using zundo! Sorry for the issues. Could you make a reproduction in Code Sandbox of the issue you're seeing?

Thanks

[alexanderhorner]

Sure. Before I do that, can you confirm that this is even an issue? Or is this how it's supposed to work?

[alexanderhorner]

Here is a Sandbox example: https://codesandbox.io/s/zundo-forked-233fhc?file=/src/App.tsx

The handleSet is debounced by 1 second. So if you wait more than 1 second between clicking increment, the history is 0, 1, 2, 3 like you expect.

Now, if I click increment rapidly 5 times, the history should be [0] (with current state being 5). What actually happens is, that the history is [4]

This happens because, even though the handleSet is being called with the previous state every time the state changes. So for increase from 0 to 1, handleSet saves 0 to the history. When you click 5 times rapidly, the first 4 increases are dropped, and only when increasing from 4 to 5, handleSet is called with 4, and saved to the history.

In other words:

When not dropping handleSet calls, the delta is between the current state, and the last state State: 0 -> State: 1 State: 1 -> State: 2 State: 2 -> State: 3 etc

When dropping handleSets, the delta should be between the last saved state, and the current one. So in this case: State: 0 -> State: 5

[charkour]

Thanks for creating that! I appreciate the explanation which helps me to debug your issue.

To clear up confusion, based on your implementation, handleSet calls will be dropped, so there's no bug there. If you expect all of the states to exist within the history then you should remove the debounce. If you need to debounce the handleSet calls for UI requirements, then we could either re-write your handleSet function or change how you're accessing the store.

Based on your explanation, the diff function is working properly. I think the best path forward is to either 1) remove the debounce method or 2) rewrite the handleSet function to not drop state changes.

Does this help?

[alexanderhorner]

The reason for dropping the handleSet calls is to debounce it, or in other words to group together fast consecutive changes.

Maybe it’s clear if I try to explain what I want to achieve. For example: let’s say I type A, wait 5 seconds, then type BCDE really quick. If I press undo now, I should get A, not ABCD. So state 1 is “A”, and state 2 is “ABCDE”, and the difference should be “add BCDE”.

The way I see it, this isn’t possible right now, because handleSet is called with prevState, which is (same for diff and equality) the last state a change was detected for, instead of the last state that was actually tracked and saved to the pastStates array.

Unless I miss something, i would expect the diff and equality function to compare the current state to the last state in the pastStates array.

[alexanderhorner]

Hey, any update on this? I still think this doesn't make much sense as it currently works.

[charkour]

Hey @alexanderhorner,

Sorry this fell off my todo list. It doesn't totally make sense, but for now, I'd recommend removing the debounce from handleSet.

[alexanderhorner]

I need the denounce to group together changes that happen very quickly after one another. In my application I have a graph, and when I move a node in that graph, every pixel counts as a new state. Of course when I press undo, I just want it to go back to its last position where I started the drag from.

As general solution i thought of denouncing the handleset method and only saving the state after nothing changed for a few hundred milliseconds. But right now it just saves the last state that got pushed (so a movement from y=10 to y=100 would save y=99)

Let me know if I can help in any way with this issue. And thank you for taking the time.

[charkour]

I'm finally getting around to this, sorry for the delay.

I don't initially know what the expected behavior is here; but intuitively, I think it's working as expected, but I could very well be wrong.

[charkour]

Hi @alexanderhorner,

If you checkout this CodeSandbox, does this solve your problem? I think you were close to getting a working solution, but I added a package that made it easy to add debouncing and store the leading change.

[alexanderhorner]

Hello @charkour, thanks for taking the time to make a demo. Unfortunately the CodeSanbox keeps refreshing in an infinite loop.

I did implement a similar solution though (without extra packages, so a bit messier), and it seems to work great.

I actually thought about this solution before, but thought it wouldnt fix the issues I had with equality and diff. But it does.

Thanks again for the help. Maybe update the cool down part of the documentation, if anyone else has a similar problem.

[charkour]

Hey @alexanderhorner, I'm glad that you found a solution!

I'll gladly improve the documentation. Could you point out what section specifically caused confusion? Thank you!

[charkour]

I've added this example to the docs in #148. Thanks for using zundo!

[alexanderhorner]

Thanks for making zundo!

[michaelabela]

hey @alexanderhorner what was your solution? I'm running into a similar issue. I have a set of points that I'm visualizing on a map and there's a marker draw mode where the user can quickly label dozens of points in a second or so. In the case that I have a map with 1 point, and then add 10 points in 1 second, I want undo to return the map to the state with just 1 point.

[alexanderhorner]

Hey man. The solution is storing the leading state (first state of the series) and saving that once the debounce timeout is reached.

Do you know what I mean? If not, I can try to find the piece of code for it tomorrow.