From 9ba5668d1828148c019aaf40ff7662824611d17c Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 24 May 2016 20:50:08 +0100 Subject: [PATCH] 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 --- src/renderers/dom/client/ReactMount.js | 6 ++ .../shared/__tests__/ReactPerf-test.js | 77 ++++++++++++++++++- 2 files changed, 81 insertions(+), 2 deletions(-) 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..083b5f64f344c 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() { @@ -215,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; @@ -228,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; @@ -256,6 +270,65 @@ 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); + instance.setState({}); + ReactDOM.unmountComponentAtNode(container); + }); + expect(ReactPerf.getExclusive(measurements)).toEqual([{ + key: 'LifeCycle', + instanceCount: 1, + totalDuration: 14, + counts: { + ctor: 1, + shouldComponentUpdate: 2, + componentWillMount: 1, + componentDidMount: 1, + componentWillReceiveProps: 1, + componentWillUpdate: 2, + componentDidUpdate: 2, + componentWillUnmount: 1, + render: 3, + }, + durations: { + ctor: 1, + shouldComponentUpdate: 2, + componentWillMount: 1, + componentDidMount: 1, + componentWillReceiveProps: 1, + componentWillUpdate: 2, + componentDidUpdate: 2, + componentWillUnmount: 1, + render: 3, + }, + }]); + }); + + 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');