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

container vs component? #756

Closed
ronag opened this issue Sep 19, 2015 · 46 comments
Closed

container vs component? #756

ronag opened this issue Sep 19, 2015 · 46 comments

Comments

@ronag
Copy link

ronag commented Sep 19, 2015

In the examples there are always a folder called "container" and one called "component". What is the thought behind this separation?

Are "containers" smart component or are they route components or something else? Should components under "components" always be dumb?

@imevro
Copy link
Contributor

imevro commented Sep 19, 2015

As for me, container is route's handler, which also pulls redux's state for that route. Then I pass down my state as prop.

For example,

containers/properties.jsx

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

import * as actions from 'actions/properties';
import Filter from 'components/properties/filter';
import List from 'components/properties/list';
import Pagination from 'components/properties/pagination';

class PropertiesContainer extends Component {
  render() {
    return (
      <section>
        <Filter filter={this.props.properties.params.filter} />
        <List items={this.props.properties.items} isFetching={this.props.properties.isFetching} />
        <Pagination pagination={this.props.properties.params.pagination} />
      </section>
    );
  }
}

function mapState(state) {
  const { properties } = state;

  return { properties };
}

function mapDispatch(dispatch) {
  return {
    actions: bindActionCreators(actions, dispatch),
  };
}

const Connector = connect(mapState, mapDispatch)(PropertiesContainer);

export default Connector;

and, for example,

components/properties/pagination.jsx

import React, { Component } from 'react';
import Pager from 'components/ui/pager';

class Pagination extends Component {
  render() {
    const { total, offset, limit } = this.props.pagination;
    const current = offset / limit;

    return (
      <Pager total={total} current={current} />
    )
  }
}

export default Pagination;

@ronag
Copy link
Author

ronag commented Sep 19, 2015

Reading from the links you provided:

"A container does data fetching and then renders its corresponding sub-component. "

I get more the feeling that "containers" are actually what in redux is called "smart components"... and routes/pages should get their own folder.

I'm not sure why "container" and "route handlers" are in any way related?

@timdorr
Copy link
Member

timdorr commented Sep 19, 2015

It's a very common practice to group your app's components by route. As @theaqua pointed out, it's also common to make each route handler/component a smart/container component. If you're using combineReducers, you'll often find yourself breaking up state, routes, and containers along the same lines. Hence, they often get associated together.

@imevro
Copy link
Contributor

imevro commented Sep 19, 2015

@ronag I didn't think you need make containers and pages. Why you can't put route handler & redux connector to state in 1 place? It's very handy. Keep it simple.

@ronag
Copy link
Author

ronag commented Sep 19, 2015

In my application I only have 3 routes/pages. On the other hand I have lots of independent panels/modules. Making one huge connect is not feasible I need to at least split it up in terms of panels.

If I do all the mapping from state to props and all data fetching in a single file it would be unmaintainable. Especially if I do all the data fetching at the same location.

@imevro
Copy link
Contributor

imevro commented Sep 19, 2015

If you have 3 routes, so you need 3 route handlers. For example, A.jsx, B.jsx and C.jsx in /containers.

In each container you pull part (not entire!) of redux state and actions binds. Then you pass down this to components like in my example. It's very maintainable, because I (and my team) know — connect to redux and actions binds always in containers, it's never will be in components (which is small and often stateless as in my example).

@ronag
Copy link
Author

ronag commented Sep 19, 2015

I think I understand your suggestion. However, as I said those 3 files will become very complicated if I put all my data fetching there. In our cause it would basically be "Login, App, Logout" where App would contain almost everything.

@ronag
Copy link
Author

ronag commented Sep 19, 2015

Maybe I'm too biased to Relay where each component has a container which describes what data to fetch and how it should look.

I'll try your suggestion and see where we end up.

@gaearon
Copy link
Contributor

gaearon commented Sep 19, 2015

I call components encapsulated React components that are driven solely by props and don't talk to Redux. Same as “dumb components”. They should stay the same regardless of your router, data fetching library, etc.

I call containers React components that are aware of Redux, Router, etc. They are more coupled to the app. Same as “smart components”.

