developit / unistore

🌶 350b / 650b state container with component actions for Preact & React
https://npm.im/unistore
2.86k stars 139 forks source link

[question] How to best handle chained async actions? #187

Closed lukeundtrug closed 3 years ago

lukeundtrug commented 4 years ago

Hi, we’re using Preact and Unistore in a rather complex app in which we now face some challenges that we don’t quite know how to tackle efficiently. We have a store which is divided in several scopes. All in all a big nested object. We then have divided our actions regarding those scopes and actions are only ever allowed to manipulate one scope. This looks somewhat like this:

// store.js

import createStore from 'unistore';
import { initialState as scope1 } from './actions/scope1';
import { initialState as scope2 } from './actions/scope2';
import { initialState as scope3 } from './actions/scope3';
const store = createStore({
    scope1,
    scope2,
    scope3,
});
export default store;

// scopeX.js -> all respectively scoped action files

export const initialState = {
...
};
const actions = (store) => {
    const __actions = {
        syncAction1: ({scope1}, foo, bar) => {
            return {
                scope1: {
                    ...scope1,
                    key1: foo,
                    key2: bar,
                },
            };
        },
        asyncAction1: async ({scope1}) => {
            const result = await doSomething();
            return {
                scope1: {
                    ...scope1,
                    key3: foo,
                    key4: bar,
                },
            };
        },
    };
    return __actions;
};
export default actions;

Now, difficulties start when dealing with asynchronous code as we might overwrite our scope1-state with a stale state from before the await. A simple solution of course is to use store.getState() instead of using scope1 as it was handed to asyncAction1.

But since we want to minimize actual store writes we started chaining actions which works quite well in synchronous use cases but in asynchronous ones we still face the dilemma that after the async task resolves our state might already have been overwritten by some different action. We also cannot use getState() to fetch the current state as we would then lose the accumulated mutations. An example of a longer chain might look like this:

...
const actions = (store) => {
    const __actions = {
        syncAction1: ({scope1}) => {
            return {
                scope1: {
                    ...scope1,
                    key1: foo,
                    key2: bar,
                },
            };
        },
        syncAction2: (state) => {
            let nextState = __actions.syncAction1(state);
            ...
            nextState = __actions.syncAction3(nextState);
            return {
                scope1: {
                    ...nextState.scope1,
                    key3: foo,
                },
            };
        },
        syncAction3: ({scope1}) => {
            return {
                scope1: {
                    ...scope1,
                    key4: foo,
                },
            };
        },
        asyncAction1: async (state) => {
            let nextState = __actions.syncAction2(state);
            nextState = await __actions.asyncAction2(nextState);
            return {
                scope1: {
                    // we can't use store.getState().scope1 here as we would lose all prior mutations
                    ...scope1,
                    key1: foo,
                    key5: bar,
                },
            };
        },
        asyncAction2: async ({scope1}) => {
            const result = await doSomething();
            return {
                scope1: {
                    // we can't use store.getState().scope1 here as we would lose the changes that might have accumulated
                    // in the state argument supplied to this action
                    ...scope1,
                    key4: result.foo,
                },
            };
        },
    };
    return __actions;
};
...

Has anyone used unistore in a similar fashion and found a general solution to assuring state safety in asynchronous use cases? (except for migrating to another state management solution :slightly_smiling_face: ) One solution would of course be to setState before every await but in our understanding of how preact works this would also introduce a whole lot of needless component re-renders as there might not always have been a mutation to the state so far.

In general, diffing against the current state would help and then deciding whether to setState or not but that is of course extremely costly as it would require deep equality checks of our state tree.

We would be happy if others would share their experience or thoughts on the topic :) Is our way of chaining actions inherently flawed?

lukeundtrug commented 4 years ago

For anyone interested: We played around in a code sandbox and came up with a helper function that suites our purposes. It effectively persists state in asynchronous actions before the actual asynchronous task begins.

It returns an updated state object or store.getState() for the calling function to work on depending on whether it is wrapped around an action or a simple async task like a fetch. Thereby, it ensures that every action always has the most current state available to keep doing its mutations.

Have a look, if you like :) https://codesandbox.io/s/unistore-demo-ghmyy?file=/index.js

developit commented 4 years ago

Hey @lukeundtrug - sorry for not replying quicker. This is an interesting scenario. Personally I've always just allowed my async actions to mutate state, and worried about update performance as a later optimization concern.

As a bit of context, updates in Preact (including those triggered by unistore) are always batched, so any updates that happen in the same "tick" (one Promise resolution) get collapsed into a single render. That does mean that very fast but asynchronous back-to-back updates will produce intermediary renders, which I gather from your comments is something you're needing to avoid.

I guess I'd split up my recommendations here into two: first, I've tended to experience this issue much less when I move some of my less state-driven logic out of actions and into standalone methods. Rather than having an action that consumes all state and produces an update, I might intentionally use a pure function that expects specific arguments and returns only the value(s) it produced itself. This doesn't work in 100% of cases, but it does cut down on the number of cases where asynchronous actions operate on shared state.

The second would be something along the lines of what you arrived at in your Codesandbox:

const ACTIONS = (store, actions) => (actions = {
  async load(state) {
    store.setState({ items: [] });
    // example of calling another action through unistore:
    await store.action(actions.fetch)('/items.json');
    return { items };
  },
  async fetch(state, url) {
    const newItems = await (await fetch(url)).json();
    const { items } = store.getState();
    return { items: items.concat(newItems) };
  }
});
lukeundtrug commented 4 years ago

Hi @developit, thanks for your reply :) I guess the intermediary renders that we experience are mostly an issue during startup. It might not even be that big of an issue, it just felt that since we didn't have a proper pattern that we followed in terms of async tasks, that, in effect, we also didn't have complete control or knowledge over which state updates got fired and where they originated.

Since we didn't have a proper best-practice established in our team we ended up stepping away from actually returning mutated state objects and instead used setState() way more often than necessary. That made the whole setup quite complex to follow, also when debugging, as some components got updated multiple times a second.

Our approach with a helper function hopefully leads to a standardized way of coding in our store. The optimization of renders is just something that comes along the way, I guess.

Thanks for clarifying on the rendering topic that's also what I realized but still wasn't quite sure about when playing around in the sandbox.

Thanks for your recommendations, especially following stricter functional programming guidelines and really engaging in function composition, which I think is what it boils down to, is definitely something of general importance that might help reduce complexity in the first place 👍

lukeundtrug commented 4 years ago

Or maybe I'll leave the question open for others who might be interested in the topic. Feel free to close it, if you feel like it, though 👍 Thanks again!