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

How to create nested reducers? #316

Closed
michalkvasnicak opened this issue Jul 24, 2015 · 33 comments
Closed

How to create nested reducers? #316

michalkvasnicak opened this issue Jul 24, 2015 · 33 comments
Labels

Comments

@michalkvasnicak
Copy link
Contributor

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

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

@gaearon
Copy link
Contributor

gaearon commented Jul 24, 2015

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

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

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

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

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

gaearon commented Jul 24, 2015

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

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

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

@gaearon
Copy link
Contributor

gaearon commented Jul 24, 2015

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

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

gaearon commented Jul 24, 2015

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

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

eelkeh commented Jul 29, 2015

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

gaearon commented Jul 29, 2015

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

eelkeh commented Jul 29, 2015

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

gaearon commented Jul 29, 2015

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

eelkeh commented Jul 29, 2015

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

@gaearon
Copy link
Contributor

gaearon commented Jul 29, 2015

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

gaearon commented Jul 29, 2015

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

rrag commented Jul 29, 2015

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

gaearon commented Jul 29, 2015

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

eelkeh commented Jul 29, 2015

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

gaearon commented Jul 29, 2015

@eelkeh

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

@eelkeh
Copy link

eelkeh commented Jul 29, 2015

@gaearon
Exactly what I was looking for, thanks!

@rrag
Copy link

rrag commented Jul 29, 2015

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

@gaearon
Copy link
Contributor

gaearon commented Jul 29, 2015

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

rrag commented Jul 29, 2015

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

leoasis commented Jul 29, 2015

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

rrag commented Jul 29, 2015

(facepalm)

@leoasis @gaearon Thank you

@gaearon
Copy link
Contributor

gaearon commented Aug 14, 2015

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

dey-dey commented Nov 2, 2016

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

@sanbornhilland
Copy link

...and current reducer factory link

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

No branches or pull requests

8 participants