Skip to content

Commit

Permalink
Suspense in Batched Mode
Browse files Browse the repository at this point in the history
Should have same semantics as Concurrent Mode.
  • Loading branch information
acdlite committed Apr 25, 2019
1 parent 93487a9 commit 944dd85
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 21 deletions.
13 changes: 7 additions & 6 deletions packages/react-reconciler/src/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ import {
NoMode,
ProfileMode,
StrictMode,
BatchedMode,
} from './ReactTypeOfMode';
import {
shouldSetTextContent,
Expand Down Expand Up @@ -1493,8 +1494,8 @@ function updateSuspenseComponent(
null,
);

if ((workInProgress.mode & ConcurrentMode) === NoMode) {
// Outside of concurrent mode, we commit the effects from the
if ((workInProgress.mode & BatchedMode) === NoMode) {
// Outside of batched mode, we commit the effects from the
// partially completed, timed-out tree, too.
const progressedState: SuspenseState = workInProgress.memoizedState;
const progressedPrimaryChild: Fiber | null =
Expand Down Expand Up @@ -1546,8 +1547,8 @@ function updateSuspenseComponent(
NoWork,
);

if ((workInProgress.mode & ConcurrentMode) === NoMode) {
// Outside of concurrent mode, we commit the effects from the
if ((workInProgress.mode & BatchedMode) === NoMode) {
// Outside of batched mode, we commit the effects from the
// partially completed, timed-out tree, too.
const progressedState: SuspenseState = workInProgress.memoizedState;
const progressedPrimaryChild: Fiber | null =
Expand Down Expand Up @@ -1629,8 +1630,8 @@ function updateSuspenseComponent(
// schedule a placement.
// primaryChildFragment.effectTag |= Placement;

if ((workInProgress.mode & ConcurrentMode) === NoMode) {
// Outside of concurrent mode, we commit the effects from the
if ((workInProgress.mode & BatchedMode) === NoMode) {
// Outside of batched mode, we commit the effects from the
// partially completed, timed-out tree, too.
const progressedState: SuspenseState = workInProgress.memoizedState;
const progressedPrimaryChild: Fiber | null =
Expand Down
6 changes: 3 additions & 3 deletions packages/react-reconciler/src/ReactFiberCompleteWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ import {
EventComponent,
EventTarget,
} from 'shared/ReactWorkTags';
import {ConcurrentMode, NoMode} from './ReactTypeOfMode';
import {NoMode, BatchedMode} from './ReactTypeOfMode';
import {
Placement,
Ref,
Expand Down Expand Up @@ -716,12 +716,12 @@ function completeWork(
}

if (nextDidTimeout && !prevDidTimeout) {
// If this subtreee is running in concurrent mode we can suspend,
// If this subtreee is running in batched mode we can suspend,
// otherwise we won't suspend.
// TODO: This will still suspend a synchronous tree if anything
// in the concurrent tree already suspended during this render.
// This is a known bug.
if ((workInProgress.mode & ConcurrentMode) !== NoMode) {
if ((workInProgress.mode & BatchedMode) !== NoMode) {
renderDidSuspend();
}
}
Expand Down
8 changes: 4 additions & 4 deletions packages/react-reconciler/src/ReactFiberUnwindWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import {
enableSuspenseServerRenderer,
enableEventAPI,
} from 'shared/ReactFeatureFlags';
import {ConcurrentMode, NoMode} from './ReactTypeOfMode';
import {NoMode, BatchedMode} from './ReactTypeOfMode';
import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent';

import {createCapturedValue} from './ReactCapturedValue';
Expand Down Expand Up @@ -223,15 +223,15 @@ function throwException(
thenables.add(thenable);
}

// If the boundary is outside of concurrent mode, we should *not*
// If the boundary is outside of batched 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
// inside a batched mode tree. If the Suspense is outside of it, we
// should *not* suspend the commit.
if ((workInProgress.mode & ConcurrentMode) === NoMode) {
if ((workInProgress.mode & BatchedMode) === NoMode) {
workInProgress.effectTag |= DidCapture;

// We're going to commit this fiber even though it didn't complete.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ let React;
let ReactFeatureFlags;
let ReactNoop;
let Scheduler;
let ReactCache;
let Suspense;
let TextResource;

describe('ReactBatchedMode', () => {
beforeEach(() => {
Expand All @@ -12,13 +15,40 @@ describe('ReactBatchedMode', () => {
React = require('react');
ReactNoop = require('react-noop-renderer');
Scheduler = require('scheduler');
ReactCache = require('react-cache');
Suspense = React.Suspense;

TextResource = ReactCache.unstable_createResource(([text, ms = 0]) => {
return new Promise((resolve, reject) =>
setTimeout(() => {
Scheduler.yieldValue(`Promise resolved [${text}]`);
resolve(text);
}, ms),
);
}, ([text, ms]) => text);
});

function Text(props) {
Scheduler.yieldValue(props.text);
return props.text;
}

function AsyncText(props) {
const text = props.text;
try {
TextResource.read([props.text, props.ms]);
Scheduler.yieldValue(text);
return props.text;
} catch (promise) {
if (typeof promise.then === 'function') {
Scheduler.yieldValue(`Suspend! [${text}]`);
} else {
Scheduler.yieldValue(`Error! [${text}]`);
}
throw promise;
}
}

it('updates flush without yielding in the next event', () => {
const root = ReactNoop.createSyncRoot();

Expand Down Expand Up @@ -55,4 +85,38 @@ describe('ReactBatchedMode', () => {
expect(Scheduler).toFlushExpired(['Hi', 'Layout effect']);
expect(root).toMatchRenderedOutput('Hi');
});

it('uses proper Suspense semantics, not legacy ones', async () => {
const root = ReactNoop.createSyncRoot();
root.render(
<Suspense fallback={<Text text="Loading..." />}>
<span>
<Text text="A" />
</span>
<span>
<AsyncText text="B" />
</span>
<span>
<Text text="C" />
</span>
</Suspense>,
);

expect(Scheduler).toFlushExpired(['A', 'Suspend! [B]', 'C', 'Loading...']);
// In Legacy Mode, A and B would mount in a hidden primary tree. In Batched
// and Concurrent Mode, nothing in the primary tree should mount. But the
// fallback should mount immediately.
expect(root).toMatchRenderedOutput('Loading...');

await jest.advanceTimersByTime(1000);
expect(Scheduler).toHaveYielded(['Promise resolved [B]']);
expect(Scheduler).toFlushExpired(['A', 'B', 'C']);
expect(root).toMatchRenderedOutput(
<React.Fragment>
<span>A</span>
<span>B</span>
<span>C</span>
</React.Fragment>,
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -941,7 +941,7 @@ describe('ReactHooksWithNoopRenderer', () => {
});

it(
'in sync mode, useEffect is deferred and updates finish synchronously ' +
'in legacy mode, useEffect is deferred and updates finish synchronously ' +
'(in a single batch)',
() => {
function Counter(props) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,8 @@ describe('ReactSuspenseFuzz', () => {
resetCache();
ReactNoop.renderLegacySyncRoot(children);
resolveAllTasks();
const syncOutput = ReactNoop.getChildrenAsJSX();
expect(syncOutput).toEqual(expectedOutput);
const legacyOutput = ReactNoop.getChildrenAsJSX();
expect(legacyOutput).toEqual(expectedOutput);
ReactNoop.renderLegacySyncRoot(null);

resetCache();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ describe('ReactSuspensePlaceholder', () => {
});

describe('when suspending during mount', () => {
it('properly accounts for base durations when a suspended times out in a sync tree', () => {
it('properly accounts for base durations when a suspended times out in a legacy tree', () => {
ReactNoop.renderLegacySyncRoot(<App shouldSuspend={true} />);
expect(Scheduler).toHaveYielded([
'App',
Expand Down Expand Up @@ -373,7 +373,7 @@ describe('ReactSuspensePlaceholder', () => {
});

describe('when suspending during update', () => {
it('properly accounts for base durations when a suspended times out in a sync tree', () => {
it('properly accounts for base durations when a suspended times out in a legacy tree', () => {
ReactNoop.renderLegacySyncRoot(
<App shouldSuspend={false} textRenderDuration={5} />,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -869,7 +869,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
);
});

describe('sync mode', () => {
describe('legacy mode mode', () => {
it('times out immediately', async () => {
function App() {
return (
Expand Down Expand Up @@ -977,7 +977,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {

it(
'continues rendering asynchronously even if a promise is captured by ' +
'a sync boundary (default mode)',
'a sync boundary (legacy mode)',
async () => {
class UpdatingText extends React.Component {
state = {text: this.props.initialText};
Expand Down Expand Up @@ -1109,7 +1109,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {

it(
'continues rendering asynchronously even if a promise is captured by ' +
'a sync boundary (strict, non-concurrent)',
'a sync boundary (strict, legacy)',
async () => {
class UpdatingText extends React.Component {
state = {text: this.props.initialText};
Expand Down

0 comments on commit 944dd85

Please sign in to comment.