Give users more flexibility in writing handleSet fn (alternative solution to PR #141)

[pstrassmann]

This is an alternative solution to addressing the issues fixed in https://github.com/charkour/zundo/pull/141

In this solution, rather than changing the conditions under which handleSet is called like in the other PR, users instead can write a handleSet fn that returns a fn that, in addition to receiving pastState as before, now also receives currentState.

This allows users to return a function that can, based on their own logic around pastState and currentState, determine when to run handleSet.

This allows users to write a handleSet function like:

 handleSet: (handleSet) => {
          const throttleFn = throttle((state) => {
              handleSet(state);
            }, 1000);

         // return a function that will run a throttle fn that runs handleSet, but only if state has changed according to
         // user defined logic
          return (
            pastState: Partial<MyState>,
            currentState?: Partial<MyState>,
          ) => {
            const hasStateChanged =
              diff(pastState, currentState ?? {}).length > 0;
            if (hasStateChanged) {
              throttleFn(pastState);
            }
          };
        },

while remaining compatible and non-breaking with existing handleSet functions written like:

 handleSet: (handleSet) => {
          return throttle((pastState) => {
            handleSet(pastState);
          }, 1000);
        },

This solution has been tested locally, and 1 relevant test has been added demonstrating a handleSet fn that uses currentState to conditionally run the throttle fn that runs handleSet

[charkour]

Thanks you for making this PR, adding tests, and providing a thorough description!

I'll have some time later today to review this in depth and provide some feedback. Thank you!

[charkour]

This "leave it to the user" approach is awesome! I think I prefer this to the other PR proposal, but I'm wondering if the bug is solvable by tweaking the options in the original issue you raised. I'll keep playing around with it, but my gut says we'll likely go with this solution here.

[pstrassmann]

As an update on this -- I've been able to successfully use a patch based on this PR for our application to resolve the issue of throttles getting triggered by untrackedState. I still think something like this makes a lot of sense to bring in to the zundo though!

I realized I did not update the README in this PR, so I will do that very soon. There is at least one interesting catch to describe, like the fact that if one uses an equality fn within handleSet ZundoOption to conditionally call handleSet, one does not also need to use an equality fn ZundoOption (unless you want the equality fns to be different).

[charkour]

Thanks for the additional update on this! I've been thinking through some possible work around/ideal solutions that don't require a core change that I'd like to run past you before we decide to go forward with this PR.

Glad you have a patch! My thought process is 1) the workarounds an are sufficient for your use case at work or 2) the workarounds aren't sufficient and we move forward with this PR. Either way, the goal will be to remove the patch from your code. It's just a temporary hold over while we make a decision. Thanks for your patience on this!

[pstrassmann]

Ok yeah that sounds good! I am definitely open to exploring and chatting about other solutions or work arounds. Thank you!

[charkour]

Hi @pstrassmann, after taking time to verify my workarounds didn't work, I spent time reviewing the PRs. Both are great, but I think the best is a combination of the two. #141 is closer to what I imagine, but we can pass more data to the curriedHandleSet function.

What do you think? If you agree, let's close this PR and you can make updates to #141

[charkour]

Closing in favor of #149