From 9dcfd28715f3e732e42b37e8d8baf3bdc52de175 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 24 May 2016 19:35:13 +0100 Subject: [PATCH 1/3] Fix componentWillUnmount() not counted by ReactPerf --- src/renderers/dom/client/ReactMount.js | 6 +++ .../shared/__tests__/ReactPerf-test.js | 50 +++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/src/renderers/dom/client/ReactMount.js b/src/renderers/dom/client/ReactMount.js index 4f45156704e0c..5e1827b746519 100644 --- a/src/renderers/dom/client/ReactMount.js +++ b/src/renderers/dom/client/ReactMount.js @@ -169,7 +169,13 @@ function batchedMountComponentIntoNode( * @see {ReactMount.unmountComponentAtNode} */ function unmountComponentFromNode(instance, container, safely) { + if (__DEV__) { + ReactInstrumentation.debugTool.onBeginFlush(); + } ReactReconciler.unmountComponent(instance, safely); + if (__DEV__) { + ReactInstrumentation.debugTool.onEndFlush(); + } if (container.nodeType === DOC_NODE_TYPE) { container = container.documentElement; diff --git a/src/renderers/shared/__tests__/ReactPerf-test.js b/src/renderers/shared/__tests__/ReactPerf-test.js index 0a24bb9919d7f..0e45418f39cd4 100644 --- a/src/renderers/shared/__tests__/ReactPerf-test.js +++ b/src/renderers/shared/__tests__/ReactPerf-test.js @@ -16,10 +16,12 @@ describe('ReactPerf', function() { var ReactDOM; var ReactPerf; var ReactTestUtils; + var emptyFunction; var App; var Box; var Div; + var LifeCycle; beforeEach(function() { var now = 0; @@ -36,6 +38,7 @@ describe('ReactPerf', function() { ReactDOM = require('ReactDOM'); ReactPerf = require('ReactPerf'); ReactTestUtils = require('ReactTestUtils'); + emptyFunction = require('emptyFunction'); App = React.createClass({ render: function() { @@ -55,6 +58,17 @@ describe('ReactPerf', function() { return
; }, }); + + LifeCycle = React.createClass({ + shouldComponentUpdate: emptyFunction.thatReturnsTrue, + componentWillMount: emptyFunction, + componentDidMount: emptyFunction, + componentWillReceiveProps: emptyFunction, + componentWillUpdate: emptyFunction, + componentDidUpdate: emptyFunction, + componentWillUnmount: emptyFunction, + render: emptyFunction.thatReturnsNull, + }); }); afterEach(function() { @@ -256,6 +270,42 @@ describe('ReactPerf', function() { }]); }); + it('should include lifecycle methods in measurements', function() { + var container = document.createElement('div'); + var measurements = measure(() => { + ReactDOM.render(, container); + ReactDOM.render(, container); + ReactDOM.unmountComponentAtNode(container); + }); + expect(ReactPerf.getExclusive(measurements)).toEqual([{ + key: 'LifeCycle', + instanceCount: 1, + totalDuration: 10, + counts: { + ctor: 1, + shouldComponentUpdate: 1, + componentWillMount: 1, + componentDidMount: 1, + componentWillReceiveProps: 1, + componentWillUpdate: 1, + componentDidUpdate: 1, + componentWillUnmount: 1, + render: 2, + }, + durations: { + ctor: 1, + shouldComponentUpdate: 1, + componentWillMount: 1, + componentDidMount: 1, + componentWillReceiveProps: 1, + componentWillUpdate: 1, + componentDidUpdate: 1, + componentWillUnmount: 1, + render: 2, + }, + }]); + }); + it('warns once when using getMeasurementsSummaryMap', function() { var measurements = measure(() => {}); spyOn(console, 'error'); From 554a37862ac58865372d6709064c7cc92c98a555 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 24 May 2016 19:44:16 +0100 Subject: [PATCH 2/3] Test that functional component render() time shows up in ReactPerf --- .../shared/__tests__/ReactPerf-test.js | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/renderers/shared/__tests__/ReactPerf-test.js b/src/renderers/shared/__tests__/ReactPerf-test.js index 0e45418f39cd4..074f77b61855a 100644 --- a/src/renderers/shared/__tests__/ReactPerf-test.js +++ b/src/renderers/shared/__tests__/ReactPerf-test.js @@ -306,6 +306,28 @@ describe('ReactPerf', function() { }]); }); + it('should include render time of functional components', function() { + function Foo() { + return null; + } + + var container = document.createElement('div'); + var measurements = measure(() => { + ReactDOM.render(, container); + }); + expect(ReactPerf.getExclusive(measurements)).toEqual([{ + key: 'Foo', + instanceCount: 1, + totalDuration: 1, + counts: { + render: 1, + }, + durations: { + render: 1, + }, + }]); + }); + it('warns once when using getMeasurementsSummaryMap', function() { var measurements = measure(() => {}); spyOn(console, 'error'); From 06a42e265b7a726dc12588df8cb64480aeb7e7ae Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 24 May 2016 20:48:53 +0100 Subject: [PATCH 3/3] Test for setState() code path updates being included --- .../shared/__tests__/ReactPerf-test.js | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/renderers/shared/__tests__/ReactPerf-test.js b/src/renderers/shared/__tests__/ReactPerf-test.js index 074f77b61855a..083b5f64f344c 100644 --- a/src/renderers/shared/__tests__/ReactPerf-test.js +++ b/src/renderers/shared/__tests__/ReactPerf-test.js @@ -229,7 +229,7 @@ describe('ReactPerf', function() { }); }); - it('should not count replacing null with a native as waste', function() { + it('should not count replacing null with a host as waste', function() { var element = null; function Foo() { return element; @@ -242,7 +242,7 @@ describe('ReactPerf', function() { }); }); - it('should not count replacing a native with null as waste', function() { + it('should not count replacing a host with null as waste', function() { var element =
; function Foo() { return element; @@ -273,35 +273,36 @@ describe('ReactPerf', function() { it('should include lifecycle methods in measurements', function() { var container = document.createElement('div'); var measurements = measure(() => { + var instance = ReactDOM.render(, container); ReactDOM.render(, container); - ReactDOM.render(, container); + instance.setState({}); ReactDOM.unmountComponentAtNode(container); }); expect(ReactPerf.getExclusive(measurements)).toEqual([{ key: 'LifeCycle', instanceCount: 1, - totalDuration: 10, + totalDuration: 14, counts: { ctor: 1, - shouldComponentUpdate: 1, + shouldComponentUpdate: 2, componentWillMount: 1, componentDidMount: 1, componentWillReceiveProps: 1, - componentWillUpdate: 1, - componentDidUpdate: 1, + componentWillUpdate: 2, + componentDidUpdate: 2, componentWillUnmount: 1, - render: 2, + render: 3, }, durations: { ctor: 1, - shouldComponentUpdate: 1, + shouldComponentUpdate: 2, componentWillMount: 1, componentDidMount: 1, componentWillReceiveProps: 1, - componentWillUpdate: 1, - componentDidUpdate: 1, + componentWillUpdate: 2, + componentDidUpdate: 2, componentWillUnmount: 1, - render: 2, + render: 3, }, }]); });