Stateless components vs components with state vs PureComponents

[petemill]

I agree wholeheartedly with the goals of this project, especially doing it asynchronous to a specific product. And yes, it's very fun!

However, I have a couple of small implementation issues that are bought up in the initial readme, and this is one of them.

I'm not sure we should enforce that components should be pure, stateless functions. Sometimes it's neccessary to use state and a component lifecycle in order to do something that is very contained within the component, and I think that's ok. I think it's worth evaluating it on a case by case basis. Examples where I think it's a valid use would be an animation, or some DOM manipulation to bring in a third party component.

Another reason I think that's ok, is that it contains some of the reasons for doing ref={} and using React lifecycle functions in to these components, and hopefully reduces a bit of mess in the products that use the components.

What I do think we can try to encourage is to use React.PureComponent instead of React.Component instead. This tells the renderer that as long as the props are exactly the same, then there won't be any rendering involved. I think stateless function components extend from React.Component, so the function is run on every new cycle.

If you disagree, I'd love to talk about it. If you agree, I can propose a change to the readme in a PR... 😄

[luixxiul]

Which one would you think is better in terms of performance?

[cezaraugusto]

ya this is still open for discussion, we still need to define what to expect from this.

The very first plan of mine was to create presentational components only, for example in the original README we had:

module.exports.Notification = (props) => {
  return (
    <div
      className={css(styles.notificationBar)}
      data-test-id={props.testId}
    >
      {
        props.notificationGroup
          .map(message => NotificationItem(props.message))
      }
    </div>
  )
}

which should be separated from the behavioral layer, being the <NotificationBar /> component under browser-laptop responsible for this. In there we could have all the lifecycles and methods to make the notification work.

The first goal was to make small components in a way that they could be quickly prototyped and changed by people w/ enough CSS/HTML skills (basically the design team).

However, I'm starting to wonder where will we get as we add more complex components such as the table component (which should be the next one), and the multiple select option (our spellchecker in browser-laptop), and if we want to go further and add complete components such as autocomplete or even tabs for example, which requires lifecycle methods to operate.

I tried adding PureComponent for tab content while refactoring but couldn't see a real perf gain, but maybe the component was just too small to see a big win or there isn't much to optimize in the component but anyway PureComponent is something to consider as we grow. I think it's valid to let this open until we came with a definitive solution.

[cezaraugusto]

What I do think we can try to encourage is to use React.PureComponent instead of React.Component instead.

ya thanks @petemill we're moving that way. Needs #9 to fulfill but new ones should be pure by default. I'll close this please re-open if you have something.