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

What is the correct way to load actions in containers? #24

Closed
louisremi opened this issue Apr 14, 2016 · 10 comments
Closed

What is the correct way to load actions in containers? #24

louisremi opened this issue Apr 14, 2016 · 10 comments
Assignees
Milestone

Comments

@louisremi
Copy link

Hi,

I'm a newbe in react-redux development. I'm starting by trying to recreate the tree-view example of redux using this generator. I have to say the work was fairly easy, until I had to write my first container. I did not have any example of how the generated code might be customized to produce a useful container, so I had to poke around a bit.

The thing that bothers me now is that I'm not sure how to "load" the actions I've created, inside my container. I ended up copying the mapDispatchToProps generated in App.js that lists all actions into my container. But that doesn't look very DRY. So there's probably something I'm missing here, and if not, something that could be improved :-)

Thank you in advance,
Louis-Rémi

@stylesuxx
Copy link
Owner

Unfortunately, yes this is the way to go, if you have several top level containers.
You could of course build your app in a way that the App container is the outer most container, and does the routing to all other containers, in this case the app container could pass down all needed actions to its children which then would not need to map the actions on their own.

However, I am open for suggestions here. If you have an idea how to improve this, please share it.

@louisremi
Copy link
Author

the app container could pass down all needed actions to its children which then would not need to map the actions on their own

I'm guessing my problem is only that I don't know how to do this. I've tried to google it but couldn't find anything useful. May I ask you to point to a relevant doc or example? Thank you in advance : )

@stylesuxx
Copy link
Owner

Well your TextNode container is just a Component, so you can use it like this in the App container:

...

import TextNode from './TextNode';
class App extends Component {
  render() {
    const {actions, nodes} = this.props;
    return <TextNode someAction={actions.someAction} anotherAction={actions.anotherAction} />;
  }
}

...

@louisremi
Copy link
Author

I took example from the Redux tree-view example. My TextNode container is a copy of the Node container in that project. I'm curious to learn why, in my case, TextNode is a component and not a container.

@stylesuxx
Copy link
Owner

Every container is a component. It is just a matter of definition. In Redux context, components that are "smart" and handle their own state are called containers. Components are "dumb or presentational" and do not have their own state and they do not care about what the app actually does, they only care about getting all the props they need.

It is explained more in depth in the redux docs.

@louisremi
Copy link
Author

Wouldn't it be more useful it the react-webpack-redux:action generator added every action to a single file (src/actions/all.js for example) instead of adding them to the App.js container?

@stylesuxx
Copy link
Owner

What would be the benefits of this in your opinion? I have it like that since I can see just by looking at a container what actions it is using and thus quickly know what the container is responsible for.

@louisremi
Copy link
Author

The benefits would be to reduce the verbosity of importing actions in containers. Instead of:

function mapDispatchToProps(dispatch) {
  /* Populated by react-webpack-redux:action */
  const actions = {
    createNode: require('../actions/nodes/createNode.js'),
    deleteNode: require('../actions/nodes/deleteNode.js'),
    addChild: require('../actions/nodes/addChild.js'),
    removeChild: require('../actions/nodes/removeChild.js'),
    createFont: require('../actions/fonts/createFont.js'),
    addFont: require('../actions/fonts/addFont.js'),
    ...
  };
  const actionMap = { actions: bindActionCreators(actions, dispatch) };
  return actionMap;
}

We could have:

import { createNode, deleteNode, addChild, removeChild, createFont, addFont } from './../actions/all';
...
function mapDispatchToProps(dispatch) {
  const actions = {
    createNode,
    deleteNode,
    addChild,
    removeChild,
    createFont,
    addFont,
    ...
  };
  const actionMap = { actions: bindActionCreators(actions, dispatch) };
  return actionMap;
}

@stylesuxx
Copy link
Owner

Totally makes sense - will look into this.

Thank you for the suggestion.

@stylesuxx
Copy link
Owner

Actions are now centrally exported from 'actions/index.js', so importing and mapping as suggested by @louisremi is now possible. Added to develop, will be part of the next release.

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

No branches or pull requests

2 participants