@samitouri / QOS-React / commits / 7f68544f0d

New feature flags to help detect unexpected lifecycle side effects (#11587)

Added `debugRenderPhaseSideEffects` feature flag to help detect unexpected side effects in pre-commit lifecycle hooks and `setState` reducers.

Brian Vaughn committed Nov 17, 2017 at 10:49 UTC 7f68544f0dabda3c27c775eb10067c97b49e313b
8 files changed +167 -2
packages/react-cs-renderer/src/ReactNativeCSFeatureFlags.js
+1
@@ -12,6 +12,7 @@ import invariant from 'fbjs/lib/invariant';
12 import typeof * as FeatureFlagsType from 'shared/ReactFeatureFlags';
13 import typeof * as CSFeatureFlagsType from './ReactNativeCSFeatureFlags';
14
15 +export const debugRenderPhaseSideEffects = false;
16 export const enableAsyncSubtreeAPI = true;
17 export const enableAsyncSchedulingByDefaultInReactDOM = false;
18 export const enableReactFragment = false;
packages/react-reconciler/src/ReactFiberBeginWork.js
+7
@@ -36,6 +36,7 @@ import {
36 Ref,
37 } from 'shared/ReactTypeOfSideEffect';
38 import {ReactCurrentOwner} from 'shared/ReactGlobalSharedState';
39 +import {debugRenderPhaseSideEffects} from 'shared/ReactFeatureFlags';
40 import invariant from 'fbjs/lib/invariant';
41 import getComponentName from 'shared/getComponentName';
42 import warning from 'fbjs/lib/warning';
@@ -269,8 +270,14 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
270 if (__DEV__) {
271 ReactDebugCurrentFiber.setCurrentPhase('render');
272 nextChildren = instance.render();
273 + if (debugRenderPhaseSideEffects) {
274 + instance.render();
275 + }
276 ReactDebugCurrentFiber.setCurrentPhase(null);
277 } else {
278 + if (debugRenderPhaseSideEffects) {
279 + instance.render();
280 + }
281 nextChildren = instance.render();
282 }
283 // React DevTools reads this flag.
packages/react-reconciler/src/ReactFiberClassComponent.js
+24 -2
@@ -11,7 +11,10 @@ import type {Fiber} from './ReactFiber';
11 import type {ExpirationTime} from './ReactFiberExpirationTime';
12
13 import {Update} from 'shared/ReactTypeOfSideEffect';
14 -import {enableAsyncSubtreeAPI} from 'shared/ReactFeatureFlags';
14 +import {
15 + debugRenderPhaseSideEffects,
16 + enableAsyncSubtreeAPI,
17 +} from 'shared/ReactFeatureFlags';
18 import {isMounted} from 'shared/ReactFiberTreeReflection';
19 import * as ReactInstanceMap from 'shared/ReactInstanceMap';
20 import emptyObject from 'fbjs/lib/emptyObject';
@@ -168,6 +171,11 @@ export default function(
171 );
172 stopPhaseTimer();
173
174 + // Simulate an async bailout/interruption by invoking lifecycle twice.
175 + if (debugRenderPhaseSideEffects) {
176 + instance.shouldComponentUpdate(newProps, newState, newContext);
177 + }
178 +
179 if (__DEV__) {
180 warning(
181 shouldUpdate !== undefined,
@@ -374,9 +382,13 @@ export default function(
382 startPhaseTimer(workInProgress, 'componentWillMount');
383 const oldState = instance.state;
384 instance.componentWillMount();
377 -
385 stopPhaseTimer();
386
387 + // Simulate an async bailout/interruption by invoking lifecycle twice.
388 + if (debugRenderPhaseSideEffects) {
389 + instance.componentWillMount();
390 + }
391 +
392 if (oldState !== instance.state) {
393 if (__DEV__) {
394 warning(
@@ -402,6 +414,11 @@ export default function(
414 instance.componentWillReceiveProps(newProps, newContext);
415 stopPhaseTimer();
416
417 + // Simulate an async bailout/interruption by invoking lifecycle twice.
418 + if (debugRenderPhaseSideEffects) {
419 + instance.componentWillReceiveProps(newProps, newContext);
420 + }
421 +
422 if (instance.state !== oldState) {
423 if (__DEV__) {
424 const componentName = getComponentName(workInProgress) || 'Component';
@@ -677,6 +694,11 @@ export default function(
694 startPhaseTimer(workInProgress, 'componentWillUpdate');
695 instance.componentWillUpdate(newProps, newState, newContext);
696 stopPhaseTimer();
697 +
698 + // Simulate an async bailout/interruption by invoking lifecycle twice.
699 + if (debugRenderPhaseSideEffects) {
700 + instance.componentWillUpdate(newProps, newState, newContext);
701 + }
702 }
703 if (typeof instance.componentDidUpdate === 'function') {
704 workInProgress.effectTag |= Update;
packages/react-reconciler/src/ReactFiberScheduler.js
+1
@@ -620,6 +620,7 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
620 if (__DEV__) {
621 ReactDebugCurrentFiber.setCurrentFiber(workInProgress);
622 }
623 +
624 let next = beginWork(current, workInProgress, nextRenderExpirationTime);
625 if (__DEV__) {
626 ReactDebugCurrentFiber.resetCurrentFiber();
packages/react-reconciler/src/ReactFiberUpdateQueue.js
+7
@@ -10,6 +10,7 @@
10 import type {Fiber} from './ReactFiber';
11 import type {ExpirationTime} from './ReactFiberExpirationTime';
12
13 +import {debugRenderPhaseSideEffects} from 'shared/ReactFeatureFlags';
14 import {Callback as CallbackEffect} from 'shared/ReactTypeOfSideEffect';
15 import {ClassComponent, HostRoot} from 'shared/ReactTypeOfWork';
16 import invariant from 'fbjs/lib/invariant';
@@ -181,6 +182,12 @@ function getStateFromUpdate(update, instance, prevState, props) {
182 const partialState = update.partialState;
183 if (typeof partialState === 'function') {
184 const updateFn = partialState;
185 +
186 + // Invoke setState callback an extra time to help detect side-effects.
187 + if (debugRenderPhaseSideEffects) {
188 + updateFn.call(instance, prevState, props);
189 + }
190 +
191 return updateFn.call(instance, prevState, props);
192 } else {
193 return partialState;
packages/react/src/__tests__/ReactAsyncClassComponent-test.js new
+123
@@ -0,0 +1,123 @@
1 +/**
2 + * Copyright (c) 2013-present, Facebook, Inc.
3 + *
4 + * This source code is licensed under the MIT license found in the
5 + * LICENSE file in the root directory of this source tree.
6 + *
7 + * @emails react-core
8 + */
9 +
10 +'use strict';
11 +
12 +var React;
13 +var ReactFeatureFlags;
14 +var ReactTestRenderer;
15 +
16 +describe('ReactAsyncClassComponent', () => {
17 + describe('debugRenderPhaseSideEffects', () => {
18 + beforeEach(() => {
19 + jest.resetModules();
20 + ReactFeatureFlags = require('shared/ReactFeatureFlags');
21 + ReactFeatureFlags.debugRenderPhaseSideEffects = true;
22 + React = require('react');
23 + ReactTestRenderer = require('react-test-renderer');
24 + });
25 +
26 + it('should invoke precommit lifecycle methods twice', () => {
27 + let log = [];
28 + let shouldComponentUpdate = false;
29 + class ClassComponent extends React.Component {
30 + state = {};
31 + componentDidMount() {
32 + log.push('componentDidMount');
33 + }
34 + componentDidUpdate() {
35 + log.push('componentDidUpdate');
36 + }
37 + componentWillMount() {
38 + log.push('componentWillMount');
39 + }
40 + componentWillReceiveProps() {
41 + log.push('componentWillReceiveProps');
42 + }
43 + componentWillUnmount() {
44 + log.push('componentWillUnmount');
45 + }
46 + componentWillUpdate() {
47 + log.push('componentWillUpdate');
48 + }
49 + shouldComponentUpdate() {
50 + log.push('shouldComponentUpdate');
51 + return shouldComponentUpdate;
52 + }
53 + render() {
54 + log.push('render');
55 + return null;
56 + }
57 + }
58 +
59 + const component = ReactTestRenderer.create(<ClassComponent />);
60 + expect(log).toEqual([
61 + 'componentWillMount',
62 + 'componentWillMount',
63 + 'render',
64 + 'render',
65 + 'componentDidMount',
66 + ]);
67 +
68 + log = [];
69 + shouldComponentUpdate = true;
70 +
71 + component.update(<ClassComponent />);
72 + expect(log).toEqual([
73 + 'componentWillReceiveProps',
74 + 'componentWillReceiveProps',
75 + 'shouldComponentUpdate',
76 + 'shouldComponentUpdate',
77 + 'componentWillUpdate',
78 + 'componentWillUpdate',
79 + 'render',
80 + 'render',
81 + 'componentDidUpdate',
82 + ]);
83 +
84 + log = [];
85 + shouldComponentUpdate = false;
86 +
87 + component.update(<ClassComponent />);
88 + expect(log).toEqual([
89 + 'componentWillReceiveProps',
90 + 'componentWillReceiveProps',
91 + 'shouldComponentUpdate',
92 + 'shouldComponentUpdate',
93 + ]);
94 + });
95 +
96 + it('should invoke setState callbacks twice', () => {
97 + class ClassComponent extends React.Component {
98 + state = {
99 + count: 1,
100 + };
101 + render() {
102 + return null;
103 + }
104 + }
105 +
106 + let setStateCount = 0;
107 +
108 + const rendered = ReactTestRenderer.create(<ClassComponent />);
109 + const instance = rendered.getInstance();
110 + instance.setState(state => {
111 + setStateCount++;
112 + return {
113 + count: state.count + 1,
114 + };
115 + });
116 +
117 + // Callback should be invoked twice
118 + expect(setStateCount).toBe(2);
119 + // But each time `state` should be the previous value
120 + expect(instance.state.count).toBe(2);
121 + });
122 + });
123 +});
packages/shared/ReactFeatureFlags.js
+3
@@ -24,6 +24,9 @@ export const enableNoopReconciler = false;
24 // Experimental persistent mode (CS):
25 export const enablePersistentReconciler = false;
26
27 +// Helps identify side effects in begin-phase lifecycle hooks and setState reducers:
28 +export const debugRenderPhaseSideEffects = false;
29 +
30 // Only used in www builds.
31 export function addUserTimingListener() {
32 invariant(false, 'Not implemented.');
scripts/rollup/shims/rollup/ReactFeatureFlags-www.js
+1
@@ -16,6 +16,7 @@ export const {
16 } = require('ReactFeatureFlags');
17
18 // The rest of the flags are static for better dead code elimination.
19 +export const debugRenderPhaseSideEffects = false;
20 export const enableAsyncSubtreeAPI = true;
21 export const enableReactFragment = false;
22 export const enableCreateRoot = true;