Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The Zustand middleware's set cannot safely be nested inside batch #1495

Open
jamesbvaughan opened this issue Feb 28, 2024 · 0 comments
Open
Labels
bug Something isn't working Custom App

Comments

@jamesbvaughan
Copy link

Describe the bug

The zustand store is modified twice when mixing the store's set function with Liveblocks' batch function.

To Reproduce

This code results in a bug:

type TestStore = {
  testArray: string[];
  setTestArray: (array: string[]) => void;
  getTestArray: () => string[];
};

const useTestStore = create<WithLiveblocks<TestStore>>(
  liveblocks(
    (set, get) => ({
      testArray: [],
      setTestArray: (array: string[]) => {
        set({ testArray: array });
      },
      getTestArray: () => get().testArray,
    }),
    {
      client: liveblocksClient,
      storageMapping: {
        testArray: true,
      },
    },
  ),
);

const { setTestArray, getTestArray, liveblocks } = useTestStore.getState();
liveblocks.enterRoom("test-room-id");

liveblocks.room.batch(() => {
  // This line modifies the zustand store, then modifies the current batch of Liveblocks changes.
  setTestArray(["element-test-id-2"]);

  // When this callback finishes, `batch` executes all the batched changes in the Liveblocks storage. Because the Zustand `set` function has finished, `isPatching` is no longer `true` and the Liveblocks zustand middleware will apply these changes to the Zustand store, duplicating them.
  // This isn't a problem for things like `LiveObject.set`, but it is a problem for `LiveList.push`.
});

console.log(
  "testArray after - this will have two elements when it should have just one",
  getTestArray(),
);

Expected behavior

I expected that the zustand middleware and the batch function would work nicely together and that the batched changes would not be duplicated in the zustand store.

Additional context

This is admittedly a pretty unusual edge case. We'd typically not hit this issue because we'd be using a single zustand set call for all the grouped changes, and the zustand middleware already uses batch effectively internally, but there are some cases where we have reason to split the set calls.

Some of our current workarounds are:

  • Find ways to use a single set call. (Not always ideal or possible with the structure of our code.)
  • Use pause and resume to at least get part of the benefit of batching changes. (Only partially helpful.)
  • Directly manipulate the Live data types. (Requires more traversal of the Live-ified version of our document than we'd like and flips the order of operations so that the Liveblocks update happens before the zustand update.)
@jamesbvaughan jamesbvaughan added bug Something isn't working triage needed The issue needs to be reviewed by the team labels Feb 28, 2024
@adigau adigau added the public repo label Apr 15, 2024 — with Linear
@adigau adigau removed triage needed The issue needs to be reviewed by the team public repo labels Apr 15, 2024
@adigau adigau added the 🎪 Room label Apr 24, 2024 — with Linear
@adigau adigau added the Engineering label May 9, 2024 — with Linear
@adigau adigau removed the Engineering label May 9, 2024
@adigau adigau removed the 🎪 Room label May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Custom App
Projects
None yet
Development

No branches or pull requests

2 participants