hanzoai/react
2
0
Fork 0
mirror of https://github.com/hanzoai/react.git synced 2026-09-24 17:02:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

Try rendering again if a timed out tree receives an update (#13921)

Browse Source 
Found a bug related to suspending inside an already mounted tree. While
investigating this I noticed we really don't have much coverage of
suspended updates. I think this would greatly benefit from some fuzz
testing; still haven't thought of a good test case, though.
This commit is contained in:
Andrew Clark
2018-10-22 22:37:15 -07:00
committed by GitHub
parent 883973451b
commit fce91d89ec
5 changed files with 226 additions and 137 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/jest-react/src/JestReact.js
+5 -7
View File
@@ -35,19 +35,17 @@ function assertYieldsWereCleared(root) {
}
export function toFlushAndYield(root, expectedYields) {
assertYieldsWereCleared(root);
const actualYields = root.unstable_flushAll();
return captureAssertion(() => {
assertYieldsWereCleared(root);
const actualYields = root.unstable_flushAll();
expect(actualYields).toEqual(expectedYields);
});
}
export function toFlushAndYieldThrough(root, expectedYields) {
assertYieldsWereCleared(root);
const actualYields = root.unstable_flushNumberOfYields(expectedYields.length);
return captureAssertion(() => {
assertYieldsWereCleared(root);
const actualYields = root.unstable_flushNumberOfYields(
expectedYields.length,
);
expect(actualYields).toEqual(expectedYields);
});
}
@@ -76,8 +74,8 @@ export function toHaveYielded(ReactTestRenderer, expectedYields) {
}
export function toFlushAndThrow(root, ...rest) {
assertYieldsWereCleared(root);
return captureAssertion(() => {
assertYieldsWereCleared(root);
expect(() => {
root.unstable_flushAll();
}).toThrow(...rest);
packages/react-reconciler/src/ReactFiberBeginWork.js
Vendored
+39 -20
View File
@@ -1018,20 +1018,16 @@ function updateSuspenseComponent(
// We should attempt to render the primary children unless this boundary
// already suspended during this render (`alreadyCaptured` is true).
let nextState: SuspenseState | null = workInProgress.memoizedState;
let nextDidTimeout;
if (nextState === null) {
// An empty suspense state means this boundary has not yet timed out.
nextDidTimeout = false;
} else {
if (!nextState.alreadyCaptured) {
// Since we haven't already suspended during this commit, clear the
// existing suspense state. We'll try rendering again.
nextDidTimeout = false;
nextState = null;
} else {
// Something in this boundary's subtree already suspended. Switch to
// rendering the fallback children. Set `alreadyCaptured` to true.
nextDidTimeout = true;
if (current !== null && nextState === current.memoizedState) {
// Create a new suspense state to avoid mutating the current tree's.
nextState = {
@@ -1046,6 +1042,7 @@ function updateSuspenseComponent(
}
}
}
const nextDidTimeout = nextState !== null && nextState.didTimeout;
// This next part is a bit confusing. If the children timeout, we switch to
// showing the fallback children in place of the "primary" children.
@@ -1127,8 +1124,8 @@ function updateSuspenseComponent(
// its fragment. We're going to skip over these entirely.
const nextFallbackChildren = nextProps.fallback;
const primaryChildFragment = createWorkInProgress(
currentFallbackChildFragment,
currentFallbackChildFragment.pendingProps,
currentPrimaryChildFragment,
currentPrimaryChildFragment.pendingProps,
NoWork,
);
primaryChildFragment.effectTag |= Placement;
@@ -1481,23 +1478,45 @@ function beginWork(
}
break;
case SuspenseComponent: {
const child = bailoutOnAlreadyFinishedWork(
current,
workInProgress,
renderExpirationTime,
);
if (child !== null) {
const nextState = workInProgress.memoizedState;
const nextDidTimeout = nextState !== null && nextState.didTimeout;
if (nextDidTimeout) {
child.childExpirationTime = NoWork;
return child.sibling;
const state: SuspenseState | null = workInProgress.memoizedState;
const didTimeout = state !== null && state.didTimeout;
if (didTimeout) {
// If this boundary is currently timed out, we need to decide
// whether to retry the primary children, or to skip over it and
// go straight to the fallback. Check the priority of the primary
// child fragment.
const primaryChildFragment: Fiber = (workInProgress.child: any);
const primaryChildExpirationTime =
primaryChildFragment.childExpirationTime;
if (
primaryChildExpirationTime !== NoWork &&
primaryChildExpirationTime <= renderExpirationTime
) {
// The primary children have pending work. Use the normal path
// to attempt to render the primary children again.
return updateSuspenseComponent(
current,
workInProgress,
renderExpirationTime,
);
} else {
return child;
// The primary children do not have pending work with sufficient
// priority. Bailout.
const child = bailoutOnAlreadyFinishedWork(
current,
workInProgress,
renderExpirationTime,
);
if (child !== null) {
// The fallback children have pending work. Skip over the
// primary children and work on the fallback.
return child.sibling;
} else {
return null;
}
}
} else {
return null;
}
break;
}
}
return bailoutOnAlreadyFinishedWork(
packages/react-reconciler/src/ReactFiberSuspenseComponent.js
Vendored
+15
View File
@@ -7,6 +7,7 @@
* @flow
*/
import type {Fiber} from './ReactFiber';
import type {ExpirationTime} from './ReactFiberExpirationTime';
export type SuspenseState = {|
@@ -23,3 +24,17 @@ export type SuspenseState = {|
// `didTimeout` because it's not set unless the boundary actually commits.
timedOutAt: ExpirationTime,
|};
export function shouldCaptureSuspense(
current: Fiber | null,
workInProgress: Fiber,
): boolean {
// In order to capture, the Suspense component must have a fallback prop.
if (workInProgress.memoizedProps.fallback === undefined) {
return false;
}
// If it was the primary children that just suspended, capture and render the
// fallback. Otherwise, don't capture and bubble to the next boundary.
const nextState: SuspenseState | null = workInProgress.memoizedState;
return nextState === null || !nextState.didTimeout;
}
packages/react-reconciler/src/ReactFiberUnwindWork.js
Vendored
+105 -105
View File
@@ -13,6 +13,7 @@ import type {ExpirationTime} from './ReactFiberExpirationTime';
import type {CapturedValue} from './ReactCapturedValue';
import type {Update} from './ReactUpdateQueue';
import type {Thenable} from './ReactFiberScheduler';
import type {SuspenseState} from './ReactFiberSuspenseComponent';
import {unstable_wrap as Schedule_tracing_wrap} from 'scheduler/tracing';
import getComponentName from 'shared/getComponentName';
@@ -35,6 +36,7 @@ import {
} from 'shared/ReactSideEffectTags';
import {enableSchedulerTracing} from 'shared/ReactFeatureFlags';
import {ConcurrentMode} from './ReactTypeOfMode';
import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent';
import {createCapturedValue} from './ReactCapturedValue';
import {
@@ -170,7 +172,7 @@ function throwException(
if (workInProgress.tag === SuspenseComponent) {
const current = workInProgress.alternate;
if (current !== null) {
const currentState = current.memoizedState;
const currentState: SuspenseState | null = current.memoizedState;
if (currentState !== null && currentState.didTimeout) {
// Reached a boundary that already timed out. Do not search
// any further.
@@ -198,116 +200,113 @@ function throwException(
// Schedule the nearest Suspense to re-render the timed out view.
workInProgress = returnFiber;
do {
if (workInProgress.tag === SuspenseComponent) {
const didTimeout = workInProgress.memoizedState;
if (
!didTimeout &&
workInProgress.memoizedProps.fallback !== undefined
) {
// Found the nearest boundary.
if (
workInProgress.tag === SuspenseComponent &&
shouldCaptureSuspense(workInProgress.alternate, workInProgress)
) {
// Found the nearest boundary.
// If the boundary is not in concurrent mode, we should not suspend, and
// likewise, when the promise resolves, we should ping synchronously.
const pingTime =
(workInProgress.mode & ConcurrentMode) === NoEffect
? Sync
: renderExpirationTime;
// If the boundary is not in concurrent mode, we should not suspend, and
// likewise, when the promise resolves, we should ping synchronously.
const pingTime =
(workInProgress.mode & ConcurrentMode) === NoEffect
? Sync
: renderExpirationTime;
// Attach a listener to the promise to "ping" the root and retry.
let onResolveOrReject = retrySuspendedRoot.bind(
null,
root,
workInProgress,
// Attach a listener to the promise to "ping" the root and retry.
let onResolveOrReject = retrySuspendedRoot.bind(
null,
root,
workInProgress,
sourceFiber,
pingTime,
);
if (enableSchedulerTracing) {
onResolveOrReject = Schedule_tracing_wrap(onResolveOrReject);
}
thenable.then(onResolveOrReject, onResolveOrReject);
// If the boundary is outside of concurrent mode, we should *not*
// suspend the commit. Pretend as if the suspended component rendered
// null and keep rendering. In the commit phase, we'll schedule a
// subsequent synchronous update to re-render the Suspense.
//
// Note: It doesn't matter whether the component that suspended was
// inside a concurrent mode tree. If the Suspense is outside of it, we
// should *not* suspend the commit.
if ((workInProgress.mode & ConcurrentMode) === NoEffect) {
workInProgress.effectTag |= CallbackEffect;
// Unmount the source fiber's children
const nextChildren = null;
reconcileChildren(
sourceFiber.alternate,
sourceFiber,
pingTime,
nextChildren,
renderExpirationTime,
);
if (enableSchedulerTracing) {
onResolveOrReject = Schedule_tracing_wrap(onResolveOrReject);
}
thenable.then(onResolveOrReject, onResolveOrReject);
sourceFiber.effectTag &= ~Incomplete;
// If the boundary is outside of concurrent mode, we should *not*
// suspend the commit. Pretend as if the suspended component rendered
// null and keep rendering. In the commit phase, we'll schedule a
// subsequent synchronous update to re-render the Suspense.
//
// Note: It doesn't matter whether the component that suspended was
// inside a concurrent mode tree. If the Suspense is outside of it, we
// should *not* suspend the commit.
if ((workInProgress.mode & ConcurrentMode) === NoEffect) {
workInProgress.effectTag |= CallbackEffect;
// Unmount the source fiber's children
const nextChildren = null;
reconcileChildren(
sourceFiber.alternate,
sourceFiber,
nextChildren,
renderExpirationTime,
);
sourceFiber.effectTag &= ~Incomplete;
if (sourceFiber.tag === ClassComponent) {
// We're going to commit this fiber even though it didn't
// complete. But we shouldn't call any lifecycle methods or
// callbacks. Remove all lifecycle effect tags.
sourceFiber.effectTag &= ~LifecycleEffectMask;
if (sourceFiber.alternate === null) {
// Set the instance back to null. We use this as a heuristic to
// detect that the fiber mounted in an inconsistent state.
sourceFiber.stateNode = null;
}
if (sourceFiber.tag === ClassComponent) {
// We're going to commit this fiber even though it didn't complete.
// But we shouldn't call any lifecycle methods or callbacks. Remove
// all lifecycle effect tags.
sourceFiber.effectTag &= ~LifecycleEffectMask;
if (sourceFiber.alternate === null) {
// Set the instance back to null. We use this as a heuristic to
// detect that the fiber mounted in an inconsistent state.
sourceFiber.stateNode = null;
}
// Exit without suspending.
return;
}
// Confirmed that the boundary is in a concurrent mode tree. Continue
// with the normal suspend path.
let absoluteTimeoutMs;
if (earliestTimeoutMs === -1) {
// If no explicit threshold is given, default to an abitrarily large
// value. The actual size doesn't matter because the threshold for the
// whole tree will be clamped to the expiration time.
absoluteTimeoutMs = maxSigned31BitInt;
} else {
if (startTimeMs === -1) {
// This suspend happened outside of any already timed-out
// placeholders. We don't know exactly when the update was scheduled,
// but we can infer an approximate start time from the expiration
// time. First, find the earliest uncommitted expiration time in the
// tree, including work that is suspended. Then subtract the offset
// used to compute an async update's expiration time. This will cause
// high priority (interactive) work to expire earlier than necessary,
// but we can account for this by adjusting for the Just Noticeable
// Difference.
const earliestExpirationTime = findEarliestOutstandingPriorityLevel(
root,
renderExpirationTime,
);
const earliestExpirationTimeMs = expirationTimeToMs(
earliestExpirationTime,
);
startTimeMs = earliestExpirationTimeMs - LOW_PRIORITY_EXPIRATION;
}
absoluteTimeoutMs = startTimeMs + earliestTimeoutMs;
}
// Mark the earliest timeout in the suspended fiber's ancestor path.
// After completing the root, we'll take the largest of all the
// suspended fiber's timeouts and use it to compute a timeout for the
// whole tree.
renderDidSuspend(root, absoluteTimeoutMs, renderExpirationTime);
workInProgress.effectTag |= ShouldCapture;
workInProgress.expirationTime = renderExpirationTime;
// Exit without suspending.
return;
}
// This boundary already captured during this render. Continue to the
// next boundary.
// Confirmed that the boundary is in a concurrent mode tree. Continue
// with the normal suspend path.
let absoluteTimeoutMs;
if (earliestTimeoutMs === -1) {
// If no explicit threshold is given, default to an abitrarily large
// value. The actual size doesn't matter because the threshold for the
// whole tree will be clamped to the expiration time.
absoluteTimeoutMs = maxSigned31BitInt;
} else {
if (startTimeMs === -1) {
// This suspend happened outside of any already timed-out
// placeholders. We don't know exactly when the update was
// scheduled, but we can infer an approximate start time from the
// expiration time. First, find the earliest uncommitted expiration
// time in the tree, including work that is suspended. Then subtract
// the offset used to compute an async update's expiration time.
// This will cause high priority (interactive) work to expire
// earlier than necessary, but we can account for this by adjusting
// for the Just Noticeable Difference.
const earliestExpirationTime = findEarliestOutstandingPriorityLevel(
root,
renderExpirationTime,
);
const earliestExpirationTimeMs = expirationTimeToMs(
earliestExpirationTime,
);
startTimeMs = earliestExpirationTimeMs - LOW_PRIORITY_EXPIRATION;
}
absoluteTimeoutMs = startTimeMs + earliestTimeoutMs;
}
// Mark the earliest timeout in the suspended fiber's ancestor path.
// After completing the root, we'll take the largest of all the
// suspended fiber's timeouts and use it to compute a timeout for the
// whole tree.
renderDidSuspend(root, absoluteTimeoutMs, renderExpirationTime);
workInProgress.effectTag |= ShouldCapture;
workInProgress.expirationTime = renderExpirationTime;
return;
}
// This boundary already captured during this render. Continue to the next
// boundary.
workInProgress = workInProgress.return;
} while (workInProgress !== null);
// No boundary was found. Fallthrough to error mode.
@@ -407,9 +406,10 @@ function unwindWork(
// Captured a suspense effect. Set the boundary's `alreadyCaptured`
// state to true so we know to render the fallback.
const current = workInProgress.alternate;
const currentState = current !== null ? current.memoizedState : null;
let nextState = workInProgress.memoizedState;
if (currentState === null) {
const currentState: SuspenseState | null =
current !== null ? current.memoizedState : null;
let nextState: SuspenseState | null = workInProgress.memoizedState;
if (nextState === null) {
// No existing state. Create a new object.
nextState = {
alreadyCaptured: true,
@@ -421,8 +421,8 @@ function unwindWork(
// Clone the object.
nextState = {
alreadyCaptured: true,
didTimeout: currentState.didTimeout,
timedOutAt: currentState.timedOutAt,
didTimeout: nextState.didTimeout,
timedOutAt: nextState.timedOutAt,
};
} else {
// Already have a clone, so it's safe to mutate.
packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js
+62 -5
View File
@@ -442,7 +442,7 @@ describe('ReactSuspense', () => {
state = {step: 1};
render() {
instance = this;
return <Text text="Stateful" />;
return <Text text={`Stateful: ${this.state.step}`} />;
}
}
@@ -458,30 +458,87 @@ describe('ReactSuspense', () => {
const root = ReactTestRenderer.create(<App text="A" />);
expect(ReactTestRenderer).toHaveYielded([
'Stateful',
'Stateful: 1',
'Suspend! [A]',
'Loading...',
]);
jest.advanceTimersByTime(1000);
expect(ReactTestRenderer).toHaveYielded(['Promise resolved [A]', 'A']);
expect(root).toMatchRenderedOutput('StatefulA');
expect(root).toMatchRenderedOutput('Stateful: 1A');
root.update(<App text="B" />);
expect(ReactTestRenderer).toHaveYielded([
'Stateful',
'Stateful: 1',
'Suspend! [B]',
'Loading...',
]);
expect(root).toMatchRenderedOutput('Loading...');
instance.setState({step: 2});
expect(ReactTestRenderer).toHaveYielded([
'Stateful: 2',
'Suspend! [B]',
'Loading...',
]);
expect(root).toMatchRenderedOutput('Loading...');
jest.advanceTimersByTime(1000);
expect(ReactTestRenderer).toHaveYielded([
'Promise resolved [B]',
'Stateful',
'B',
'B',
]);
expect(root).toMatchRenderedOutput('Stateful: 2B');
});
it('retries when an update is scheduled on a timed out tree', () => {
let instance;
class Stateful extends React.Component {
state = {step: 1};
render() {
instance = this;
return <AsyncText ms={1000} text={`Step: ${this.state.step}`} />;
}
}
function App(props) {
return (
<Suspense maxDuration={10} fallback={<Text text="Loading..." />}>
<Stateful />
</Suspense>
);
}
const root = ReactTestRenderer.create(<App />, {
unstable_isConcurrent: true,
});
// Initial render
expect(root).toFlushAndYield(['Suspend! [Step: 1]', 'Loading...']);
jest.advanceTimersByTime(1000);
expect(ReactTestRenderer).toHaveYielded(['Promise resolved [Step: 1]']);
expect(root).toFlushAndYield(['Step: 1']);
expect(root).toMatchRenderedOutput('Step: 1');
// Update that suspends
instance.setState({step: 2});
expect(root).toFlushAndYield(['Suspend! [Step: 2]', 'Loading...']);
jest.advanceTimersByTime(500);
expect(root).toMatchRenderedOutput('Loading...');
// Update while still suspended
instance.setState({step: 3});
expect(root).toFlushAndYield(['Suspend! [Step: 3]']);
expect(root).toMatchRenderedOutput('Loading...');
jest.advanceTimersByTime(1000);
expect(ReactTestRenderer).toHaveYielded([
'Promise resolved [Step: 2]',
'Promise resolved [Step: 3]',
]);
expect(root).toFlushAndYield(['Step: 3']);
expect(root).toMatchRenderedOutput('Step: 3');
});
});
});