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

Another CombineReducers method #397

Closed
EddyP23 opened this issue Apr 20, 2017 · 5 comments
Closed

Another CombineReducers method #397

EddyP23 opened this issue Apr 20, 2017 · 5 comments

Comments

@EddyP23
Copy link

EddyP23 commented Apr 20, 2017

This is a proposal for enhancement.

There is a combineReducers function in utils that combines reducers that act upon elements in the object itself.

Say I have the following object:

let StoreState = {
param1: string,
param2: string
}

and then I have two reducers that act on param1 and param2 respectively.

What I am looking at is having a different way of combining reducers that could combine a reducer that acts upon the whole StoreState object itself together with reducers that act on param1 and param2 separately.

We have some actions that want to manage state of the whole StoreState objects and some that need manage sub-states. We could solve this using effects but it feels that it would over-complicate the design.

@kylecordes
Copy link
Contributor

The output of combineReducers is a function. You can wrap another function around that, which then serves as a reducer for the entire combined state. Is there a need for a utility to help do so? It already seems pretty trivial.

(One of the common misuse patterns we see is Effects that only consume actions and emit other actions to affect the state - without any interaction with the outside world. Such logic is better and more simply implemented in a reducer of course.)

@EddyP23
Copy link
Author

EddyP23 commented Apr 20, 2017

@kylecordes thanks for the swift reply and an overall comment to the problem. :)

I would think that there is a place for it in the utils as it seems to be a pretty common task. Why don't you like adding that to the utils?

@kylecordes
Copy link
Contributor

@EddyP23 I don't see how there is anything you could add to utils that would make it any easier though. You just write a function which delegates to the output of combineReducers before or after doing its own overall reduction work.

However, this should probably be visited again once the exciting new version ships with support in the box for lazy loaded additional reducers. Once that is in place, it probably would be helpful to have some explicit, named way to hand the machinery an overall reducer.

@EddyP23
Copy link
Author

EddyP23 commented Apr 21, 2017

@kylecordes Yeah, I think I get what you mean know.

The remaining issue we have is that if there are some properties that child reducers don't act on, they will get removed in a new state construction, in line from utils:

return hasChanged ? nextState : state;

What we have now in our utils 'fork' is:

return hasChanged ? Object.assign({}, state, nextChildState) : state;

This way we leave all properties of state that are not touched by child reducers the way they are and our parent reducer looks like this:

export function parentReducer(state = initialState, action: any): ParentState {
    if (action.type === SomeAction) {
        // do whatever
    }
    return combineReducers(state, action);
}

Maybe this is something you think of including in the utils.ts?

@robwormald
Copy link
Contributor

Please check this against NgRx v4, and if it’s still an issue, please reopen on https://github.com/ngrx/platform. Thanks!

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

No branches or pull requests

3 participants