Skip to content
This repository has been archived by the owner on Oct 26, 2018. It is now read-only.

Include any previous state in routeReducer #183

Merged
merged 1 commit into from
Jan 12, 2016
Merged

Include any previous state in routeReducer #183

merged 1 commit into from
Jan 12, 2016

Conversation

timdorr
Copy link
Member

@timdorr timdorr commented Jan 12, 2016

This was wiping out state when not using combineReducers or reducing this reducer with others.

@timdorr
Copy link
Member Author

timdorr commented Jan 12, 2016

I'm going to be a big jerk and merge this in without confirmation, given it's a minor (but important!) change.

timdorr added a commit that referenced this pull request Jan 12, 2016
Include any previous state in routeReducer
@timdorr timdorr merged commit c02d693 into master Jan 12, 2016
@timdorr timdorr deleted the missing-state branch January 12, 2016 17:06
@jlongster
Copy link
Member

Care to explain more? What kind of situation does it wipe out state? The reducer should only be concerned about only its own state.

@timdorr
Copy link
Member Author

timdorr commented Jan 12, 2016

A reducer in Redux gets all previous state and returns the new state object. This only doesn't apply when using combineReducers, which isolates the reducer to one top level key on state. Remember, there is ultimately only one reducer function run by dispatch, it's up to the user to break that out however it makes sense.

I use reduce-reducers to give my reducers access to the whole state for interdependant bits (namely nested resources). If I put routeReducer as the last reducer in the chain, I would end up with only the location data in my state and nothing else.

@jlongster
Copy link
Member

Ok, I'm fine with this code, but I'm surprised that other projects do that... Having state that's dependent on other state is discouraged. Usually the contract is you take in state you care about, and return state you care about.

@timdorr
Copy link
Member Author

timdorr commented Jan 12, 2016

Sorry, I didn't mean that it's dependent, just that I have actions that affect other parts of state than one single top-level key. I get back nested resources and need to put them into my state in multiple places.

But that is actually incorrect. The reducer function takes in the full state. It's only the optional helper function combineReducers that isolates reducers.

It took me some time to figure out this myself when I skipped ahead to combineReducers without writing a simple reducer first. It's probably one of the more confusing things provided by Redux IMHO.

@jlongster
Copy link
Member

Sure, but combineReducers makes something where all actions are broadcasted to all "sub" reducers. You should have no problem updating multiple parts of your state from one action.

I know that there only exists 1 reducer and it just takes the whole state from the store. I don't see why you would have problems updating various parts of the state, I do that all the time.

But I digress; this change is fine.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants