Fix componentWillUnmount() not counted by ReactPerf (#6858)
* Fix componentWillUnmount() not counted by ReactPerf * Test that functional component render() time shows up in ReactPerf * Test for setState() code path updates being included
Dan Abramov committed
May 24, 2016 at 20:50 UTC
9ba5668d1828148c019aaf40ff7662824611d17c
2 files changed
+81
-2
src/renderers/dom/client/ReactMount.js
+6
@@ -169,7 +169,13 @@ function batchedMountComponentIntoNode(
169
* @see {ReactMount.unmountComponentAtNode}
170
*/
171
function unmountComponentFromNode(instance, container, safely) {
172
+ if (__DEV__) {
173
+ ReactInstrumentation.debugTool.onBeginFlush();
174
+ }
175
ReactReconciler.unmountComponent(instance, safely);
176
+ if (__DEV__) {
177
+ ReactInstrumentation.debugTool.onEndFlush();
178
+ }
179
180
if (container.nodeType === DOC_NODE_TYPE) {
181
container = container.documentElement;
src/renderers/shared/__tests__/ReactPerf-test.js
+75
-2
@@ -16,10 +16,12 @@ describe('ReactPerf', function() {
16
var ReactDOM;
17
var ReactPerf;
18
var ReactTestUtils;
19
+ var emptyFunction;
20
21
var App;
22
var Box;
23
var Div;
24
+ var LifeCycle;
25
26
beforeEach(function() {
27
var now = 0;
@@ -36,6 +38,7 @@ describe('ReactPerf', function() {
38
ReactDOM = require('ReactDOM');
39
ReactPerf = require('ReactPerf');
40
ReactTestUtils = require('ReactTestUtils');
41
+ emptyFunction = require('emptyFunction');
42
43
App = React.createClass({
44
render: function() {
@@ -55,6 +58,17 @@ describe('ReactPerf', function() {
58
return <div {...this.props} />;
59
},
60
});
61
+
62
+ LifeCycle = React.createClass({
63
+ shouldComponentUpdate: emptyFunction.thatReturnsTrue,
64
+ componentWillMount: emptyFunction,
65
+ componentDidMount: emptyFunction,
66
+ componentWillReceiveProps: emptyFunction,
67
+ componentWillUpdate: emptyFunction,
68
+ componentDidUpdate: emptyFunction,
69
+ componentWillUnmount: emptyFunction,
70
+ render: emptyFunction.thatReturnsNull,
71
+ });
72
});
73
74
afterEach(function() {
@@ -215,7 +229,7 @@ describe('ReactPerf', function() {
229
});
230
});
231
218
- it('should not count replacing null with a native as waste', function() {
232
+ it('should not count replacing null with a host as waste', function() {
233
var element = null;
234
function Foo() {
235
return element;
@@ -228,7 +242,7 @@ describe('ReactPerf', function() {
242
});
243
});
244
231
- it('should not count replacing a native with null as waste', function() {
245
+ it('should not count replacing a host with null as waste', function() {
246
var element = <div />;
247
function Foo() {
248
return element;
@@ -256,6 +270,65 @@ describe('ReactPerf', function() {
270
}]);
271
});
272
273
+ it('should include lifecycle methods in measurements', function() {
274
+ var container = document.createElement('div');
275
+ var measurements = measure(() => {
276
+ var instance = ReactDOM.render(<LifeCycle />, container);
277
+ ReactDOM.render(<LifeCycle />, container);
278
+ instance.setState({});
279
+ ReactDOM.unmountComponentAtNode(container);
280
+ });
281
+ expect(ReactPerf.getExclusive(measurements)).toEqual([{
282
+ key: 'LifeCycle',
283
+ instanceCount: 1,
284
+ totalDuration: 14,
285
+ counts: {
286
+ ctor: 1,
287
+ shouldComponentUpdate: 2,
288
+ componentWillMount: 1,
289
+ componentDidMount: 1,
290
+ componentWillReceiveProps: 1,
291
+ componentWillUpdate: 2,
292
+ componentDidUpdate: 2,
293
+ componentWillUnmount: 1,
294
+ render: 3,
295
+ },
296
+ durations: {
297
+ ctor: 1,
298
+ shouldComponentUpdate: 2,
299
+ componentWillMount: 1,
300
+ componentDidMount: 1,
301
+ componentWillReceiveProps: 1,
302
+ componentWillUpdate: 2,
303
+ componentDidUpdate: 2,
304
+ componentWillUnmount: 1,
305
+ render: 3,
306
+ },
307
+ }]);
308
+ });
309
+
310
+ it('should include render time of functional components', function() {
311
+ function Foo() {
312
+ return null;
313
+ }
314
+
315
+ var container = document.createElement('div');
316
+ var measurements = measure(() => {
317
+ ReactDOM.render(<Foo />, container);
318
+ });
319
+ expect(ReactPerf.getExclusive(measurements)).toEqual([{
320
+ key: 'Foo',
321
+ instanceCount: 1,
322
+ totalDuration: 1,
323
+ counts: {
324
+ render: 1,
325
+ },
326
+ durations: {
327
+ render: 1,
328
+ },
329
+ }]);
330
+ });
331
+
332
it('warns once when using getMeasurementsSummaryMap', function() {
333
var measurements = measure(() => {});
334
spyOn(console, 'error');