React "Containers / Components" are an anti-pattern

Yeah, the "container vs component" distinction is falling more and more out of favor with me. It's an okay pattern to start, but it's a pretty hard black/white in a place where things are a lot more grey.

In my opinion, there's no reason for this:

// app/index.jsx
import AppComponent from './App'

const AppContainer = ({foo, bar, baz, buz, wham, bam, alakazam, setFoo, setBar, setBaz, setBuz, setWham, setBam, setAlakazam}) =>
  <AppComponent foo={foo} bar={bar} baz={baz} buz={buz} wham={wham} bam={bam} alakazam={alakazam} setFoo={setFoo}, setBar={setBar}, setBaz={setBaz}, setBuz={setBuz}, setWham={setWham}, setBam={setBam}, setAlakazam={setAlakazam} />

export default connect(
  {foo, bar, baz, buz, wham, bam, alakazam},
  {setFoo, setBar, setBaz, setBuz, setWham, setBam, setAlakazam}
)(AppContainer)

// app/App.jsx
const App = ({foo, bar, baz, buz, wham, bam, alakazam, setFoo, setBar, setBaz, setBuz, setWham, setBam, setAlakazam}) =>
  <Header/>
  <Body foo={foo} bar={bar} baz={baz} buz={buz} wham={wham} bam={bam} alakazam={alakazam} setFoo={setFoo}, setBar={setBar}, setBaz={setBaz}, setBuz={setBuz}, setWham={setWham}, setBam={setBam}, setAlakazam={setAlakazam} />

export default App

It causes more issues than it prevents, because...

(a) There's so much duplication it hurts. We get around some of this with ...'s and other methods, but the fact of the matter is that our "containers" usually take a bunch of props and render a single component which takes all of those props. That's no bueno.

(b) App.jsx is still a really smart component. What does separating "containers" from "components" get us? Like... why is that a valuable design pattern? The only upside it has, imo, is that it allows for dumb components. And the only reason those are useful in any way is because they can be re-used all over the place. App.jsx isn't that. App.jsx is a "container" with a thin mask on. Its status as a "component" doesn't do anything helpful for us, and only hinders us.

(c) AppContainer is now a dumping ground for logic. So, we have a bunch of duplication, App.jsx isn't actually doing anything useful, but the most insidious problem is this one. Because we've labeled AppContainer as "smart", it feels like the right place to dump parsers, validators, handlers, fetchers, assemblers, etc, etc etc. This is what we typically see with Containers:

// import everything
import fetchFoo from 'fooFetchers'
// and bar, and baz, and every other fetcher individually
// be sure to pass those in via connect
connect({}, {fetchFoo, fetchBar, ...})(AppContainer)
// be sure to validate those proptypes
static propTypes = { fetchFoo: func, fetchBar: anotherFunc, fetchBaz: aaaandAnotherFunc }

onComponentDidMount() {
  this.props.clearSomeData()
  this.props.fetchFoo()
  this.props.fetchBar()
  if (baz) { this.props.fetchBuz(this.props.someParam) }
}

updateTheThing(param1, param2) {
  if (condition && complexLogic || !caveat) {
    this.props.updateTheThing(param1, param2)
  } else {
    this.props.dontUpdate(param1, param2 || '')
  }
}

constructor() { this.updateTheThing = this.updateTheThing.bind(this) }

So, a bunch of overhead to import, map, and validate everything. Some overhead even in just pulling a bunch of properties off of this.props. And no separation of concerns between formatting, massaging data, defaults, updating, handling component or app state, etc. It's all here. Good luck testing any of this.

So what should this actually look like?

import fetchDependencies from './my-app-api-service'

class AppContainer extends Component {
  componentDidMount() {
    this.props.dispatch(fetchDependencies())
  }

  static propTypes = { dispatch: function }

  render() {
    return <App { ...this.props }/>
  }
}

export default connect({stuff})(AppContainer)

Now you can test your API handler and make sure it's getting everything needed for the app, without having to full-render a component just to hook into its lifecycle. The complex parser/updater can be tested and used wherever it's needed — and it's not actually needed here, so some other component can import and use it. The thing is... in a perfect world, every container should look pretty much like this. And in my "front end revamps" work, you can see that this is what almost every container becomes once you pull re-usable logic into services and handle state correctly. In fact, most containers become stateless functional components.

So the question becomes... why even have a "dumb" App component at all? It's not actually dumb, it leads to repetitious code. Is it really a problem to change AppContainer's render method to this?

render() { return <div><Header/><Body/></div> }

I submit that it's not. And that the heuristic for "is this a container or a component?" (in other words, "is this a smart or dumb component?") shouldn't be "is this connected to the store?" but rather "is this re-usable?"

markalfred/containers-vs-components.md