zundo: Updating the value of an untracked value prior to updating a tracked value results in pastStates not updating when using handleSet (Sandbox example)

This was fixed in v2.1.0

Thanks @charkour! I made some minor changes based on your feedback to PR 141. The main new change is to dry up the conditional calling of curriedHandleSet based on equality/diff functions and make sure it is being handled identically in both ways of setting zustand state, as you helped me understand.

After some noodling, I do not currently think it is possible to pass additional parameters to curriedHandleSet without adding some minor changes to the API (handleSet needs to be able to receive more parameters to be able to receive more parameters 🥴). Although I think the downsides of not being able to do this are minor, there is likely a way to do this that would require a more major refactor of how zundo works (I only have vague fuzzy maybe fake ideas).

However, I also don’t think that the API changes required to be able to pass additional paramaters to curriedHandleSet are particularly significant (they would be non breaking I think, and I don’t think users would need to refactor any code at all). So I demo’d what that would look like in an additional PR here. I am not sure though what changes require majors version bumps and which do not. Anyways, that PR is sort of a hybrid of the two other PRs – if you happen to like it better.

I know it’s the holidays so no rush and please take your time, and thanks again for your help with all of this!

This has been resolved in https://github.com/charkour/zundo/pull/149

I’ll release v2.1.0 shortly.

Hey @pstrassmann, thank you for the awesome work on both the bug write-up and creating multiple PRs. This is what makes open source so awesome. I hope you had a nice holiday too.

As for version changes, I recommend checking out the Semver docs. In short, if we are adding features (such a more parameters in a callback) then it’s a minor version change, but if we remove or change parameters or the external behavior, then it’s a major version change. For small fixes, it would be a patch. I have my gripes with semver (I wish it were at least four numbers, not three), but it’s what the industry uses 😃

Thanks for being conscious of avoiding breaking changes! Happy to chat more about this if you have more questions. Thank you

We are looking at adding zundo in prod to a currently live application at Classy!

I’ll work on patching the package locally today. I believe patching should be a suitable workaround in the short term, thank you for that! In the meantime, I’m happy to address any feedback big or small related to either of the potential solutions in the PRs.

Also, if you’d prefer to implement an alternative solution yourself or reimplement one of my solutions to be more in line with your stylistic preferences, that is of course also okay! Take whatever time you need to feel comfortable with a solution.

Thank you for your help!

Ah that’s wonderful, thank you for that!

Hi @charkour – Thank you for bearing with my as I get a better hold of my issue. Please disregard the last Sandbox I sent you, I was confused. Unfortunately the combined setter doesn’t work for us (described more in detail below) due to users being able to modify state in different places in the application all within the throttle interval.

The TL;DR of my issue is: Is there a way to conditionally trigger a throttle depending on which fields of the zustand store change? If so, would it require handleSet callback to have access to pastState and currentState?

More detail:

To summarize, the issue I’m having is that when using both partialize and handleSet to implement a throttle, changing the state of an untrackedValue in zustand triggers the throttle, making it so that any subsequent state setting of tracked values (that fall within the throttle window) to not be registered. I now realizse this isn’t a bug – throttle is working exactly as intended by running on every change to the zustand store. And because a change to the store could contain changes to both untracked and tracked values (eg theoretically something like incrementUntrackedValueAndBears()), this makes sense as the way it would work.

However, in our application, users may perform actions in various places that change untracked and tracked values, making it unrealistic to combine these actions into a single state setter as you suggested like incrementUntrackedValueAndBears()

As a contrived example, take two buttons

const handleButtonClickA = () => {
   incrementUntrackedValue()
}

const handleButtonClickB = () => {
   incrementTrackedValue()
}

Sandbox for above example: https://codesandbox.io/p/sandbox/zundo-untrackedvalue-wq9cm8?file=%2Fsrc%2FApp.tsx%3A63%2C11

If ButtonA is clicked and then ButtonB is clicked all within the throttle interval, the change to the trackedValue will not be registered in history, as ButtonA initiated the throttle interval.

A potential solution to my problem could be to make throttle smarter – if a state setting action only changes zustand store fields that are untracked, throttle interval should not be initiated. If I could do something like that, then:

// This would never initiate the throttle interval
const handleButtonClickA = () => {
   incrementUntrackedValue()
}

// If clicked immediately after ButtonA, this would be registered in history and throttle interval initiated.
const handleButtonClickB = () => {
   incrementTrackedValue()
}

I’m still chewing on this one, but if you have any insight for how to do this – or a potential change to a forked version of zundo, it would be much appreciated!

To achieve this, I’m wondering if the handleSet callback would need access to both pastState and currentState, diffing the two, and checking whether the diff only contains changes to untracked fields. Is something like this within the realm of possibility? I feel like other users with relatively complex applications could also encounter this issue, given that zustand appears to be biased towards structuring things as a single store? But I’m not sure.

Thanks so much for taking the time to help with this!

@charkour Thank you for taking a look! It is very much appreciated!

I don’t know how helpful this is, but in my initial attempts to debug this, the issue appears to possibly be (proximally) caused by the equality function receiving a stale value for newState.

Because the newState is then evaluated to be identical to the pastState, a piece of state history is not added and therefore pastStates is not updated with anything. And this only happens if an untracked value is modified in the store before and in the same callback as a tracked value is modified.

Here is a sandbox with this (ever so slightly further) debugging:

https://codesandbox.io/p/sandbox/zundo-with-partialize-and-deep-equal-debugging-jzgq47?file=%2Fsrc%2FApp.tsx%3A35%2C9

And a code example showing the minor debugging attempt (see equality)

    {
      equality: (pastState, newState) => {
        console.log("pastBears: ", pastState.bears);
        console.log("newBears: ", newState.bears);
        console.log("areBearsEqual: ", pastState.bears === newState.bears); // true

        return deepEqual(pastState, newState);
      },
      handleSet: (handleSet) =>
        throttle<typeof handleSet>((state) => {
          console.log("handleSet called");
          handleSet(state);
        }, 500),
      partialize: (state: MyState): HistoryTrackedState => {
        const { untrackedValue, ...trackedValues } = state;
        return { ...trackedValues };
      },
    },

Thank you again!!