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

[Feedback Request] Does Zustand v5-rc work for you? #2741

Closed
dai-shi opened this issue Sep 14, 2024 · 36 comments
Closed

[Feedback Request] Does Zustand v5-rc work for you? #2741

dai-shi opened this issue Sep 14, 2024 · 36 comments

Comments

@dai-shi
Copy link
Member

dai-shi commented Sep 14, 2024

How to Install

npm i zustand@next

Migration Guide

https://github.com/pmndrs/zustand/blob/main/docs/migrations/migrating-to-v5.md

How to respond

Add comments to this issue or add reactions to this issue comment.

👍 : I've tested v5-rc and it works fine for me.
👎 : I've tested v5-rc and found an issue. < Comment below!
👀 : I'll try it later.

@pavitra-infocusp
Copy link

pavitra-infocusp commented Sep 16, 2024

Is it intentional to push an RC release to the latest?
image

Anyways, my app doesn't work. It crashed when I open the page :(

Unexpected Application Error!
Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate. React limits the number of nested updates to prevent infinite loops.
Error: Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate. React limits the number of nested updates to prevent infinite loops.
    at checkForNestedUpdates (http://localhost:5173/node_modules/.vite/deps/chunk-HHZE2HUH.js?v=9df50b02:19659:19)
    at scheduleUpdateOnFiber (http://localhost:5173/node_modules/.vite/deps/chunk-HHZE2HUH.js?v=9df50b02:18533:11)
    at dispatchSetState (http://localhost:5173/node_modules/.vite/deps/chunk-HHZE2HUH.js?v=9df50b02:12403:15)
    at update (http://localhost:5173/node_modules/.vite/deps/@nextui-org_react.js?v=54cc688f:14580:11)
    at http://localhost:5173/node_modules/.vite/deps/@nextui-org_react.js?v=54cc688f:14583:7
    at commitHookEffectListMount (http://localhost:5173/node_modules/.vite/deps/chunk-HHZE2HUH.js?v=9df50b02:16915:34)
    at commitLayoutEffectOnFiber (http://localhost:5173/node_modules/.vite/deps/chunk-HHZE2HUH.js?v=9df50b02:17002:23)
    at commitLayoutMountEffects_complete (http://localhost:5173/node_modules/.vite/deps/chunk-HHZE2HUH.js?v=9df50b02:17980:17)
    at commitLayoutEffects_begin (http://localhost:5173/node_modules/.vite/deps/chunk-HHZE2HUH.js?v=9df50b02:17969:15)
    at commitLayoutEffects (http://localhost:5173/node_modules/.vite/deps/chunk-HHZE2HUH.js?v=9df50b02:17920:11)

Grepping the usages of useStore in the code:

const openModal = useStore((state) => state.openModal);

const [searchValue, setSearchValue] = useStore((state) => [state.searchValue, state.setSearchValue]);

const [statusFilter, setStatusFilter] = useStore((state) => [state.statusFilter, state.setStatusFilter]);

const [statusFilter, searchValue] = useStore((state) => [state.statusFilter, state.searchValue]);

const isOpen = useStore((state) => state.modalOpen);
const closeModal = useStore((state) => state.closeModal);

@dai-shi
Copy link
Member Author

dai-shi commented Sep 16, 2024

Yes, this is the last resort I can think of to get some feedback. related tweet
Thanks for reporting.
@pavitra-infocusp Can you try this?

### Requiring stable selector outputs

@pavitra-infocusp
Copy link

@dai-shi I have updated the comment above with how I use zustand. It's a new and simple app. Do I have to provide a Fallback for all selectors? That would be so unnecessary, wouldn't it be?

@dai-shi
Copy link
Member Author

dai-shi commented Sep 16, 2024

const [searchValue, setSearchValue] = useStore((state) => [state.searchValue, state.setSearchValue]);

This looks bad. Even with v4, this can cause extra re-renders. So, in that sense, v5 catches such bugs.
The easiest fix would be to use useShallow.

provide a Fallback for all selectors?

In this case, that wouldn't solve it anyway.

@pavitra-infocusp
Copy link

So array == bad is the problem, because it creates a new array on each render?

@dai-shi
Copy link
Member Author

dai-shi commented Sep 16, 2024

it creates a new array on each render?

True.

@pavitra-infocusp
Copy link

-const [searchValue, setSearchValue] = useStore((state) => [state.searchValue, state.setSearchValue]);
+const searchValue = useStore((state) => state.searchValue);
+const setSearchValue = useStore((state) => state.setSearchValue);

Is this the only way to make the selector references stable? Is it possible to group them together to reduce boilerplate and function calls?

@dbritto-dev
Copy link
Collaborator

@pavitra-infocusp sure

const [searchValue, setSearchValue] = useStore(useShallow((state) => [state.searchValue, state.setSearchValue]));

@pavitra-infocusp
Copy link

pavitra-infocusp commented Sep 16, 2024

Thanks @dai-shi and @dbritto-dev! Zustand v5 is now working fine.

@Olfdev
Copy link

Olfdev commented Sep 18, 2024

I was having troubles as well on 5.0.0 rc2 where v4.5.5 was working fine (or at least, not telling me there was issues but was running fine). I spent a few hours trying to find the culprit because the error logs where completely useless.

  1. Warning: The result of getSnapshot should be cached to avoid an infinite loop
  2. Uncaught Error: Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate. React limits the number of nested updates to prevent infinite loops.

I had to use useShallow, that's it. Now I need to go see what this is all about to understand why this suddenly happened.

from:

const {tables, fetchTables} = useUserStore((state) => ({tables: state.tables, fetchTables: state.fetchTables}));

to:

const {tables, fetchTables} = useUserStore(useShallow(state) => ({tables: state.tables, fetchTables: state.fetchTables})));

@dai-shi
Copy link
Member Author

dai-shi commented Sep 18, 2024

Thanks for reporting! This is a bit unexpected to me, as my assumption was it can cause the infinite loop even in v4. I think we need to clarify this in the v5 migration guide.

👉 #2749

@mehimanshupatil
Copy link

typescript error when using with immer middleware and slice pattern.
error location Inside Immer callback, where function arguments are being spread (...a)
reproducible example

Types of parameters 'shouldReplace' and 'replace' are incompatible.
Type 'true' is not assignable to type 'false'.

@dai-shi
Copy link
Member Author

dai-shi commented Sep 19, 2024

Thanks reporting. #2580 is the related PR.
@Yonom Can you investigate it please? We need a test too.

@cuquo
Copy link

cuquo commented Sep 21, 2024

Everything works with the RC. I tried previous versions, and the only additional step I needed to take was to avoid having a fallback to empty arrays. With the React compiler, there was an issue with react.Ref when I created the Store context. However, that was fixed by the React team some weeks ago.

@dbritto-dev
Copy link
Collaborator

dbritto-dev commented Sep 24, 2024

@mehimanshupatil here you go -> https://tsplay.dev/N57P9W

@wuarmin
Copy link

wuarmin commented Sep 24, 2024

In development everything works, but in production build I get weird and hard to debug errors. I already disabled minify and enabled sourcemap in my vite.config.js but without success.

Screenshot from 2024-09-24 20-09-58
Can you imagine what is going on here?

@dbritto-dev
Copy link
Collaborator

@wuarmin would you mind creating a minimal repro on stackblitz? I'd be happy to help

@dai-shi
Copy link
Member Author

dai-shi commented Sep 24, 2024

https://react.dev/errors/185

It seems like unstable selector result. I wonder why it doesn't happen in development.

@wuarmin
Copy link

wuarmin commented Sep 25, 2024

@wuarmin would you mind creating a minimal repro on stackblitz? I'd be happy to help

Buh, I'm pretty busy at the moment. I will try, but I probably won't be able to do this repro this week.

It seems like unstable selector result. I wonder why it doesn't happen in development.

Yes, it was pretty sobering when I deployed and then nothing worked anymore. I searched for a long time until I found out that zustand had something to do with it.

This is the zustand store that triggers the error in the prod area, among other things:

import { create } from "zustand";
import { useQuery, keepPreviousData } from "@tanstack/react-query";
import { gql } from "graphql-request";
import { apiClient } from "@/features/query";
import { Order } from "@/graphql/fragments";

const query = gql`
  query FilteredOrders($after: String, $first: Int, $query: OrdersQueryInput!) {
    filteredOrders(after: $after, first: $first, query: $query) {
      totalCount
      nodes {
        ...Order
      }
    }
  }
  ${Order}
`;

export const statusCategories = [
  { id: "all", name: "Alle" },
  { id: "action_required", name: "Aktion erforderlich" },
];

const afterForPage = (page, itemsPerPage) => String((page - 1) * itemsPerPage);

const pageParams = (page, itemsPerPage) => ({
  after: afterForPage(page, itemsPerPage),
  first: itemsPerPage,
});

const paginationFilterState = (set, get) => ({
  filterItems: { statusCategory: "action_required" },
  filters: [],
  sort: [{ id: "id", desc: true }],
  rowSelection: {},
  page: 1,
  actions: {
    applyFilters: (filters) => set({ filters, page: 1 }),
    applyFilterItem: (id, value) => {
      const newFilterItems = { ...get().filterItems, [id]: value };
      if (!value) {
        delete newFilterItems[id];
      }
      set({ filterItems: newFilterItems, page: 1 });
    },
    goToPage: (page) => set({ page, rowSelection: {} }),
    sort: (sort) => set({ sort }),
    applyRowSelection: (rowSelection) => set({ rowSelection }),
  },
});

const useQueryStore = create(paginationFilterState);

const useFilter = () =>
  useQueryStore(({ filters, filterItems }) => {
    return {
      ...filters.reduce((acc, { id, value }) => ({ ...acc, [id]: value }), {}),
      ...filterItems,
    };
  });
export const useFilterItems = () =>
  useQueryStore(({ filterItems }) => filterItems);
export const useFilters = () => useQueryStore(({ filters }) => filters);
export const usePage = () => useQueryStore(({ page }) => page);
export const useRowSelection = () =>
  useQueryStore(({ rowSelection }) => rowSelection);
export const useSort = () => useQueryStore(({ sort }) => sort);
export const useQueryActions = () => useQueryStore((state) => state.actions);

export function useOrders({ itemsPerPage = 100 } = {}) {
  const filter = useFilter();
  const page = usePage();
  const sort = useSort();

  return useQuery({
    queryKey: ["orders", "list", filter, sort, page, itemsPerPage],
    placeholderData: keepPreviousData,
    queryFn: async () => {
      return apiClient
        .rq(query, {
          ...pageParams(page, itemsPerPage),
          query: { filter, sort },
        })
        .then((data) => ({
          totalPages: Math.ceil(data.filteredOrders.totalCount / itemsPerPage),
          records: data.filteredOrders.nodes,
        }));
    },
  });
}

@dai-shi Do you see a problem.

Thanks

@dai-shi
Copy link
Member Author

dai-shi commented Sep 25, 2024

  useQueryStore(({ filters, filterItems }) => {
    return {
      // ...
    };
  });

is creating a new object every time which causes infinite loops.

@dai-shi
Copy link
Member Author

dai-shi commented Sep 25, 2024

Wow, I didn't notice that. @dbritto-dev Can you fix?

@dbritto-dev
Copy link
Collaborator

@dai-shi sure. BTW, we talked about that in useStore

@wuarmin
Copy link

wuarmin commented Sep 25, 2024

is creating a new object every time which causes infinite loops.
Thanks @dai-shi

What's the best way to rewrite this:

const useFilter = () =>
  useQueryStore(({ filters, filterItems }) => {
    return {
      ...filters.reduce((acc, { id, value }) => ({ ...acc, [id]: value }), {}),
      ...filterItems,
    };
  });

@dai-shi
Copy link
Member Author

dai-shi commented Sep 25, 2024

It's tricky. proxy-memoize may help, but I think you should at least separate to useFilters and useFilterItems. Then, half of the problem is solved, but then if we are changing the signature anyway, it's breaking, so you should also consider avoid reduce completely. Hm, maybe this works.

const useFilters = () => {
  const filters = useQueryStore((state) => state.filters);
  return filters.reduce((acc, { id, value }) => ({ ...acc, [id]: value }), {});
};
const useFilterItems = () => useQueryStore((state) => state.filterItems);

@Yonom
Copy link
Contributor

Yonom commented Sep 25, 2024

re: @mehimanshupatil @dai-shi

Minimal example - works in v4, breaks in v5

import { create, StateCreator } from "zustand";
import { immer } from "zustand/middleware/immer";

interface Slice1 {
  foo: string;
}

interface Slice2 {
  bar: string;
}

const createSlice1: StateCreator<Slice1, [], [], Slice1> = (set) => ({
  foo: "test",
});

export const useBoundStore = create<Slice1 & Slice2>()(
  immer((...a) => ({
    ...createSlice1(...a),
  }))
);

Fix in v5:

const createSlice1: StateCreator<Slice1 & Slice2, [], [], Slice1> = (set) => ({

The first parameter of StateCreator now requires the entire correct type of the store. I think this is a bug?

Is the StateCreator's first parameter supposed to contain the entire type of the store?

Counterpoint: the example for slices pattern in the docs does not include the entire store type (is missing the SharedSlice type) which works in v4 but does not work in v5 if you are using immer.
https://zustand.docs.pmnd.rs/guides/typescript#slices-pattern

I unfortunately don't have the bandwidth in the next two weeks to investigate why. Help is appreciated!

@dai-shi
Copy link
Member Author

dai-shi commented Sep 25, 2024

Is the StateCreator's first parameter supposed to contain the entire type of the store?

Yes, I believe so. (And, yes, I noticed in some cases, it can be a Partial of it.)
So, I think the docs can be improved. @dbritto-dev What do you think?

@dbritto-dev
Copy link
Collaborator

dbritto-dev commented Sep 25, 2024

@dai-shi @Yonom v5 looks good to me, some people think that the slice only get the slice state instead the whole store state when actually you get the whole store state

@dai-shi
Copy link
Member Author

dai-shi commented Sep 26, 2024

@dbritto-dev

BearSlice & FishSlice,

Should that be BearSlice & FishSlice & SharedSlice?

@wuarmin
Copy link

wuarmin commented Sep 26, 2024

@dai-shi

It's tricky. proxy-memoize may help, but I think you should at least separate to useFilters and useFilterItems. Then, half of the problem is solved, but then if we are changing the signature anyway, it's breaking, so you should also consider avoid reduce completely. Hm, maybe this works.

const useFilters = () => {
  const filters = useQueryStore((state) => state.filters);
  return filters.reduce((acc, { id, value }) => ({ ...acc, [id]: value }), {});
};
const useFilterItems = () => useQueryStore((state) => state.filterItems);

Why do you think that following works?

const useFilters = () => {
  const filters = useQueryStore((state) => state.filters);
  return filters.reduce((acc, { id, value }) => ({ ...acc, [id]: value }), {});
};

It returns everytime a new reference, right? Is this expected to work in v5?
Thanks

@dai-shi
Copy link
Member Author

dai-shi commented Sep 26, 2024

It should work in v5 and v4 too (v4 doesn't loop infinitely, but still causes extra rerenders without this fix.)
What's important is the selector result being stable.

@wuarmin
Copy link

wuarmin commented Sep 26, 2024

It should work in v5 and v4 too (v4 doesn't loop infinitely, but still causes extra rerenders without this fix.) What's important is the selector result being stable.

Oh, I understand. So following should work too. Right?

const useFilter = () => {
  const filters = useQueryStore(({ filters }) => filters);
  const filterItems = useQueryStore(({ filterItems }) => filterItems);

  return {
    ...filters.reduce((acc, { id, value }) => ({ ...acc, [id]: value }), {}),
    ...filterItems,
  };
};

Thanks

@dbritto-dev
Copy link
Collaborator

@wuarmin yeah, that's the way to solve it. I'll add to learn section a new doc to talk about "How to make the selector result stable"

@cpatti97100
Copy link

Hi just installed zustand here for the first time and got version 5.0.0-rc.2. can I still use the docs at https://zustand.docs.pmnd.rs? thanks!

@dbritto-dev
Copy link
Collaborator

@cpatti97100 sure, we added new docs for APIs and Hooks recently

@dai-shi
Copy link
Member Author

dai-shi commented Oct 14, 2024

Thanks everyone for giving feedback!

https://github.com/pmndrs/zustand/releases/tag/v5.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants