[Fiber] PureComponent #8118
[Fiber] PureComponent #8118
Conversation
| } | ||
|
|
||
| const type = workInProgress.type; | ||
| if (type.prototype && type.prototype.isPureReactComponent) { |
There was a problem hiding this comment.
Sorta unfortunate if we need to check this every time. @sebmarkbage is there a place we could store this flag on the fiber?
There was a problem hiding this comment.
We could add a new TypeOfWork https://github.com/facebook/react/blob/master/src/renderers/shared/fiber/ReactTypeOfWork.js
There was a problem hiding this comment.
I figured we start off with all of these being detected on demand and then we can create optimized paths for common patterns. E.g. for things that have shouldComponentUpdate, and things that are pure components and things that have no update life-cycles etc. by storing that in the tag like @acdlite said.
There was a problem hiding this comment.
Ideally the element itself could already have this information in its tag so that we don't even have to detect this during mount.
There was a problem hiding this comment.
However, this can explode number of code paths so I figured we might want to optimize for collections of common patterns. For example: isPureComponent + no life-cycles. isPureComponent + only componentDidUpdate/componentWillUnmount but no other life-cycles. That way we can branch early and not do any of the checks. Never schedule update effects etc. EDIT: Although I guess it could also be a bitmask that allows this check to be optimized regardless of other early branches.
There was a problem hiding this comment.
I like the bitmask idea
There was a problem hiding this comment.
Maybe let's wait on the bitmap thing for now until we know the layout we want for it.
There was a problem hiding this comment.
Oh yeah I didn't mean I would add it now :D
| !(updateQueue && updateQueue.isForced) && | ||
| workInProgress.memoizedProps !== null && | ||
| !instance.shouldComponentUpdate(newProps, newState)) { | ||
| if (!checkShouldComponentUpdate(workInProgress, workInProgress.memoizedProps, newProps, newState)) { |
| @@ -73,6 +74,28 @@ module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : Priori | |||
| }, | |||
| }; | |||
|
|
|||
| function checkShouldComponentUpdate(workInProgress, oldProps, newProps, newState) { | |||
| if (oldProps === null || | |||
There was a problem hiding this comment.
formatting please:
if (
oldProps === null ||
(workInProgress.updateQueue && workInProgress.updateQueue.isForced)
) {
(Also, may as well pull updateQueue out to a var like before?)
There was a problem hiding this comment.
or meh, you can leave it if you like the other way. we're inconsistent.
acdlite
force-pushed
the
fiberpurecomponent
branch
2 times, most recently
from
0ea690a to
ecbe4fa
Compare
|
One more lint. |
Passes existing PureComponent tests
acdlite
force-pushed
the
fiberpurecomponent
branch
from
ecbe4fa to
59c9464
Compare
|
My bad, I had my ESLint plugin disabled. |
Pretty straightforward. Passes existing tests.