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

clunky combine store usage #291

Closed
jgibo opened this issue Jan 29, 2021 · 7 comments
Closed

clunky combine store usage #291

jgibo opened this issue Jan 29, 2021 · 7 comments

Comments

@jgibo
Copy link

jgibo commented Jan 29, 2021

I've followed the recommended way to combine stores in #228. It's clunky to work with as inside the stores you have to set state as if you're setting from the root of the state, this makes everything overly verbose. I imagine in many cases sub-stores won't touch other stores state and having one root store would mainly be for simplifying co-dependent state queries outside of the stores.

Take a root store made up of sub-stores like this:

const useStore = create((set, get, api) => ({
  users: { ...createUsersStore(set, get, api) },
  restrictedUsers: { ...createRestrictedUsersStore(set, get, api) }
}));

Inside the sub-stores, it'd be intuitive to set state like this:

const createUsersStore = (set, get, api) => ({
  data: [],
  setUsers: users => set({data: users})
})

Example:

// expected state after calling useStore.getState().users.setUsers(['user1', 'user2']) :
{
  users: {
    data: ['user1', 'user2']
  },
  restrictedUsers: {...}
}

// instead, it looks like this:
{
  data: ['user1', 'user2'],
  users: {
    data: []
  },
  restrictedUsers: {...}
}

To fix this issue, you need to prepend all set object params with the sub-store key, and spread the current values:

const createUsersStore = (set, get, api) => ({
  data: [],
  setUsers: users => set({users: {...get().users, data: users}})
})

This feels like unnecessary ceremony. Immer could be a solution, but I like to have my async logic contained in store actions; Immer doesn't like async. The solution I've come to is a helper like this:

const _setIn = (set, get, propName) => setObjArg => set({ [propName]: { ...get()[propName], ...setObjArg } });

This helper can then be passed down into the stores

const useStore = create((set, get, api) => {
  const setIn = curry(_setIn)(set, get);
  return {
    users: { ...createUsersStore(set, get, api, setIn("users")) },
    restrictedUsers: {...createRestrictedUsersStore(set, get, api, setIn("restrictedUsers"))}
  };
});

How setting now looks, and works intuitively:

const createUsersStore = (set, get, api, setIn) => ({
  data: [],
  setUsers: users => setIn({data: users})
})

This works well, but only supports set(obj); no support for set(state => ...). Is there an alternative solution (or modification to mine) that matches this, and ideally doesn't limit the api of set. I feel a solution to this should be documented as in real world apps it's quite common to have co-dependent stores, and having one global store simplifies the application logic for these co-dependencies (imo).

Zustand is fantastic, I want to harness the full power of it without introducing ridiculous ceremony like Redux.

@jgibo jgibo changed the title Clunky combine store usage clunky combine store usage Jan 29, 2021
@dai-shi
Copy link
Member

dai-shi commented Jan 29, 2021

Hey, I actually agree with this problem.
We have some discussions previously: #161, #163, #178

Unfortunately, it didn't reach a convincing good pattern.
So far, using immer seems the only pattern to explain easily.

Immer doesn't like async.

But, I didn't know this use case.


I'm happy to discuss if you or someone could take the lead on this topic.

@jgibo
Copy link
Author

jgibo commented Jan 30, 2021

I will take a look through those other issues, cheers.

Immer doesn't like async: Bit of a bad take on my part. While it does support async, I think it isn't possible to set state incrementally inside the producer - new state is created when producer exits, for async this is when the promise resolves. I may be wrong here, I'll need to double check...

Assuming that's right ^, if you had a async fetch action in the store, and wanted to set isFetching before and after the network call, and for store listeners to pickup on the incremental updates, this wouldn't be possible in the same action.

By no means an expert on state management patterns, but happy to help keep the conversation going and contribute where I can.

@jgibo
Copy link
Author

jgibo commented Jan 31, 2021

There's some good solutions in those issues you mentioned above. I've settled on using your scopedSet solution from Issue 161, for anyone else looking to use this solution, I had to wrap and spread the ternary expression to get it to work nicely:

const scopedSet = (ns, set) => (update) => {
    set(prev => ({
      [ns]: { ...prev[ns], ...(typeof update === 'function' ? update(prev[ns]) : update) }
    }))
}

I'm using it by passing the scoped setter as an extra arg into the stores:

const useStore = create(devtools((set, get, api) => ({
    users: createUsersStore(set, get, api, scopedSet('users', set)),
    restrictedUsers: createRestrictedUsersStore(set, get, api, scopedSet('restrictedUsers', set))
})));

