-
Notifications
You must be signed in to change notification settings - Fork 47.2k
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
Add React.PureComponent #7195
Add React.PureComponent #7195
Conversation
); | ||
} | ||
} | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: could be else if
for less nesting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be but I prefer parallel structure in my if/else-if cases so I left it like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be but I prefer parallel structure in my if/else-if cases so I left it like this.
IMO, always good to use least syntax~
When documenting this I'd say it's important to note that callback handlers that are bound in render will always fail |
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 |
ReactComponent.call(this, props, context, updater); | ||
} | ||
|
||
Object.assign(ReactPureComponent.prototype, ReactComponent.prototype); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we extend ReactComponent? What about people doing instanceof React.Component
even though they shouldn't?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All right.
This provides an easy way to indicate that components should only rerender when given new props, like PureRenderMixin. If you rely on mutation in your React components, you can continue to use `React.Component`. Inheriting from `React.PureComponent` indicates to React that your component doesn't need to rerender when the props are unchanged. We'll compare the old and new props before each render and short-circuit if they're unchanged. It's like an automatic shouldComponentUpdate.
@jimfb This is mitigated somewhat in production by the babel constant elements transform, no? |
This provides an easy way to indicate that components should only rerender when given new props, like PureRenderMixin. If you rely on mutation in your React components, you can continue to use `React.Component`. Inheriting from `React.PureComponent` indicates to React that your component doesn't need to rerender when the props are unchanged. We'll compare the old and new props before each render and short-circuit if they're unchanged. It's like an automatic shouldComponentUpdate.
This provides an easy way to indicate that components should only rerender when given new props, like PureRenderMixin. If you rely on mutation in your React components, you can continue to use `React.Component`. Inheriting from `React.PureComponent` indicates to React that your component doesn't need to rerender when the props are unchanged. We'll compare the old and new props before each render and short-circuit if they're unchanged. It's like an automatic shouldComponentUpdate. (cherry picked from commit c8fbdac)
Is there also going to be a |
For |
That's the plan. |
Great, thanks. |
@spicyj don't you think |
Actually, that's a good point. We should make it a worse name so that nobody relies on this implementation detail. :) |
Why was the decision to implement this class favoured over making React better equipped to deal with functional stateless components? |
@brycehanscomb I believe the intention is to add this as a non-breaking opt-in change, and then in React 16 change the internal SFC wrapper class to derive from |
It's a stepping stone. We'll keep thinking about optimizing functional stateless components. 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. For now we added a base class because we wanted an official way of marking component as compatible with shallow equality checks, without using mixins. We'll see how and if we want to extend this behavior in some cases to functional components but it's not as straightforward as just making them behave the same. |
I remember from the other discussion thread that SFC were supposed to check the nearest class parent and based on it being Pure or not should opt in the being pure themselves. Is it the part of this release? |
No, we decided to not proceed with this because the community discussion around it was controversial. These heuristics would be far from obvious, and we feel that this is not a good time to introduce them. Maybe we’ll figure out something more explicit in the future. |
Hi @gaearon! In the Changelog of react 15.3 (https://github.com/facebook/react/blob/v15.3.0/CHANGELOG.md), says that React.PureComponent was added, and refers this discussion. But here you said that this feature didn't was incorporated. After all, it's in the React 15.3 or not? Is there some documentation about this? Thanks :) |
PureComponent is included; the heuristics for stateless functional components are not. |
The feature is incorporated. My comment referred specifically to the previous comment: #7195 (comment).
So “heuristics” for functional components are not in, but PureComponent is. |
Great! Thanks guys! :) |
Is there a plan to add docs to https://facebook.github.io/react/docs about I'm running into some unexpected |
This seems like something you’d want to file with Jest. |
*/ | ||
function ReactPureComponent(props, context, updater) { | ||
// Duplicated from ReactComponent. | ||
this.props = props; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's not a duplication, right? 🍺
This provides an easy way to indicate that components should only rerender when given new props, like PureRenderMixin. If you rely on mutation in your React components, you can continue to use
React.Component
.Inheriting from
React.PureComponent
indicates to React that your component doesn't need to rerender when the props are unchanged. We'll compare the old and new props before each render and short-circuit if they're unchanged. It's like an automatic shouldComponentUpdate.