generalui / hooks-for-redux

modular redux - in half the code
MIT License
96 stars 13 forks source link

Added support for createStore to accept a reducer function #12

Open mattcgenui opened 2 years ago

shanebdavis commented 2 years ago

I could be wrong, but I think there is an easier way to do this. Reducers can be chained. If the reducer passed in is a function, we can just use logic something like this:

// (initialReducer = {}, ...args) =>
const combinedReducer = combineReducers(reducers)
const overallReducer = (state, action) => initialReducer(combinedReducer(state, action), action)
store.replaceReducer(overallReducer)
mattcgenui commented 2 years ago

This should work with a regular reducer function. However, if the initialReducer is a combined reducer, this will throw a UnexpectedStateShapeWarningMessage since we send a state with extra slices. That why we added a workaround if initialReducer.name === 'combination'. Does that make sense?

shanebdavis commented 2 years ago

Interesting. I didn't know about UnexpectedStateShapeWarning. It's unfortunate to have to end-run a false warning message with some fairly runtime heavy code.

Looking at your overall implementation, I'm concerned about the performance of the rootReducers. They do a lot of work every time an action is dispatched that could be cached. The actual work in the rootReducer should be as light as possible.

Example:

// these never change, but currently they will be executed for every dispatched action
const initialSlices = initialReducer ? initialReducer({}, '') : {}
const initialKeys = Object.keys(initialSlices)
const knownKeys = Object.keys(reducers)

Are there any other opportunities to move any for loops, Object.keys calls or calls like hasOwnProperty out of the run-every-dispatched-action loop of the rootReducer?