Is connect() in leaf-like components a sign of anti-pattern in react+redux?

Question

Currently working on a react + redux project.

I'm also using normalizr to handle the data structure and reselect to gather the right data for the app components.

All seems to be working well.

I find myself in a situation where a leaf-like component needs data from the store, and thus I need to connect() the component to implement it.

As a simplified example, imagine the app is a book editing system with multiple users gathering feedback.

Book
    Chapters
        Chapter
            Comments
        Comments
    Comments

At different levels of the app, users may contribute to the content and/or provide comments.

Consider I'm rendering a Chapter, it has content (and an author), and comments (each with their own content and author).

Currently I would connect() and reselect the chapter content based on the ID.

Because the database is normalised with normalizr, I'm really only getting the basic content fields of the chapter, and the user ID of the author.

To render the comments, I would use a connected component that can reselect the comments linked to the chapter, then render each comment component individually.

Again, because the database is normalised with normalizr, I really only get the basic content and the user ID of the comment author.

Now, to render something as simple as an author badge, I need to use another connected component to fetch the user details from the user ID I have (both when rendering the chapter author and for each individual comment author).

The component would be something simple like this:

@connect(
    createSelector(
        (state) => state.entities.get('users'),
        (state,props) => props.id,
        (users,id) => ( { user:users.get(id)})
    )
)
class User extends Component {

    render() {
        const { user } = this.props

        if (!user)
            return null
        return <div className='user'>
                    <Avatar name={`${user.first_name} ${user.last_name}`} size={24} round={true}  />
                </div>

    }
}

User.propTypes = {
    id : PropTypes.string.isRequired
}

export default User

And it seemingly works fine.

I've tried to do the opposite and de-normalise the data back at a higher level so that for example chapter data would embed the user data directly, rather than just the user ID, and pass it on directly to User – but that only seemed to just make really complicated selectors, and because my data is immutable, it just re-creates objects every time.

So, my question is, is having leaf-like component (like User above) connect() to the store to render a sign of anti-pattern?

Am I doing the right thing, or looking at this the wrong way?

Clockwork

Answer 1

I think your intuition is correct. Nothing wrong with connecting components at any level (including leaf nodes), as long as the API makes sense -- that is, given some props you can reason about the output of the component.

The notion of smart vs dumb components is a bit outdated. Rather, it is better to think about connected vs unconnected components. When considering whether you create a connected vs unconnected components, there are a few things to consider.

Module boundaries

If you divided your app into smaller modules, it is usually better to constrain their interactions to a small API surface. For example, say that users and comments are in separate modules, then I would say it makes more sense for <Comment> component to use a connected <User id={comment.userId}/> component rather than having it grab the user data out itself.

Single Responsibility Principle

A connected component that has too much responsibility is a code smell. For example, the <Comment> component's responsibility can be to grab comment data, and render it, and handle user interaction (with the comment) in the form of action dispatches. If it needs to handle grabbing user data, and handling interactions with user module, then it is doing too much. It is better to delegate related responsibilities to another connected component.

This is also known as the "fat-controller" problem.

Performance

By having a big connected component at the top that passes data down, it actually negatively impacts performance. This is because each state change will update the top-level reference, then each component will get re-rendered, and React will need to perform reconciliation for all the components.

Redux optimizes connected components by assuming they are pure (i.e. if prop references are the same, then skip re-render). If you connect the leaf nodes, then a change in state will only re-render affected leaf nodes -- skipping a lot of reconciliation. This can be seen in action here: https://github.com/mweststrate/redux-todomvc/blob/master/components/TodoItem.js

Reuse and testability

The last thing I want to mention is reuse and testing. A connected component is not reusable if you need to 1) connect it to another part of the state atom, 2) pass in the data directly (e.g. I already have user data, so I just want a pure render). In the same token, connected components are harder to test because you need to setup their environment first before you can render them (e.g. create store, pass store to <Provider>, etc.).

This problem can be mitigated by exporting both connected and unconnected components in places where they make sense.

export const Comment = ({ comment }) => (
  <p>
    <User id={comment.userId}/>
   { comment.text }
  </p>
)

export default connect((state, props) => ({
  comment: state.comments[props.id]
}))(Comment)


// later on...
import Comment, { Comment as Unconnected } from './comment'

Arvind

Thank you, that make sense. My attempts at a top level connected component were breaking your "Module boundaries" point (which I had found through one of your website articles actually - jaysoo.ca/2016/02/28/organizing-redux-application). Good to hear that insight. – Clockwork

Answer 2

I agree with @Kevin He's answer that it's not really an anti-pattern, but there are usually better approaches that make your data flow easier to trace.

To accomplish what you're going for without connecting your leaf-like components, you can adjust your selectors to fetch more complete sets of data. For instance, for your <Chapter/> container component, you could use the following:

export const createChapterDataSelector = () => {
  const chapterCommentsSelector = createSelector(
    (state) => state.entities.get('comments'),
    (state, props) => props.id,
    (comments, chapterId) => comments.filter((comment) => comment.get('chapterID') === chapterId)
  )

  return createSelector(
    (state, props) => state.entities.getIn(['chapters', props.id]),
    (state) => state.entities.get('users'),
    chapterCommentsSelector,
    (chapter, users, chapterComments) => I.Map({
      title: chapter.get('title'),
      content: chapter.get('content')
      author: users.get(chapter.get('author')),
      comments: chapterComments.map((comment) => I.Map({
        content: comment.get('content')
        author: users.get(comment.get('author'))
      }))
    })
  )
}

