Don't call componentDidUpdate if shouldComponentUpdate returns false
This fix relies on the props and state objects being different to know if we can avoid a componentDidUpdate. This is not a great solution because we actually want to use the new props/state object even if sCU returns false. That's the current semantics and it can actually be important because new rerenders that are able to reuse props objects are more likely to have the new props object so we won't be able to quickly bail out next time. I don't have a better idea atm though.
Sebastian Markbage committed
Oct 25, 2016 at 15:21 UTC
46bde584867b3eaebe917ef0c742badc1f84bcb6
2 files changed
+107
-1
src/renderers/shared/fiber/ReactFiberCompleteWork.js
+8
-1
@@ -129,7 +129,14 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
129
// Transfer update queue to callbackList field so callbacks can be
130
// called during commit phase.
131
workInProgress.callbackList = workInProgress.updateQueue;
132
- markUpdate(workInProgress);
132
+ if (current) {
133
+ if (current.memoizedProps !== workInProgress.memoizedProps ||
134
+ current.memoizedState !== workInProgress.memoizedState) {
135
+ markUpdate(workInProgress);
136
+ }
137
+ } else {
138
+ markUpdate(workInProgress);
139
+ }
140
return null;
141
case HostContainer:
142
transferOutput(workInProgress.child, workInProgress);
src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
+99
@@ -1281,4 +1281,103 @@ describe('ReactIncremental', () => {
1281
1282
});
1283
1284
+ it('skips will/DidUpdate when bailing unless an update was already in progress', () => {
1285
+ var ops = [];
1286
+
1287
+ class LifeCycle extends React.Component {
1288
+ componentWillMount() {
1289
+ ops.push('componentWillMount');
1290
+ }
1291
+ componentDidMount() {
1292
+ ops.push('componentDidMount');
1293
+ }
1294
+ componentWillReceiveProps(nextProps) {
1295
+ ops.push('componentWillReceiveProps');
1296
+ }
1297
+ shouldComponentUpdate(nextProps) {
1298
+ ops.push('shouldComponentUpdate');
1299
+ // Bail
1300
+ return this.props.x !== nextProps.x;
1301
+ }
1302
+ componentWillUpdate(nextProps) {
1303
+ ops.push('componentWillUpdate');
1304
+ }
1305
+ componentDidUpdate(prevProps) {
1306
+ ops.push('componentDidUpdate');
1307
+ }
1308
+ render() {
1309
+ ops.push('render');
1310
+ return <span />;
1311
+ }
1312
+ }
1313
+
1314
+ function Sibling() {
1315
+ ops.push('render sibling');
1316
+ return <span />;
1317
+ }
1318
+
1319
+ function App(props) {
1320
+ return [
1321
+ <LifeCycle x={props.x} />,
1322
+ <Sibling />,
1323
+ ];
1324
+ }
1325
+
1326
+ ReactNoop.render(<App x={0} />);
1327
+ ReactNoop.flush();
1328
+
1329
+ expect(ops).toEqual([
1330
+ 'componentWillMount',
1331
+ 'render',
1332
+ 'render sibling',
1333
+ 'componentDidMount',
1334
+ ]);
1335
+
1336
+ ops = [];
1337
+
1338
+ // Update to same props
1339
+ ReactNoop.render(<App x={0} />);
1340
+ ReactNoop.flush();
1341
+
1342
+ expect(ops).toEqual([
1343
+ 'componentWillReceiveProps',
1344
+ 'shouldComponentUpdate',
1345
+ // no componentWillUpdate
1346
+ // no render
1347
+ 'render sibling',
1348
+ // no componentDidUpdate
1349
+ ]);
1350
+
1351
+ ops = [];
1352
+
1353
+ // Begin updating to new props...
1354
+ ReactNoop.render(<App x={1} />);
1355
+ ReactNoop.flushDeferredPri(30);
1356
+
1357
+ expect(ops).toEqual([
1358
+ 'componentWillReceiveProps',
1359
+ 'shouldComponentUpdate',
1360
+ 'componentWillUpdate',
1361
+ 'render',
1362
+ 'render sibling',
1363
+ // no componentDidUpdate yet
1364
+ ]);
1365
+
1366
+ ops = [];
1367
+
1368
+ // ...but we'll interrupt it to rerender the same props.
1369
+ ReactNoop.render(<App x={1} />);
1370
+ ReactNoop.flush();
1371
+
1372
+ // We can bail out this time, but we must call componentDidUpdate.
1373
+ expect(ops).toEqual([
1374
+ 'componentWillReceiveProps',
1375
+ 'shouldComponentUpdate',
1376
+ // no componentWillUpdate
1377
+ // no render
1378
+ 'render sibling',
1379
+ 'componentDidUpdate',
1380
+ ]);
1381
+ });
1382
+
1383
});