Strict mode and default mode should have same Suspense semantics (#13882)
In the default mode, Suspense has special semantics where, in addition to timing out immediately, we don't unwind the stack before rendering the fallback. Instead, we commit the tree in an inconsistent state, then synchronous render *again* to switch to the fallback. This is slower but is less likely to cause issues with older components that perform side effects in the render phase (e.g. componentWillMount, componentWillUpdate, and componentWillReceiveProps). We should do this in strict mode, too, so that there are no semantic differences (in prod, at least) between default mode and strict mode. The rationale is that it makes it easier to wrap a tree in strict mode and start migrating components incrementally without worrying about new bugs in production.
Andrew Clark committed
Oct 18, 2018 at 15:42 UTC
55b82794237a14a5cf106f61d84237dc9a3e04f6
3 files changed
+68
-70
packages/react-reconciler/src/ReactFiberScheduler.js
+5
-10
@@ -110,12 +110,7 @@ import {
110
computeAsyncExpiration,
111
computeInteractiveExpiration,
112
} from './ReactFiberExpirationTime';
113
-import {
114
- ConcurrentMode,
115
- ProfileMode,
116
- NoContext,
117
- StrictMode,
118
-} from './ReactTypeOfMode';
113
+import {ConcurrentMode, ProfileMode, NoContext} from './ReactTypeOfMode';
114
import {enqueueUpdate, resetCurrentlyProcessingQueue} from './ReactUpdateQueue';
115
import {createCapturedValue} from './ReactCapturedValue';
116
import {
@@ -1602,10 +1597,10 @@ function retrySuspendedRoot(
1597
}
1598
1599
scheduleWorkToRoot(boundaryFiber, retryTime);
1605
- if ((boundaryFiber.mode & StrictMode) === NoContext) {
1606
- // Outside of strict mode, we must schedule an update on the source fiber,
1607
- // too, since it already committed in an inconsistent state and therefore
1608
- // does not have any pending work.
1600
+ if ((boundaryFiber.mode & ConcurrentMode) === NoContext) {
1601
+ // Outside of concurrent mode, we must schedule an update on the source
1602
+ // fiber, too, since it already committed in an inconsistent state and
1603
+ // therefore does not have any pending work.
1604
scheduleWorkToRoot(sourceFiber, retryTime);
1605
}
1606
packages/react-reconciler/src/ReactFiberUnwindWork.js
+9
-9
@@ -36,7 +36,7 @@ import {
36
LifecycleEffectMask,
37
} from 'shared/ReactSideEffectTags';
38
import {enableSchedulerTracing} from 'shared/ReactFeatureFlags';
39
-import {StrictMode, ConcurrentMode} from './ReactTypeOfMode';
39
+import {ConcurrentMode} from './ReactTypeOfMode';
40
41
import {createCapturedValue} from './ReactCapturedValue';
42
import {
@@ -230,15 +230,15 @@ function throwException(
230
}
231
thenable.then(onResolveOrReject, onResolveOrReject);
232
233
- // If the boundary is outside of strict mode, we should *not* suspend
234
- // the commit. Pretend as if the suspended component rendered null and
235
- // keep rendering. In the commit phase, we'll schedule a subsequent
236
- // synchronous update to re-render the Suspense.
233
+ // If the boundary is outside of concurrent mode, we should *not*
234
+ // suspend the commit. Pretend as if the suspended component rendered
235
+ // null and keep rendering. In the commit phase, we'll schedule a
236
+ // subsequent synchronous update to re-render the Suspense.
237
//
238
// Note: It doesn't matter whether the component that suspended was
239
- // inside a strict mode tree. If the Suspense is outside of it, we
239
+ // inside a concurrent mode tree. If the Suspense is outside of it, we
240
// should *not* suspend the commit.
241
- if ((workInProgress.mode & StrictMode) === NoEffect) {
241
+ if ((workInProgress.mode & ConcurrentMode) === NoEffect) {
242
workInProgress.effectTag |= CallbackEffect;
243
244
// Unmount the source fiber's children
@@ -274,8 +274,8 @@ function throwException(
274
return;
275
}
276
277
- // Confirmed that the boundary is in a strict mode tree. Continue with
278
- // the normal suspend path.
277
+ // Confirmed that the boundary is in a concurrent mode tree. Continue
278
+ // with the normal suspend path.
279
280
let absoluteTimeoutMs;
281
if (earliestTimeoutMs === -1) {
packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js
+54
-51
@@ -1023,7 +1023,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
1023
1024
it(
1025
'continues rendering asynchronously even if a promise is captured by ' +
1026
- 'a sync boundary (strict)',
1026
+ 'a sync boundary (default mode)',
1027
async () => {
1028
class UpdatingText extends React.Component {
1029
state = {text: this.props.initialText};
@@ -1036,7 +1036,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
1036
const text2 = React.createRef(null);
1037
function App() {
1038
return (
1039
- <StrictMode>
1039
+ <Fragment>
1040
<Suspense
1041
maxDuration={1000}
1042
fallback={<Text text="Loading..." />}>
@@ -1063,7 +1063,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
1063
)}
1064
</UpdatingText>
1065
</ConcurrentMode>
1066
- </StrictMode>
1066
+ </Fragment>
1067
);
1068
}
1069
@@ -1076,25 +1076,26 @@ describe('ReactSuspenseWithNoopRenderer', () => {
1076
'Before',
1077
'Suspend! [Async: 1]',
1078
'After',
1079
- 'Loading...',
1079
'Before',
1080
'Sync: 1',
1081
'After',
1082
'Did mount',
1083
+ // The placeholder is rendered in a subsequent commit
1084
+ 'Loading...',
1085
'Promise resolved [Async: 1]',
1085
- 'Before',
1086
'Async: 1',
1087
- 'After',
1088
- ]);
1089
- expect(ReactNoop.getChildren()).toEqual([
1090
- span('Before'),
1091
- span('Async: 1'),
1092
- span('After'),
1093
-
1094
- span('Before'),
1095
- span('Sync: 1'),
1096
- span('After'),
1087
]);
1088
+ expect(ReactNoop.getChildrenAsJSX()).toEqual(
1089
+ <React.Fragment>
1090
+ <span prop="Before" />
1091
+ <span prop="Async: 1" />
1092
+ <span prop="After" />
1093
+
1094
+ <span prop="Before" />
1095
+ <span prop="Sync: 1" />
1096
+ <span prop="After" />
1097
+ </React.Fragment>,
1098
+ );
1099
1100
// Update. This starts out asynchronously.
1101
text1.current.setState({text: 'Async: 2'}, () =>
@@ -1105,59 +1106,61 @@ describe('ReactSuspenseWithNoopRenderer', () => {
1106
);
1107
1108
// Start rendering asynchronously
1108
- ReactNoop.flushThrough([
1109
- 'Before',
1109
+ ReactNoop.flushThrough(['Before']);
1110
+
1111
+ // Now render the next child, which suspends
1112
+ expect(ReactNoop.flushNextYield()).toEqual([
1113
// This child suspends
1114
'Suspend! [Async: 2]',
1112
- // But we can still render the rest of the async tree asynchronously
1113
- 'After',
1115
]);
1115
-
1116
- // Suspend during an async render.
1117
- expect(ReactNoop.flushNextYield()).toEqual(['Loading...']);
1118
- expect(ReactNoop.flush()).toEqual(['Before', 'Sync: 2', 'After']);
1119
- // Commit was suspended.
1120
- expect(ReactNoop.getChildren()).toEqual([
1121
- span('Before'),
1122
- span('Async: 1'),
1123
- span('After'),
1124
-
1125
- span('Before'),
1126
- span('Sync: 1'),
1127
- span('After'),
1128
- ]);
1129
-
1130
- // When the placeholder is pinged, the boundary re-
1131
- // renders asynchronously.
1132
- ReactNoop.expire(100);
1133
- await advanceTimers(100);
1116
expect(ReactNoop.flush()).toEqual([
1135
- 'Promise resolved [Async: 2]',
1136
- 'Before',
1137
- 'Async: 2',
1117
'After',
1118
'Before',
1119
'Sync: 2',
1120
'After',
1121
'Update 1 did commit',
1122
'Update 2 did commit',
1123
+
1124
+ // Switch to the placeholder in a subsequent commit
1125
+ 'Loading...',
1126
]);
1127
+ expect(ReactNoop.getChildrenAsJSX()).toEqual(
1128
+ <React.Fragment>
1129
+ <span hidden={true} prop="Before" />
1130
+ <span hidden={true} prop="After" />
1131
+ <span prop="Loading..." />
1132
1146
- expect(ReactNoop.getChildren()).toEqual([
1147
- span('Before'),
1148
- span('Async: 2'),
1149
- span('After'),
1133
+ <span prop="Before" />
1134
+ <span prop="Sync: 2" />
1135
+ <span prop="After" />
1136
+ </React.Fragment>,
1137
+ );
1138
1151
- span('Before'),
1152
- span('Sync: 2'),
1153
- span('After'),
1139
+ // When the placeholder is pinged, the boundary must be re-rendered
1140
+ // synchronously.
1141
+ await advanceTimers(100);
1142
+ expect(ReactNoop.clearYields()).toEqual([
1143
+ 'Promise resolved [Async: 2]',
1144
+ 'Async: 2',
1145
]);
1146
+
1147
+ expect(ReactNoop.getChildrenAsJSX()).toEqual(
1148
+ <React.Fragment>
1149
+ <span prop="Before" />
1150
+ <span prop="Async: 2" />
1151
+ <span prop="After" />
1152
+
1153
+ <span prop="Before" />
1154
+ <span prop="Sync: 2" />
1155
+ <span prop="After" />
1156
+ </React.Fragment>,
1157
+ );
1158
},
1159
);
1160
1161
it(
1162
'continues rendering asynchronously even if a promise is captured by ' +
1160
- 'a sync boundary (loose)',
1163
+ 'a sync boundary (strict, non-concurrent)',
1164
async () => {
1165
class UpdatingText extends React.Component {
1166
state = {text: this.props.initialText};
@@ -1170,7 +1173,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
1173
const text2 = React.createRef(null);
1174
function App() {
1175
return (
1173
- <Fragment>
1176
+ <StrictMode>
1177
<Suspense
1178
maxDuration={1000}
1179
fallback={<Text text="Loading..." />}>
@@ -1197,7 +1200,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
1200
)}
1201
</UpdatingText>
1202
</ConcurrentMode>
1200
- </Fragment>
1203
+ </StrictMode>
1204
);
1205
}
1206