@ronag
Copy link
Author

ronag commented Sep 19, 2015

@gaearon: Perfect. That clarifies the examples. Thank you.

@soulmachine
Copy link

@gaearon So "dumb components" are stateless components, which can be written in a simpler syntax Since React 0.14, for example:

var Aquarium = (props) => {
  var fish = getFish(props.species);
  return <Tank>{fish}</Tank>;
};

Am I correct?

@imevro
Copy link
Contributor

imevro commented Dec 14, 2015

@soulmachine yep.

@gaearon
Copy link
Contributor

gaearon commented Dec 14, 2015

Not necessarily. Syntax is not the point.

"Dumb" components aka "presentational" components are components that receive all data by props, aren't aware of Redux, and only specify the appearance but not the behaviour.

@soulmachine
Copy link

@theaqua @gaearon Thanks!

@ghost
Copy link

ghost commented Dec 15, 2015

Although the term "container" is already quite popular in the react/redux nomenclature, we decided to call them "connectors" in the project I'm working on, to avoid any confusion with layout/graphical containers.

@Dattaya
Copy link

Dattaya commented Dec 15, 2015

btw, how to call them was previously discussed here reduxjs/react-redux#170. I keep everything inside a components folder and presentational ones one level deeper, inside components/presentational, and I think it's fine because they're unlikely to be needed by parts of the app other than containers.

@pswai
Copy link

pswai commented Jan 20, 2016

@gaearon so is a component that dispatch considered "dumb" or "smart"? It is particularly useful to have a leaf or intermediate component to dispatch an action instead of bubbling the event back to top component to do the dispatch.

@gaearon
Copy link
Contributor

gaearon commented Jan 20, 2016

Yeah it can be occasionally useful although I prefer to connect() such components so action creator is injected as a prop. It doesn't really matter how you call it :-)

@benmonro
Copy link

What about when you're building a module of components? For example, suppose i have a separate node module for our navigation menu <NavMenu> I want people to do code like this:

import {NavMenu} from 'my-redux-aware-components';

export function myPage(props) {
  return (<div><NavMenu routes={props.routes} /></div>);
}

so do I just name it 'NavMenuContainer'? that seems weird to me. Should I name the component NavMenuComponent instead? both seem weird to me. In this case the component only calls connect to get 1 field from the state. Is it really so bad to just inline the call to connect like this?

