reduxjs / redux

A JS library for predictable global state management
https://redux.js.org
MIT License
60.88k stars 15.27k forks source link

How to create nested reducers? #316

Closed michalkvasnicak closed 9 years ago

michalkvasnicak commented 9 years ago

Maybe I am missing something but how can I define reducer that will handle actions for instance of some resource which is stored in parent.

Simple example:

const initialState = {
  articles: {
    total: 1,
    articles: [{ id: 1, /* some article props */}]
  }
}

And I want to increase views for the article with id 1 but I don't want to have all logic in articles store.

store.dispatch({ type: 'ARTICLE_VIEW', id: 1 });

I know I can handle it in articles store, but can it be done in the way I described? That I can dynamically define reducers and not only in one level but nested?

sergey-lapin commented 9 years ago

@michalkvasnicak You can try this https://github.com/lapanoid/redux-delegator works for me :)

gaearon commented 9 years ago

Can you give a slightly larger example? I'd write some code but because articles is all there is, I can't explain how to do nesting (and whether it's a good idea :-). An actually nested example would help here.

michalkvasnicak commented 9 years ago

I don't know if this help but I work on simple testing tool.

I have test system state reducer:

import { START, FINISH } from '../constants/systemActionsTypes';
import { TEST_PASSED, TEST_FAILED, TEST_TIMED_OUT } from '../constants/testsActionsTypes';

const initialState = {
    state: 'initial',
    total: 0,
    nextTest: 0,
    passed: 0,
    failed: 0,
    timedout: 0,
    duration: null
};

export default function system(state = initialState, action) {
    switch (action.type) {
        case START:
            return {
                ...state,
                total: action.total
            };
        case FINISH:
            return {
                ...state,
                state: 'finished',
                duration: action.duration
            };
        case TEST_PASSED:
            return {
                ...state,
                nextTest: state.nextTest + 1,
                passed: state.passed + 1
            };
        case TEST_TIMED_OUT:
            return {
                ...state,
                failed: state.failed + 1,
                nextTest: state.nextTest + 1,
                timedout: state.timedout + 1
            };
        case TEST_FAILED:
            return {
                ...state,
                nextTest: state.nextTest + 1,
                failed: state.failed + 1
            };
        default:
            return state;
    }
}

And I have tests reducer:

import {
    TEST_REGISTERED, TEST_STARTED, TEST_PASSED, TEST_FAILED, TEST_TIMED_OUT
} from '../constants/testsActionsTypes';

export default function tests(state = [], action) {
    let sliced;
    let test;

    switch (action.type) {
        case TEST_REGISTERED:
            return state.slice().push({
                number: action.number,
                description: action.description
            });
        case TEST_STARTED:
            sliced = state.slice();
            test = sliced[action.number - 1];
            test.state = 'started';

            return sliced;
        case TEST_PASSED:
            sliced = state.slice();
            test = sliced[action.number - 1];
            test.state = 'passed';

            return sliced;
        case TEST_FAILED:
            sliced = state.slice();
            test = sliced[action.number - 1];
            test.state = 'failed';
            test.error = action.error

            return sliced;
        case TEST_TIMED_OUT:
            sliced = state.slice();
            test = sliced[action.number - 1];
            test.state = 'timed_out';
            test.error = action.error;

            return sliced;
        default:
            return state;
    }
}

What I want is to have a reducer for given test so I don't need to do this array things but every test will have its own reducer so if test number is equal, it will change its state. And ideally to have tests reducer as child of system reducer too.

this code is mess, I know

sergey-lapin commented 9 years ago

You can compose it in such way

function test(test = [], action) {

  switch (action.type) {
    case TEST_STARTED:
      test.state = 'started';
      return test;
    case TEST_PASSED:
      test.state = 'passed';
      return test;
    case TEST_FAILED:
      test.state = 'failed';
      test.error = action.error
      return test;
    case TEST_TIMED_OUT: 
      test.state = 'timed_out';
      test.error = action.error;
      return test;
    default:
      return test;
  }
}

export default function tests(state = [], action) {
  switch (action.type) {
    case TEST_REGISTERED:
      return state.slice().push({
        number: action.number,
        description: action.description
      });
    case TEST_STARTED:
    case TEST_PASSED:
    case TEST_FAILED:
    case TEST_TIMED_OUT:
      const sliced = state.slice();
      sliced[action.number - 1] = test(sliced[action.number - 1], action)
      state.setSlice(sliced) // I guess..
      return state;
    default:
      return state;
  }
}
michalkvasnicak commented 9 years ago

Why I didn't think of this? :) Thank you, but are there any other possible implementations? Because now I need to keep a fall-through for actions just to dispatch them to test reducer.

sergey-lapin commented 9 years ago

Try https://github.com/acdlite/redux-actions#handleactiontype-reducer--reducermap, this can reduce some boilerplate but still it did not solve "carring actions" problem. I build https://github.com/lapanoid/redux-delegator for that purpose, but it sticked to immutable js currently, do not have time to fix this:(

gaearon commented 9 years ago

I'll take a look at this later. One single thing I want to say for now is that you should try hard to keep your data normalized and, if possible, flat.

For example, if you have a list of reorderable articles, and each article has an author, your data shape shouldn't be { articles: [{ name, author }, ... ] }. It should be { authorsById: { id -> author, ... }, articlesById: { id -> { name, authorId }, ... }, articles: [articleId, ... ] }.

See also https://github.com/gaearon/normalizr elaborating on this approach and implementing such normalization for nested API responses.

michalkvasnicak commented 9 years ago

Yes I know, normally I would store them in hash id => object. I can do it now too but it doesn't answer my question :)

michalkvasnicak commented 9 years ago

I just don't want to have all logic in single reducer if it is possible.

gaearon commented 9 years ago

The point is that, if you keep data normalized, you can use combineReducers to just split the reducer into smaller reducers managing articlesById, authorsById, articles separately because each needs to be changed by a different set of actions. Moreover you may find it possible to create reducer factories that create reducers managing objects of a particular shape responding to a particular set of actions.

michalkvasnicak commented 9 years ago

Ok so instead of nesting the data I should just keep flat structure and for example create new test reducer just for an test of given number?

const state = {
   system: { state: 'finished', total: 2, passed: 2},
   tests: [/* array of tests e.g. for rendering of lists*/],
   test1: { state: 'passed', duration: 100 /* */ },
   test2: { /* .... */}
}
gaearon commented 9 years ago

I'll take a look later. If you could share a sample project to make sure I can run the code and play with it that would help.

michalkvasnicak commented 9 years ago

Here is project where I want to use it (now it uses only code from above). https://github.com/michalkvasnicak/prever/tree/develop

It is dev version, not refactored, tests are missing, etc.

eelkeh commented 9 years ago

I'm struggling with a similar case, normalized stores do help with fetching deeply nested data, but it gets tricky after the merged objects are part of the store. Then, the nested authors have replaced the flat list of reference id's on the article, so now the objects passing through the article reducers don't have the normalized shape anymore... not sure how to deal with this.

gaearon commented 9 years ago

I'm struggling with a similar case, normalized stores do help with fetching deeply nested data, but it gets tricky after the merged objects are part of the store.

I don't think I understand. Can you describe the shape of your state before and after? You should keep everything normalized in the state. Even after fetching.

eelkeh commented 9 years ago

You should keep everything normalized in the state. Even after fetching.

When the reducer returns something like mergeInto(articles, authors) the state isn't normalized anymore, so I guess I'm not sure where to merge the states so a component can make use of it.

Can you describe the shape of your state before and after?

Before [id, id, ...] and after [{author...}, {author...}]

gaearon commented 9 years ago

@eelkeh

We're probably talking about different things. Please show your data shape before and after. Do these ideas help? https://github.com/gaearon/flux-react-router-example#how-i-classify-stores

eelkeh commented 9 years ago

Will take a look, thanks! Updated above comment with data shape.

gaearon commented 9 years ago

Oh, okay, then my musings there should help. Don't remove the IDs. Store IDs and articles separately. (You can even use different reducers.)

gaearon commented 9 years ago

Your data shape should be

{
  feed: [id, id, ...],
  articlesById: { id -> article, id -> article, ... }
}

Similarly, for nested objects:

{
  feed: [id, id, ...],
  articlesById: { id -> article: { id, name, authorId }, id -> article: { id, name, authorId }, ... },
  authorsById: { id -> author, id -> author, ... }
}
rrag commented 9 years ago

For stores which have arrays in them, like so

{
  rows: [
    { id:0, height:100, width, 100, foo: "bar" },
    { id:1, height:200, width, 200, foo: "baz" },
    { id:2, height:300, width, 300, foo: "foobar" },
  ]
}

the action is dispatched as resize(index, newHeight, newWidth)

Is this the recommendation? or is there a way the resize action can avoid the index parameter?

gaearon commented 9 years ago

@rrag

I'd do it like this:

{
  rowsOrder: [0, 1, 2],
  rowsById: {
    0: { id:0, height:100, width, 100, foo: "bar" },
    1: { id:1, height:200, width, 200, foo: "baz" },
    2: { id:2, height:300, width, 300, foo: "foobar" },
  }
}

Then you can always pass id in actions and grab it from state.rowsById[action.id] for modification. When you need to implement re-ordering, you just reorder IDs inside state.rowsOrder.

eelkeh commented 9 years ago

Thanks, that project helped a lot, from: https://github.com/gaearon/flux-react-router-example/blob/master/scripts/pages/UserPage.js#L36

function getState(props) {
  ...
  const starredOwners = starred.map(repo => UserStore.get(repo.owner));

In redux, should that logic be part of @connect at the top level component for example?

gaearon commented 9 years ago

@eelkeh

I'd write a selector function with reselect and give it to @connect.

eelkeh commented 9 years ago

@gaearon Exactly what I was looking for, thanks!

rrag commented 9 years ago

any way I can avoid sending the id into the action? nothing wrong with that way, but just curious.

gaearon commented 9 years ago

@rrag

Why avoid it? You want to resize a specific row. The reducer can't guess which one. It makes sense that id is part of the action. What am I missing?

rrag commented 9 years ago

good question. nothing wrong and you are not missing anything also :)

Store shape

{
  rows: [
    { id:0, height:100, width, 100, query: "bar", results: [ ... ], showMenu: true },
    { id:1, height:200, width, 200, query: "", results: [], showMenu: false },
    { id:2, height:300, width, 300, query: "", results: [], showMenu: false },
  ]
}

Table.jsx

render() {
  return (
    <div>
      {this.props.rows.map(each => <Row ...each {...bindActionCreators()}/>}
    </div>
  );
}

Actions.js

...
export function resize(id, width, height) {
...
export function search(id, query) {
...
export function showDropDown(id) {
...
export function clearSearch(id) {
...

Row.jsx

render() {
  var actions = bindActionCreators(Actions, this.props.dispatch);
  return (
    <Resizeable width={this.props.width} height={this.props.height} onResize={actions.resize} id={this.props.id}>
      <DropDownComponent showMenu={this.props.showMenu} onClick={actions.showDropDown}  id={this.props.id} />
      <SearchBar query={this.props.query} onSearch={actions.search} id={this.props.id} />
      <SearchResults results={this.props.results} onClear={actions.clearSearch} id={this.props.id} />
    </Resizeable>
  )
}

if you see the Row I end up passing the id to all its children, and each of its children need to know the id, and the actions on the children of Row have to call the action with the id and anything else relevant.

Now I have leaked the id into SearchBar which does not need to know the id

I saw the TODO example and the way it handles arrays and I convinced myself this is the right way, but now I had to use SearchBar outside of the array and it does not make sense for it to have an id

leoasis commented 9 years ago

@rrag you don't need to pass id on those, just prebind the actionCreators with the id from this.props.id here and send that prebound function as a callback prop. Or use a closure:

<SearchBar query={this.props.query} onSearch={actions.search.bind(null, this.props.id)} />
// or
<SearchBar query={this.props.query} onSearch={query => actions.search(this.props.id, query)} />
rrag commented 9 years ago

(facepalm)

@leoasis @gaearon Thank you

gaearon commented 9 years ago

Async Actions example shows how network request state reducer can be dynamically used. Real world example goes further by demonstrating how you can create a reducer factory for reusable reducers.

I'm closing because these are the patterns solving the problem described in this issue.

dey-dey commented 7 years ago

@gaearon for posterity, the current Async Actions link :)

sanbornhilland commented 6 years ago

...and current reducer factory link