This repository has been archived by the owner on Jul 19, 2019. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 47
Add June 2 notes #18
Merged
Merged
Add June 2 notes #18
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
## June 2 ([discuss](https://github.com/reactjs/core-notes/pull/18)) | ||
|
||
### Attendees | ||
|
||
* [Ben](https://twitter.com/soprano) (React) | ||
* [Jim](http://github.com/jimfb) (React) | ||
* [Keyan](https://twitter.com/keyanzhang) (React, intern) | ||
* [Paul](https://twitter.com/zpao) (React) | ||
* [Sebastian](https://twitter.com/sebmarkbage) (React) | ||
* [Shayne](https://github.com/shayne) (React Native) | ||
* [Tom](https://twitter.com/tomocchino) (React) | ||
|
||
### Getting Rid of `PureRenderMixin` | ||
|
||
* This is one of the most commonly used mixins. | ||
* We need to have a good story around applying it to classes and SFCs (**stateless functional components**). | ||
* Let’s consider a few alternatives. | ||
|
||
#### Proposal 1: `PureComponent` and heuristics for SFCs | ||
|
||
* Create `React.PureComponent` base class (kinda like a class with `PureRenderMixin`). | ||
* Make SFCs “inherit” their purity from the closest class-based parent. | ||
* This works because if the class above is pure, SFC wouldn’t re-render anyway. | ||
* Implemented in [#6914](https://github.com/facebook/react/pull/6914) with some additional explanation [here](https://github.com/facebook/react/pull/6914#issuecomment-222364942). | ||
|
||
##### Concerns | ||
|
||
* Any heuristics based model will probably not work in 100% of cases, and could cause confusion. | ||
* If you pass children through, you don’t know anything about them, including whether they use mutation. | ||
|
||
##### Potential Mitigations | ||
|
||
* Warn if a `PureComponent` takes in a child element (or child element which changes). | ||
* In dev mode, do dry-run reconciliation past all `shouldComponentUpdate`s and warn if `shouldComponentUpdate` lied to React. | ||
|
||
#### Proposal 2: SFCs Always Shallowly Compare Props | ||
|
||
* Effectively a `PureRenderMixin` on every SFC. | ||
|
||
##### Concerns | ||
|
||
* Means that component authors generally can’t / shouldn’t use SFCs because `PureRenderMixin` shouldn’t be used with components that take children. | ||
* Intuitively, people expect SFCs to behave like functions. The bailout would be surprising, and there isn’t a particularly good way of discovering why the function isn’t behaving like a normal JavaScript function. | ||
* Performance. It’s not clear that adding `PureRenderMixin` to every SFC would actually improve overall performance (because it means extra reads/compares, retaining objects longer and into subsequent GC generations, etc). | ||
|
||
#### Proposal 3: `createPureElement` | ||
|
||
* Provide some flag on components at the calling side to denote that they should be pure. | ||
* This would tell React to effectively “use `PureRenderMixin`” on that particular instance, *not all instances*. | ||
* Many syntax possibilities: `pure={true}`, or `<*Component />`, `React.createPureElement()`, etc. | ||
|
||
### ES Class Progress | ||
|
||
* [Ben](https://twitter.com/soprano) is leading the effort to port Facebook codebase to ES classes. | ||
* We plan to enable property initializer syntax internally, and codemod all methods except lifecycles to it. | ||
|
||
------------ | ||
|
||
Please feel free to discuss these notes in the [corresponding pull request](https://github.com/reactjs/core-notes/pull/18). |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This seems really clean. I like the
<*Component />
syntax, though I thinkpure={true}
better fits what my expectation would be if someone asked me 'how do you tell if a component implements (formerly) PureRenderMixin?'The downside would be that the calling component must have additional information on the expectations of the component. I suspect folks would just end up defining
pure: true
ingetDefaultProps
.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.
@spicyj @jimfb @sebmarkbage
Has there been a discussion of pros and cons of the third approach? I missed the meeting so I can’t tell 😄 .