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

RFC: Use PureComponent #1020

Closed
layershifter opened this issue Dec 12, 2016 · 7 comments
Closed

RFC: Use PureComponent #1020

layershifter opened this issue Dec 12, 2016 · 7 comments

Comments

@layershifter
Copy link
Member

layershifter commented Dec 12, 2016

Hello, guys!

PureComponent

I want to speak about next optimization topic - PureComponent. It's previously known as pure render mixin, it makes shallow compare of props inside shouldComponentUpdate(). Inheriting from React.PureComponent indicates to React that your component doesn't need to rerender when the props are unchanged (#7195).

I'm using it in my app and it's perfect 😸 There are some benchmarks and conclusions.

Benefits

  • Faster rerendering.
  • Refs.
  • Unification.

We will loose stateless components, according to Dan Abramov they are same as statefull inside, so the same slow as Component child. Also, now we have stateless, statefull and statefull component without real state (#607), after refactoring we will have Component and PureComponent classes 👍

Limitations

  • React ^0.15.3.
  • Stateless components looks really nice 😿

@jcarbo @levithomason thoughts?

@levithomason
Copy link
Member

I don't think we should implement this, here's why:

Quoting @jimfb from facebook/react#7195 (comment):

In addition to event handlers bound in render, it is probably also worth mentioning that any component which takes jsx children will also always fail shallowEqual, since the children elements will be re-created for every render. I think most people don't realize this.

Almost all of our components can render JSX children, meaning, someone's app can potentially be shallow comparing all our components for no reason.

Quoting Abramov from facebook/react#7195 (comment):

However just making every one of them work like PureComponent can slow down your app. Please don't think shallow equality checks are extremely cheap. They can help your app when placed strategically but just making every single component pure can actually make your app slower. Tradeoffs.

Quoting Abramov again, from react-pure-render:

If a component in the middle of your rendering chain has pure rendering, but some nested component relies on a context change up the tree, the nested component won't learn about context change and won't update. This is a known React issue that exists because context is an experimental feature and is not finished. However some React libraries already rely on context, for example, React Router. My suggestion for now is to use pure components in apps relying on such libraries very carefully, and only use pure rendering for leaf-ish components that are known not to rely on any parent context.

Finally, per the PureComponent docs:

If your React component's render() function renders the same result given the same props and state, you can use React.PureComponent for a performance boost in some cases.

Note

React.PureComponent's shouldComponentUpdate() only shallowly compares the objects. If these contain complex data structures, it may produce false-negatives for deeper differences. Only mix into components which have simple props and state, or use forceUpdate() when you know deep data structures have changed. Or, consider using immutable objects to facilitate fast comparisons of nested data.

Furthermore, React.PureComponent's shouldComponentUpdate() skips prop updates for the whole component subtree. Make sure all the children components are also "pure".

Note, the distinction of "some cases" in the performance boost comment. Moreover, consider the warning that follows.

Conclusion

If we did add PureComponent, it would probably only be to the Flag and Icon since these are the only components that we can know for certain will not have JSX children.

Even if we could add this feature we have no way of ensuring that child components are pure since users are free to use any child components they wish inside ours.

We should likely leave this implementation to apps that integrate our lib rather than implementing it ourselves.

@qoalu
Copy link
Contributor

qoalu commented May 15, 2017

don't think shallow equality checks are extremely cheap

So in some cases it's better to rerender?

I ran into the issue of slowness on changes to an input. In perf tool I noticed that a Form and all its children rerender every time I make an input in a completely different part of the component.

To improve this I wrapped the Form into a PureComponent.
image

Is splitting into subcomponents the preferred/suggested way to optimize performance when working with Semantic UI?

@levithomason
Copy link
Member

levithomason commented May 15, 2017

This is due to your top level component being connected. For maximum performance, you can connect each form control individually. This way, when your data changes, only the form controls that are connected to that data update.

Is splitting into subcomponents the preferred/suggested way to optimize performance when working with Semantic UI?

I would say yes. However, this is a Redux performance pattern and not specific to Semantic UI React.

@brianespinosa
Copy link
Member

If we did add PureComponent, it would probably only be to the Flag and Icon since these are the only components that we can know for certain will not have JSX children.

@levithomason I have not seen any further discussion about these two components being converted to PureComponent. We were running some Perf tests today and saw some decent performance hits for Icon. Would converting Flag and Icon to PureComponent be a good PR?

@levithomason
Copy link
Member

The icon perf issues are a known issue and not related to PureComponent but to #1184. In short, the propType validator is very expensive and needs memoized.

I would still be up for using PureComponent in Icon and Flag except that it is only available for React 15.3 (released on June 29, 2016). There may be users on older versions of React. However, we could simply add shallowequal and implement shouldComponentUpdate in these two components ourselves. This is exactly what PureComponent does for you.

PR for manually implementing shallowequal and SCU in Icon and Flag? Yes please :)

@DavidLozzi
Copy link

DavidLozzi commented Nov 30, 2018

Hate to open an old issue, BUT is there a workaround or best practice in the event I want to use something like PureComponent with Semantic? We have 20 renderings of a Card component, and several subcomponents, and if a user clicks a textbox, it all rerenders, locks the screen and takes 3-4 seconds before they can type. Narrowed it down to the Card component...

My parent components are using PureComponent but it appears to not care much about that....

@layershifter
Copy link
Member Author

Wrap your components with React.memo: https://reactjs.org/docs/react-api.html#reactmemo

@Semantic-Org Semantic-Org locked as resolved and limited conversation to collaborators Dec 1, 2018
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

5 participants