This example uses a function that returns a selector specifically for a given Chapter ID so that each <Chapter /> component gets its own memoized selector, in case you have more than one. (Multiple different <Chapter /> components sharing the same selector would wreck the memoization). I've also split chapterCommentsSelector into a separate reselect selector so that it will be memoized, because it transforms (filters, in this case) the data from the state.

In your <Chapter /> component, you can call createChapterDataSelector(), which will give you a selector that provides an Immutable Map containing all of the data you'll need for that <Chapter /> and all of its descendants. Then you can simply pass the props down normally.

Two major benefits of passing props the normal React way are traceable data flow and component reusability. A <Comment /> component that gets passed 'content', 'authorName', and 'authorAvatar' props to render is easy to understand and use. You can use that anywhere in your app that you want to display a comment. Imagine that your app shows a preview of a comment as it's being written. With a "dumb" component, this is trivial. But if your component requires a matching entity in your Redux store, that's a problem because that comment may not exist in the store yet if it's still being written.

However, there may come a time when it makes more sense to connect() components farther down the line. One strong case for this would be if you find that you're passing a ton of props through middle-man components that don't need them, just to get them to their final destination.

From the Redux docs:

Try to keep your presentation components separate. Create container components by connecting them when it’s convenient. Whenever you feel like you’re duplicating code in parent components to provide data for same kinds of children, time to extract a container. Generally as soon as you feel a parent knows too much about “personal” data or actions of its children, time to extract a container. In general, try to find a balance between understandable data flow and areas of responsibility with your components.

The recommended approach seems to be to start with fewer connected container components, and then only extract more containers when you need to.

Breaker

Thanks for the example alternative solution. I'm not sure if it is a "better approach" though. As mentioned in my question, I've tried to load most of the data beforehand – though maybe not as neatly as you do – and it seems to make complicated selectors. – Clockwork

Also, where would you connect() a selector based on props? (the createChapterDataSelector(this.props.id) example you have) – Clockwork

You're totally right about the createChapterDataSelector being passed a prop, that doesn't make sense - I'll edit my answer. – Breaker

Ok, just fixed it. Now you just call createChapterDataSelector() without any arguments. As for your first comment, I guess that's a matter of opinion. I don't think the selectors I wrote are too complicated - they just compose together selectors that would otherwise have to be spread out across multiple connected components. I personally think it's easier to manage it in one place, and it gives you the added benefit of being able to track the data flow through React props. – Breaker

Ultimately you should find a balance of connected container components that makes sense to you, though. This is just one alternative, with some benefits. If you want to do what you described in your question, I don't think that's an anti-pattern. – Breaker

Thanks for adding further details to your answer. I (think I) understand what you mean, and yes, maybe it is seemingly ok to use on the current scale but when the app get complicated, will the root selector become totally unmanageable.. I guess the other way to ask the question is, is there any benefit in parsing the data before I need it – and, say I'm writing a new "view" of the chapter, do I then need a separate new selector that doesn't do the extra processing? – Clockwork

Yes, you'd definitely want to split up your selectors into logical chunks (even if they're used by the same component), as opposed to having one monolithic one. My example was just meant to show how you can make selectors that do more work for you so you don't have to just grab IDs and then use them to connect() other components. In a real project, I would split that up to make the selectors more reusable (e.g. for your new "view" example). Anyway, the main benefit to this approach, as opposed to connecting a ton of stuff, is a more traceable data flow. – Breaker

Thanks for the further explanation.. I'm still not sure on the "more traceable data flow" part.. a component that can get show a user from an ID doesn't seem much more complicated than a component that shows a user from an a given object model. – Clockwork

The component itself isn't more complicated, but how it fits into the component hierarchy is less clear. A <Comment /> component that gets passed 'content', 'authorName', and 'authorAvatar' props to render is easy to understand and use. You can use that anywhere in your app that you want to display a comment. Imagine that your app shows a preview of a comment as it's being written. With a "dumb" component, this is trivial. But if your component requires a matching entity in your Redux store, that's a problem because that comment may not exist in the store yet if it's still being written. – Breaker

Ah yes, that is very good food for thought. I like that point. – Clockwork

Answer 3

Redux suggests that you only connect your upper-level containers to the store. You can pass every props you want for leaves from containers. In this way, it is more easier to trace the data flow.

This is just a personal preference thing, there is nothing wrong to connect leaf-like component to the store, it just adds some complexity to your data flow, thus increase the difficulty to debug.

If you find out that in your app, it is much easier to connect a leaf-like component to the store, then I suggest do it. But it shouldn't happen very often.

Potbelly

Thank you for your input on this. Could you expand on "it just adds some complexity to your data flow".. to me, it does and does not, because the selectors are seemingly more complex. – Clockwork

This is outdated. The very early advice from Dan was to only connect at the top of your app structure. However, practical experience has shown that more flexibility is needed, and the current advice is to connect wherever it makes sense for your app. See redux.js.org/docs/FAQ.html#react-multiple-components . – Deluxe

The updated doc link is: redux.js.org/docs/faq/ReactRedux.html#react-multiple-components – Allisan