export default const NavMenu = connect(state => ({currentPath:state.routing.path})(React.createClas({...}));

Curious to hear your thoughts @gaearon on when (if at all) it's "ok" to just inline the connect call...

@gaearon
Copy link
Contributor

gaearon commented Jan 20, 2016

It doesn't matter how you call it. Why not call it NavMenu?

I don't understand the question about inlining.
It would help if you presented two approaches you're comparing.

@benmonro
Copy link

ok so for example.
option 1 (2 separate files, 1 for container, 1 for component)

containers/NavMenu

import {connect} from 'react-redux';
import NavMenu from '../components/NavMenu';

export default connect(state => ({currentPath:state.routing.path})(NavMenu);

option 2 (1 single file containing both):
components/NavMenu

import {connect} from 'react-redux';

export default connect(state => ({currentPath:state.routing.path})(React.createClass({
  render() {
      return <div>the menu {this.props.currentPath} goes here</div>;
  }
});

what I mean by inlining is just having a single file (as opposed to 2 files) which is both the container and the component since there's only the 1 little bit about the currentPath that's needed from state. Is this just something that should always be avoided or is it reasonable to do it in simple cases like this?

@gaearon
Copy link
Contributor

gaearon commented Jan 20, 2016

Sure, it's reasonable to do this in simple cases.

@benmonro
Copy link

ok cool. when would you draw the line and say "ok I'd move this into 2 separate files"?

@gaearon
Copy link
Contributor

gaearon commented Jan 20, 2016

When the component starts to mixes data concerns (how to retrieve / calculate data, how to dispatch actions) with presentation (how it looks). When it becomes hard to test or reuse in different context.

@sompylasar
Copy link

@benmonro One more thought. If you're building a module of components, you would sometimes want to connect these components to different parts of your app's state tree, or test their presentation separately with various states. In this case, having connect inside this module of components would limit this ability.

@benmonro
Copy link

thanks @gaearon

@sompylasar very good point! thanks

@benmonro
Copy link

hmm ok after going through my code to refactor it to split it out, I now have a follow up question. suppose that I also have a <NavMenuItem> container. The <NavMenu> component needs to reference <NavMenuItem /> as a child element. Should i just do import NavMenuItem from '../containers/NavMenuItem' in the NavMenu component? How does this affect testability @sompylasar?

@sompylasar
Copy link

I would make the NavMenuItem a pure presentational component with all needed data passed into it via props by NavMenu. This would allow to test it separately. So, you would have two presentational (NavMenu, NavMenuItem) and one connected ( connect(...)(NavMenuItem) ) component.

@c089
Copy link

c089 commented Jan 22, 2016

A thing I'm missing in this discussion: When you have them in one file, you can't use shallow rendering for testing. I've seen people expose both the presentational component and the container component to work around this, and test then separately, so in this case I'd rather have two files to make it explicit that these are two things. This also highlights the separation of concerns here and the fact that the presentational component is independent and reusable.

@gaearon
Copy link
Contributor

gaearon commented Jan 26, 2016

FWIW I updated the Presentational and Container Components article to reflect my current thinking.

@gaearon
Copy link
Contributor

gaearon commented Feb 2, 2016

We no longer discourage creating container components in the updated docs.
http://redux.js.org/docs/basics/UsageWithReact.html

@LucaColonnello
Copy link

@gaearon in your actual example in the Redux doc seems that components can have containers as child.
Am I wrong? How can this affect testability of a dumb component that render inside it a smart one?
However I don't find the way to let components be the latest ones in the hierarchy...

I have an app that render a Simple Data List component (Dumb).
Inside it every item has to be connected to the store, so it's a Smart one.

It's ok according to the doc, but can it bring some problem?

Thank!

@gaearon
Copy link
Contributor

gaearon commented Feb 7, 2016

How can this affect testability of a dumb component that render inside it a smart one?

This does make testing a little harder to set up (you need to initialize the store as well). When this is an inconvenience, extract more presentational components that accept children so you can pass container components inside. In general, the division is up to you, and you need to evaluate the tradeoffs (ease of writing, refactoring, testing, etc), and choose how to separate components for yourself.

@LucaColonnello
Copy link

Ok, so there's no right way. We have to evaluate case by case.. Many
thanks!!

Il lunedì 8 febbraio 2016, Dan Abramov notifications@github.com ha
scritto:

How can this affect testability of a dumb component that render inside it
a smart one?

This does make testing a little harder to set up (you need to initialize
the store as well). When this is an inconvenience, extract more
presentational components that accept children so you can pass container
components inside. In general, the division is up to you, and you need to
evaluate the tradeoffs (ease of writing, refactoring, testing, etc), and
choose how to separate components for yourself.


Reply to this email directly or view it on GitHub
#756 (comment).

Luca Colonnello
+39 345 8948718
luca.colonnello91@gmail.com

@Emilios1995
Copy link

What about 'layout components'? I mean when you have many containers that need to be in a single component to pass it to the router/navigator, this wrapping component is going to be a 'dumb presentational container of containers' right?

@LucaColonnello
Copy link

@Emilios1995 I have the same issue...
I have a Page component that use inside a layout component.
This Layout component have menus, headers, footer.. The content is the child passed by Page to Layout..
Menus and headers are container!! So Layout potentially is a container, but isn't connected to the store..

However, if I try to pass to layout the menus and the header, i have a page (container) that render layout (component) and passing to it menus and header (containers).

Doing this the hierarcy is correct, but I have to repeat menus and header in every page, and bith of them are the same in every page for me..

@gaearon
Copy link
Contributor

gaearon commented Feb 27, 2016

@LucaColonnello I don’t quite understand the issue without the code. May I ask you to create a StackOverflow question with a simple example illustrating your problem? I’d be happy to take a look.

@LucaColonnello
Copy link

ASAP

Il sabato 27 febbraio 2016, Dan Abramov notifications@github.com ha
scritto:

@LucaColonnello https://github.com/LucaColonnello I don’t quite
understand the issue without the code. May I ask you to create a
StackOverflow question with a simple example illustrating your problem? I’d
be happy to take a look.


Reply to this email directly or view it on GitHub
#756 (comment).

Luca Colonnello
+39 345 8948718
luca.colonnello91@gmail.com

@Emilios1995
Copy link

@LucaColonnello
Copy link

I think Dan's article on medium
https://medium.com/@dan_abramov/smart-and-dumb-components-7ca2f9a7c7d0#.3y00gw1mq
clarifies all the issues...

2016-03-01 18:19 GMT+01:00 Emilio Srougo notifications@github.com:

@gaearon https://github.com/gaearon It's not an issue, I think it's
rather a question which I've made here:
http://stackoverflow.com/questions/35729025/should-the-route-handlers-use-containers-or-presentational-components?noredirect=1#comment59133192_35729025


Reply to this email directly or view it on GitHub
#756 (comment).

Luca Colonnello
+39 345 8948718
luca.colonnello91@gmail.com

@tonytonyjan
Copy link

tonytonyjan commented May 4, 2016

@gaearon
Copy link
Contributor

gaearon commented May 8, 2016

I wonder that if a presentational component may contain container components inside, how can it be reusable?

If all its usage scenarios include a specific container inside, I don’t see what’s not reusable about it. And if not, make it accept this.props.children and create other presentational components that pass specific containers or presentational components inside.

@zenato
Copy link

zenato commented Jul 1, 2016

@gaearon Is [Root component] container component on React-Router?

  • route.js
<Route path="/" component={Root}>
      <IndexRoute component={Main} />
      <Route path="/account/signIn" component={SignIn} />
</Route>
  • root.js
export default class Root extends React.Component {
  render() {
    return (
      <div>
        <div id="container" className="container">
          {this.props.children}
        </div>
      </div>
    );
  }

Thanks.

@ChuckJonas
Copy link

@gaearon above you said that you prefer to connect components in order to gain access to dispatch (as opposed to passing it down through from parent components).

If you connect these components, and they also have props which could be mapped from the reducers which are currently being passed in by the parent, would you refactor these to come from connect? Or use ownProps to keep them passed by parent.

Is there a functional/performance difference between the two options?

@dvzrd
Copy link

dvzrd commented Dec 10, 2017

I don't have much experience working with huge redux projects but I've been researching and thinking a lot about optimizing react/redux file structures. Let me know if this makes any sense or not:

src/
  components/
    header/ 
      navigation.js # nav menu list
      index.js # Header component
  modules/
    header/
      actions.js # header actions (sticky scroll, collapse mobile menu, etc...)
      reducer.js # header actions reducer (export to modules index)
      index.js # HeaderContainer (connect to Header component)
    index.js # combineReducers and export default configureStore (and middleware)

another concept:

src/
  components/
    navigation.js
    logo.js
    link.js
    list.js
    item.js
  modules/
    header/
      actions.js # header actions 
      wrapper.js # header class (smart) component - to wrap header with functionality (was previously classified as container)
      container.js # header functional (dumb) component - to contain universal components
      index.js # header actions reducer - to export into modules rootReducer 

Or is it just better to keep components, containers and redux modules separate (even though they share the same name)? Thanks for any input.

@saranshbansal
Copy link

I have worked with enterprise level react-redux projects and through my experience, I can say one thing that it solely depends on you how to define the architecture of your project. I don't follow any of the container/component based architectural variations because they are impractical in the sense that React's purpose is to make reusable components based UI.

So I have come up with a simple approach of grouping entire projects based on Modules and it has worked really well till now in terms of scalability, maintainability, and code readability.

image
image
image

For me, container and smart-component are exactly the same. The simple definition is:
A container/smart-component is the one which contains JSX markup + event handlers + api calls + redux's connect/MSTP/MSDP.
A dumb component is PURELY presentational, functional component.

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

No branches or pull requests