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

Added support for createStore to accept a reducer function #12

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mattcgenui
Copy link
Contributor

@mattcgenui mattcgenui commented Mar 21, 2022

No description provided.

@mattcgenui mattcgenui closed this Mar 22, 2022
@mattcgenui mattcgenui reopened this Mar 22, 2022
@mattcgenui mattcgenui marked this pull request as ready for review March 22, 2022 17:39
@shanebdavis
Copy link
Member

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
Copy link
Contributor Author

mattcgenui commented Mar 22, 2022

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
Copy link
Member

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?

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

Successfully merging this pull request may close these issues.

3 participants