mattkrick / redux-optimistic-ui

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

Throws error with non-root optimistic reducer and Redux initialState #22

Closed marcins closed 7 years ago

marcins commented 7 years ago

For this minimal example:

const optimist = require('./lib/index');
const redux = require('redux');
const usersReducer = (state = {}, action) => {
    return state;
}

const rootReducer = redux.combineReducers({
    users: optimist.optimistic(usersReducer)
});

const store = redux.createStore(rootReducer, {
    users: {
        'foo': {
            name: 'bar'
        }
    }
});

console.log(optimist.ensureState(store.getState()));

I get an error:

TypeError: state.get is not a function
    at [...]/redux-optimistic-ui/lib/index.js:119:29

This is because there are appear to actually be multiple @@redux/INIT actions - the first with state being undefined and the second with the initialState. It appears that when the optimistic reducer runs for this second init, this is when Redux passes in the initialState even though the optimistic reducer thinks it's already ready it has lost the state it setup the first time.

A potential fix appears to be changing the code around line 105 to:

if (!isReady || state === undefined || action.type === '@@redux/INIT') {
      isReady = true
      state = preloadState(reducer(ensureState(state), {}));
}

.. but I still need to verify this doesn't have any adverse side-effects (and write some proper failing tests too).

marcins commented 7 years ago

Ah, after having a look at the existing tests, and especially the 'with redux' test, I think that's what the preloadState function is intended for? That seems a bit clunky to have to explicitly wrap the relevant bits of initial state in preloadState calls - it seems the approach above (checking for action.type to be @@redux/INIT) obviates the need for using preloadState in your initial state.

moimael commented 7 years ago

Any progress on this one ?