26
votes

I'm trying to refactor my app to separate presentational and container components. My container components are just the presentational components wrapped in connect() calls from react-redux, which map state and action creators to the presentational components' props.

todo-list.container.js

import React, {Component} from 'react';
import {connect} from 'react-redux';

import {fetchTodos} from '../actions/todo.actions';
import TodoList from '../components/todo-list.component';

export default connect(({todo}) => ({state: {todo}}), {fetchTodos})(TodoList);

todo-list.component.jsx

import React, {Component} from 'react';

import TodoContainer from '../containers/todo.container';

export default class TodoList extends Component {
    componentDidMount () {
        this.props.fetchTodos();
    }

    render () {
        const todoState = this.props.state.todo;

        return (
            <ul className="list-unstyled todo-list">
                {todoState.order.map(id => {
                    const todo = todoState.todos[id];
                    return <li key={todo.id}><TodoContainer todo={todo} /></li>;
                })}
            </ul>
        );
    }
};

todo.container.js

import React, {Component} from 'react';
import {connect} from 'react-redux';

import {createTodo, updateTodo, deleteTodo} from '../actions/todo.actions';
import Todo from '../components/todo.component';

export default connect(null, {createTodo, updateTodo, deleteTodo})(Todo);

todo.component.jsx

import React, {Component} from 'react';

import '../styles/todo.component.css';

export default class Todo extends Component {
    render () {
        return (
            <div className="todo">
                {todo.description}
            </div>
        );
    }
};

What I'm trying to figure out is this: I know I should not be embedding the <TodoContainer /> element inside of TodoList because TodoList is a presentational component and it should only nest other presentational components inside of it. But if I replace it with just a <Todo /> presentational component, then I have to map every state prop and action creator prop in TodoListContainer that the Todo component would need and pass them all down the chain manually as props. This is something I want to avoid of course, especially if I start nesting more levels or start depending on more props coming from Redux.

Am I approaching this correctly? It seems that I shouldn't be trying to embed a container component inside of a presentational component in general, because if I can decouple presentational components from Redux, they become more reusable. At the same time, I don't know how else to embed a component that requires access to Redux state/dispatch inside of any other component that has markup.

1
I think you can alleviate most pains you are encountering by refactoring a bit. Right now, each Todo is connected, you could compose the list of Todos (map) inside your todo-list.component and pass the three actions to your Todo component (now just a presentational component). The Todo component doesn't really need to be connected. - Mario Tacke
You're right. I just mean that, in a more complex app, there might be more than two levels of nesting, and the nested component might also require more than just a few props to be injected. What if Todo had a nested component AddTodoForm that required createTodo? I'd have to pass createTodo from TodoList to Todo just to forward it onto AddTodo. That's exactly the kind of problem that react-redux seeks to solve -- adding dependencies to a "root" component just to forward them down the tree to whichever component needs them. - M Miller
There is nothing wrong with nesting smart containers (aside from making them harder to test). I think structuring components makes all the difference and reduces this kind of nesting. For example: should createTodo really be part of Todo? IMO, no, it is part of the Container around it, possibly a few levels up. - Mario Tacke
I think I get your point. I think that NotificationList and CalendarWidget should be containers (smart) like you said, but wrapping a presentational component. gaearon makes this clear in his tutorials where he designs a "List" (dumb) and "TodoList" (smart). In your example, Dashboard is just composing other components, so I think it is fair to have multiple smart containers below it, but those only implementing presentational components. - Mario Tacke
I think that makes sense. Dashboard is a presentational component but it really shouldn't need to be tested itself since all it does is compose other components and some visual markup. I appreciate your help! As a side note, having read some articles by gaearon, I'm confused by one of his proposed solutions, which is to use this.props.children. He mentions using this but I haven't seen any examples of using children to get around the issue. Do you happen to know of any? (Also, if you want to phrase any of the comments you made as an answer, I'm happy to accept it.) - M Miller

1 Answers

13
votes

To specifically answer your question: It is okay to nest presentational and container components. After all, they are all just components. In the interest of easy testing however, I would prefer nesting presentational components over container components. It all comes down to a clear structuring of your components. I find that starting in a single file and then slowly component-izing works well.

Have a look at nesting children and utilizing this.props.children to wrap child elements in a presentational component.

Example (removed some code for brevity)

List (presentational component)

import React, { Component, PropTypes } from 'react';

export default class List extends Component {
  static propTypes = {
    children: PropTypes.node
  }

  render () {
    return (
      <div className="generic-list-markup">
        {this.props.children} <----- wrapping all children
      </div>
    );
  }
}

Todo (presentational component)

import React, { Component, PropTypes } from 'react';

export default class Todo extends Component {
  static propTypes = {
    description: PropTypes.string.isRequired
  }

  render () {
    return (
      <div className="generic-list-markup">
        {this.props.description}
      </div>
    );
  }
}

TodoList (container component)

import React, { Component, PropTypes } from 'react';
import { connect } from 'react-redux';
import { createTodo, updateTodo, deleteTodo } from 'actions';
import List from 'components/List';
import Todo from 'components/Todo';

export class TodoList extends Component {
  static propTypes = {
    todos: PropTypes.array.isRequired,
    create: PropTypes.func.isRequired
  }

  render () {
    return (
      <div>
        <List> <---------- using our presentational component
          {this.props.todos.map((todo, key) =>
            <Todo key={key} description={todo.description} />)}
        </List>
        <a href="#" onClick={this.props.create}>Add Todo</a>
      </div>
    );
  }
}

const stateToProps = state => ({
  todos: state.todos
});

const dispatchToProps = dispatch = ({
  create: () => dispatch(createTodo())
});

export default connect(stateToProps, dispatchToProps)(TodoList);

DashboardView (presentational component)

import React, { Component } from 'react';
import TodoList from 'containers/TodoList';

export default class DashboardView extends Component {
  render () {
    return (
      <div>
        <TodoList />
      </div>
    );
  }
};