-
Notifications
You must be signed in to change notification settings - Fork 48.8k
[Fiber] Simplify coroutines by making yields stateless #8840
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
Changes from all commits
648d6e1
fd8d5f7
e8500fb
754f72a
975ad92
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,7 +29,6 @@ var { | |
} = require('ReactPortal'); | ||
|
||
var ReactFiber = require('ReactFiber'); | ||
var ReactReifiedYield = require('ReactReifiedYield'); | ||
var ReactTypeOfSideEffect = require('ReactTypeOfSideEffect'); | ||
var ReactTypeOfWork = require('ReactTypeOfWork'); | ||
|
||
|
@@ -53,11 +52,6 @@ const { | |
createFiberFromPortal, | ||
} = ReactFiber; | ||
|
||
const { | ||
createReifiedYield, | ||
createUpdatedReifiedYield, | ||
} = ReactReifiedYield; | ||
|
||
const isArray = Array.isArray; | ||
|
||
const { | ||
|
@@ -316,21 +310,16 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { | |
yieldNode : ReactYield, | ||
priority : PriorityLevel | ||
) : Fiber { | ||
// TODO: Should this also compare continuation to determine whether to reuse? | ||
if (current == null || current.tag !== YieldComponent) { | ||
// Insert | ||
const reifiedYield = createReifiedYield(yieldNode); | ||
const created = createFiberFromYield(yieldNode, priority); | ||
created.type = reifiedYield; | ||
created.type = yieldNode.value; | ||
created.return = returnFiber; | ||
return created; | ||
} else { | ||
// Move based on index | ||
const existing = useFiber(current, priority); | ||
existing.type = createUpdatedReifiedYield( | ||
current.type, | ||
yieldNode | ||
); | ||
existing.type = yieldNode.value; | ||
existing.return = returnFiber; | ||
return existing; | ||
} | ||
|
@@ -411,9 +400,8 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { | |
} | ||
|
||
case REACT_YIELD_TYPE: { | ||
const reifiedYield = createReifiedYield(newChild); | ||
const created = createFiberFromYield(newChild, priority); | ||
created.type = reifiedYield; | ||
created.type = newChild.value; | ||
created.return = returnFiber; | ||
return created; | ||
} | ||
|
@@ -474,7 +462,10 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { | |
} | ||
|
||
case REACT_YIELD_TYPE: { | ||
if (newChild.key === key) { | ||
// Yields doesn't have keys. If the previous node is implicitly keyed | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: don't have keys |
||
// we can continue to replace it without aborting even if it is not a | ||
// yield. | ||
if (key === null) { | ||
return updateYield(returnFiber, oldFiber, newChild, priority); | ||
} else { | ||
return null; | ||
|
@@ -527,9 +518,9 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { | |
} | ||
|
||
case REACT_YIELD_TYPE: { | ||
const matchedFiber = existingChildren.get( | ||
newChild.key === null ? newIdx : newChild.key | ||
) || null; | ||
// Yields doesn't have keys, so we neither have to check the old nor | ||
// new node for the key. If both are yields, they match. | ||
const matchedFiber = existingChildren.get(newIdx) || null; | ||
return updateYield(returnFiber, matchedFiber, newChild, priority); | ||
} | ||
|
||
|
@@ -561,7 +552,6 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { | |
switch (child.$$typeof) { | ||
case REACT_ELEMENT_TYPE: | ||
case REACT_COROUTINE_TYPE: | ||
case REACT_YIELD_TYPE: | ||
case REACT_PORTAL_TYPE: | ||
const key = child.key; | ||
if (typeof key !== 'string') { | ||
|
@@ -1019,34 +1009,22 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { | |
yieldNode : ReactYield, | ||
priority : PriorityLevel | ||
) : Fiber { | ||
const key = yieldNode.key; | ||
// There's no need to check for keys on yields since they're stateless. | ||
let child = currentFirstChild; | ||
while (child) { | ||
// TODO: If key === null and child.key === null, then this only applies to | ||
// the first item in the list. | ||
if (child.key === key) { | ||
if (child.tag === YieldComponent) { | ||
deleteRemainingChildren(returnFiber, child.sibling); | ||
const existing = useFiber(child, priority); | ||
existing.type = createUpdatedReifiedYield( | ||
child.type, | ||
yieldNode | ||
); | ||
existing.return = returnFiber; | ||
return existing; | ||
} else { | ||
deleteRemainingChildren(returnFiber, child); | ||
break; | ||
} | ||
if (child) { | ||
if (child.tag === YieldComponent) { | ||
deleteRemainingChildren(returnFiber, child.sibling); | ||
const existing = useFiber(child, priority); | ||
existing.type = yieldNode.value; | ||
existing.return = returnFiber; | ||
return existing; | ||
} else { | ||
deleteChild(returnFiber, child); | ||
deleteRemainingChildren(returnFiber, child); | ||
} | ||
child = child.sibling; | ||
} | ||
|
||
const reifiedYield = createReifiedYield(yieldNode); | ||
const created = createFiberFromYield(yieldNode, priority); | ||
created.type = reifiedYield; | ||
created.type = yieldNode.value; | ||
created.return = returnFiber; | ||
return created; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -516,13 +516,54 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>( | |
} | ||
} | ||
} else if (nextCoroutine === null || workInProgress.memoizedProps === nextCoroutine) { | ||
return bailoutOnAlreadyFinishedWork(current, workInProgress); | ||
nextCoroutine = workInProgress.memoizedProps; | ||
// TODO: When bailing out, we might need to return the stateNode instead | ||
// of the child. To check it for work. | ||
// return bailoutOnAlreadyFinishedWork(current, workInProgress); | ||
} | ||
|
||
const nextChildren = nextCoroutine.children; | ||
const priorityLevel = workInProgress.pendingWorkPriority; | ||
|
||
// The following is a fork of reconcileChildrenAtPriority but using | ||
// stateNode to store the child. | ||
|
||
// At this point any memoization is no longer valid since we'll have changed | ||
// the children. | ||
workInProgress.memoizedProps = null; | ||
if (!current) { | ||
workInProgress.stateNode = mountChildFibersInPlace( | ||
workInProgress, | ||
workInProgress.stateNode, | ||
nextChildren, | ||
priorityLevel | ||
); | ||
} else if (current.child === workInProgress.child) { | ||
clearDeletions(workInProgress); | ||
|
||
workInProgress.stateNode = reconcileChildFibers( | ||
workInProgress, | ||
workInProgress.stateNode, | ||
nextChildren, | ||
priorityLevel | ||
); | ||
|
||
transferDeletions(workInProgress); | ||
} else { | ||
workInProgress.stateNode = reconcileChildFibersInPlace( | ||
workInProgress, | ||
workInProgress.stateNode, | ||
nextChildren, | ||
priorityLevel | ||
); | ||
|
||
transferDeletions(workInProgress); | ||
} | ||
reconcileChildren(current, workInProgress, nextCoroutine.children); | ||
|
||
memoizeProps(workInProgress, nextCoroutine); | ||
// This doesn't take arbitrary time so we could synchronously just begin | ||
// eagerly do the work of workInProgress.child as an optimization. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: probably need to change |
||
return workInProgress.child; | ||
return workInProgress.stateNode; | ||
} | ||
|
||
function updatePortalComponent(current, workInProgress) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,6 @@ import type { Fiber } from 'ReactFiber'; | |
import type { HostContext } from 'ReactFiberHostContext'; | ||
import type { FiberRoot } from 'ReactFiberRoot'; | ||
import type { HostConfig } from 'ReactFiberReconciler'; | ||
import type { ReifiedYield } from 'ReactReifiedYield'; | ||
|
||
var { reconcileChildFibers } = require('ReactChildFiber'); | ||
var { | ||
|
@@ -66,29 +65,40 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>( | |
popHostContainer, | ||
} = hostContext; | ||
|
||
function markChildAsProgressed(current, workInProgress, priorityLevel) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe move to child fiber since it's duplicated between phases? |
||
// We now have clones. Let's store them as the currently progressed work. | ||
workInProgress.progressedChild = workInProgress.child; | ||
workInProgress.progressedPriority = priorityLevel; | ||
if (current) { | ||
// We also store it on the current. When the alternate swaps in we can | ||
// continue from this point. | ||
current.progressedChild = workInProgress.progressedChild; | ||
current.progressedPriority = workInProgress.progressedPriority; | ||
} | ||
} | ||
|
||
function markUpdate(workInProgress : Fiber) { | ||
// Tag the fiber with an update effect. This turns a Placement into | ||
// an UpdateAndPlacement. | ||
workInProgress.effectTag |= Update; | ||
} | ||
|
||
function appendAllYields(yields : Array<ReifiedYield>, workInProgress : Fiber) { | ||
let node = workInProgress.child; | ||
function appendAllYields(yields : Array<mixed>, workInProgress : Fiber) { | ||
let node = workInProgress.stateNode; | ||
if (node) { | ||
node.return = workInProgress; | ||
} | ||
while (node) { | ||
if (node.tag === HostComponent || node.tag === HostText || | ||
node.tag === HostPortal) { | ||
throw new Error('A coroutine cannot have host component children.'); | ||
} else if (node.tag === YieldComponent) { | ||
yields.push(node.type); | ||
} else if (node.child) { | ||
// TODO: Coroutines need to visit the stateNode. | ||
node.child.return = node; | ||
node = node.child; | ||
continue; | ||
} | ||
if (node === workInProgress) { | ||
return; | ||
} | ||
while (!node.sibling) { | ||
if (!node.return || node.return === workInProgress) { | ||
return; | ||
|
@@ -117,22 +127,23 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>( | |
|
||
// Build up the yields. | ||
// TODO: Compare this to a generator or opaque helpers like Children. | ||
var yields : Array<ReifiedYield> = []; | ||
var yields : Array<mixed> = []; | ||
appendAllYields(yields, workInProgress); | ||
var fn = coroutine.handler; | ||
var props = coroutine.props; | ||
var nextChildren = fn(props, yields); | ||
|
||
var currentFirstChild = current ? current.stateNode : null; | ||
var currentFirstChild = current ? current.child : null; | ||
// Inherit the priority of the returnFiber. | ||
const priority = workInProgress.pendingWorkPriority; | ||
workInProgress.stateNode = reconcileChildFibers( | ||
workInProgress.child = reconcileChildFibers( | ||
workInProgress, | ||
currentFirstChild, | ||
nextChildren, | ||
priority | ||
); | ||
return workInProgress.stateNode; | ||
markChildAsProgressed(current, workInProgress, priority); | ||
return workInProgress.child; | ||
} | ||
|
||
function appendAllChildren(parent : I, workInProgress : Fiber) { | ||
|
@@ -147,7 +158,6 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>( | |
// down its children. Instead, we'll get insertions from each child in | ||
// the portal directly. | ||
} else if (node.child) { | ||
// TODO: Coroutines need to visit the stateNode. | ||
node = node.child; | ||
continue; | ||
} | ||
|
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.
Can this break hidden classes if it's a number?
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.
Yes but only in V8. Others use NaN tagging. It's likely already unlike that you'd use it this way. We can warn if it becomes an issue.