mattkrick / redux-optimistic-ui

a reducer enhancer to enable type-agnostic optimistic updates
MIT License
693 stars 36 forks source link

Doesn't work with server-sider rendering #16

Closed clayne11 closed 7 years ago

clayne11 commented 7 years ago

I'm trying to use redux-optimistic-ui with server-side rendering. The issue is that redux-optimistic-ui thinks that the state is already initialized when it in fact isn't.

Redux performs two passes to initialize the reducer and test to ensure that your reducer doesn't listen for @@redux actions. What this means is the the isReady flag is set before the state is initialized.

Since I'm rehydrating my data from the server, the initial state is already set (it's NOT undefined), and so it doesn't get wrapped by redux-optimistic-ui since the isReady flag is also set from the redux initialization passes.

mattkrick commented 7 years ago

i'm not quite sure i understand. Here's an example of it using SSR though: https://github.com/mattkrick/meatier

clayne11 commented 7 years ago

You're not making the reducer optimistic when the app starts. You're doing it asynchronously after you've already loaded the reducer and gone through the initialization.

Additionally, you're making the entire reducer optimistic, not one of the sub-reducers which I am, so there's no easy way to add it after you've already configured the reducer.

When I bootstrap my app, I wire up all of the reducers in one file, making one of the sub-reducers optimistic. The initialization actions in redux are firing and passing in undefined for the state and setting the isReady flag to true. When the @@init action finally comes through, the initial state is already set from the server, however it hasn't been made optimistic yet. Since the isReady flag is already true though and the state is not undefined, redux-optimistic-ui is not running the lines to set up the state.

We could fix this issue by adding a property to the optimistic state for isReady. Rather than checking flag outside of the closure for isReady we check a property on the state itself.

const isInitialized  = (state, readyValue) => (
  state !== undefined &&
  typeof state.get === 'function' &&
  state.get('isReady') === readyValue
);

export const optimistic = (reducer, rawConfig = {}) => {
  const config = Object.assign({
    maxHistory: 100
  }, rawConfig);
  const readyValue = Math.random()

  return (state, action) => {
    if (!isInitialized(state, readyValue)) {
      state = Map({
        history: List(),
        current: reducer(ensureState(state), {}),
        beforeState: undefined,
        isReady: readyValue
      });
    }
    const historySize = state.get('history').size;
    const {type, id} = (action.meta && action.meta.optimistic) || {};

    // a historySize means there is at least 1 outstanding fetch
    if (historySize) {
      if (type !== COMMIT && type !== REVERT) {
        if (historySize > config.maxHistory) {
          console.error(`@@optimist: Possible memory leak detected.
                  Verify all actions result in a commit or revert and
                  don't use optimistic-UI for long-running server fetches`);
        }
        // if it's a BEGIN but we already have a historySize, treat it like a non-opt
        return state.withMutations(mutState => {
          mutState
            .set('history', state.get('history').push(action))
            .set('current', reducer(state.get('current'), action))
        });
      }
      // for resolutions, remove the id so it's not treated like an optimistic action
      action.meta.optimistic.id = undefined;

      // include the resolution in the history & current state
      const nextState = state.withMutations(mutState => {
        mutState
          .set('history', state.get('history').push(action))
          .set('current', reducer(state.get('current'), action))
      });

      const applyFunc = type === COMMIT ? applyCommit : applyRevert;
      return applyFunc(nextState, id, reducer);
    }
    // create a beforeState since one doesn't already exist
    if (type === BEGIN) {
      return state.withMutations(mutState => {
        mutState
          .set('history', state.get('history').push(action))
          .set('current', reducer(state.get('current'), action))
          .set('beforeState', state.get('current'))
      });
    }

    // standard action escape
    return state.set('current', reducer(state.get('current'), action));
  };
};
bhj commented 7 years ago

Hi @mattkrick, I'm not using SSR or async routes/reducers but is anything special needed when using a sub-reducer like this? When wrapped in optimistic its initial state comes back undefined, though everything works when I wrap combineReducers itself.

mattkrick commented 7 years ago

@bhj i'd need to see some code. if you're loading up some existing state, you'll wanna use preloadState

bhj commented 7 years ago

@mattkrick I forgot to ensureState(state.subreducer) instead of just ensureState(state). Of course ensureState isn't magic. 🤓🌴 Thanks for the help and your work!