export const createUsersStore = (set, get, api, scopedSet) => ({
    isFetching: false,
    lastFetched: 0,
    data: [],

    fetch: async () => {
        scopedSet(state => ({isFetching: !state.isFetching})); // state updater fn scopes as well
        const data = await usersApi.getUsers();
        scopedSet({data, isFetching: false, lastFetched: Date.now()});
        return data;
    }
});

#161 has alternative patterns for using scopedSet.

Not sure I can contribute much else than has already been discussed in the other issues. The integrated solution in #178 looks promising, I would use it if it was a part of Zustand, but the simple solution above solves my problem just fine.

@jgibo jgibo closed this as completed Jan 31, 2021
@dai-shi
Copy link
Member

dai-shi commented Jan 31, 2021

That's good to hear.

Some possible improvements for any volunteers would be:

  1. document scopedSet usage in readme.md
  2. continue and investigate the idea in Namespacing / slicing application state alike Redux' combineReducers #178 and create a new utility function in middleware.ts

@lautin0
Copy link

lautin0 commented Feb 3, 2021

This is very close to what I want but I am using TypeScript. Unfortunately, I got some error with this approach. How can I achieve this outcome in TypeScript?

Here is the error,

Sub-store:

Screenshot 2021-02-03 at 10 16 23 PM

Main store:

Screenshot 2021-02-03 at 10 16 36 PM

tyvm!

@dai-shi
Copy link
Member

dai-shi commented Feb 3, 2021

because it's not scoped? #161 might help, but i'm not so sure.

@andreicimpoesu
Copy link

andreicimpoesu commented Oct 3, 2024

What about?

import { create } from "zustand";
import "./styles.css";

const mockPromise = () =>
  new Promise<{ totalBooks: number }>((resolve) =>
    setTimeout(() => resolve({ totalBooks: 1000 }), 2000)
  );

interface IAsyncDataLoader<T> {
  data: T | null;
  isLoading: boolean;
  loadData: (asyncFn: () => Promise<T>) => Promise<T | undefined>;
}

// TODO: caching for loadData() params & force
const asyncDataLoader = (staticStoreKey: string, set: any, get: any) => {
  if (typeof staticStoreKey !== "string") {
    throw new Error('missing asyncdDataLoader("staticStoreKey", ...)');
  }

  return {
    loadData: async (asyncFn: () => Promise<any>) => {
      set((state: any) => ({
        ...state, // TODO: i think it's already merged
        [staticStoreKey]: {
          ...get()[staticStoreKey],
          isLoading: true,
          data: null,
        },
      }));

      const asyncFnData = await asyncFn();

      set((state: any) => ({
        ...state,
        [staticStoreKey]: {
          ...get()[staticStoreKey],
          isLoading: false,
          data: asyncFnData,
        },
      }));

      return asyncFnData;
    },

    data: null,
    isLoading: false,
  };
};

interface IBooksStore {
  syncTotalBooks: number;
  syncIncreaseTotalBooks: (value: number) => void;
  asyncBooksData: IAsyncDataLoader<{ totalBooks: number }>;
}

const useBooksStore = create<IBooksStore>((set, get) => {
  return {
    syncTotalBooks: 0,
    syncIncreaseTotalBooks: (value) =>
      set((state) => ({
        ...state,
        syncTotalBooks: state.syncTotalBooks + value,
      })),
    ["asyncBooksData"]: asyncDataLoader("asyncBooksData", set, get),
  };
});

export default function App() {
  const totalBooks = useBooksStore((store) => store.syncTotalBooks);
  const increaseTotalBooks = useBooksStore(
    (store) => store.syncIncreaseTotalBooks
  );
  const booksData = useBooksStore((store) => store.asyncBooksData);

  const handleLoadBooksBtnClick = () => {
    booksData.loadData(async () => await mockPromise());
  };

  return (
    <>
      <div className="App">
        <h1>Hello CodeSandbox</h1>
        <h2>Start editing to see some magic happen!</h2>
      </div>

      <div>
        <h3>Total nr. of books sync: {totalBooks}</h3>

        <button onClick={() => increaseTotalBooks(2)}>Add two more</button>

        <h3>
          Total nr. of books async:{" "}
          {booksData.isLoading ? "Loading" : booksData.data?.totalBooks ?? "0"}
        </h3>

        <button onClick={handleLoadBooksBtnClick}>Load books</button>
      </div>
    </>
  );
}

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

4 participants