diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 25cd68b0ea175..cc51116413d6b 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -2893,7 +2893,12 @@ function beginWork( } case Profiler: if (enableProfilerTimer) { - workInProgress.effectTag |= Update; + // Profiler should only call onRender when one of its descendants actually rendered. + const hasChildWork = + workInProgress.childExpirationTime >= renderExpirationTime; + if (hasChildWork) { + workInProgress.effectTag |= Update; + } } break; case SuspenseComponent: { diff --git a/packages/react/src/__tests__/ReactProfiler-test.internal.js b/packages/react/src/__tests__/ReactProfiler-test.internal.js index db64f1f86e5eb..150573b4950d0 100644 --- a/packages/react/src/__tests__/ReactProfiler-test.internal.js +++ b/packages/react/src/__tests__/ReactProfiler-test.internal.js @@ -295,6 +295,79 @@ describe('Profiler', () => { ); }); + it('does not report work done on a sibling', () => { + const callback = jest.fn(); + + const DoesNotUpdate = React.memo(function DoesNotUpdateInner() { + Scheduler.unstable_advanceTime(10); + return null; + }, () => true); + + let updateProfilerSibling; + + function ProfilerSibling() { + const [count, setCount] = React.useState(0); + updateProfilerSibling = () => setCount(count + 1); + return null; + } + + function App() { + return ( + + + + + + + ); + } + + const renderer = ReactTestRenderer.create(); + + expect(callback).toHaveBeenCalledTimes(1); + + let call = callback.mock.calls[0]; + + expect(call).toHaveLength(enableSchedulerTracing ? 7 : 6); + expect(call[0]).toBe('test'); + expect(call[1]).toBe('mount'); + expect(call[2]).toBe(10); // actual time + expect(call[3]).toBe(10); // base time + expect(call[4]).toBe(0); // start time + expect(call[5]).toBe(10); // commit time + expect(call[6]).toEqual(enableSchedulerTracing ? new Set() : undefined); // interaction events + + callback.mockReset(); + + Scheduler.unstable_advanceTime(20); // 10 -> 30 + + // Updating a parent should report a re-render, + // since React technically did a little bit of work between the Profiler and the bailed out subtree. + renderer.update(); + + expect(callback).toHaveBeenCalledTimes(1); + + call = callback.mock.calls[0]; + + expect(call).toHaveLength(enableSchedulerTracing ? 7 : 6); + expect(call[0]).toBe('test'); + expect(call[1]).toBe('update'); + expect(call[2]).toBe(0); // actual time + expect(call[3]).toBe(10); // base time + expect(call[4]).toBe(30); // start time + expect(call[5]).toBe(30); // commit time + expect(call[6]).toEqual(enableSchedulerTracing ? new Set() : undefined); // interaction events + + callback.mockReset(); + + Scheduler.unstable_advanceTime(20); // 30 -> 50 + + // Updating a sibling should not report a re-render. + ReactTestRenderer.act(updateProfilerSibling); + + expect(callback).not.toHaveBeenCalled(); + }); + it('logs render times for both mount and update', () => { const callback = jest.fn(); @@ -372,7 +445,7 @@ describe('Profiler', () => { Scheduler.unstable_advanceTime(5); // 0 -> 5 ReactTestRenderer.create( - <> + @@ -380,7 +453,7 @@ describe('Profiler', () => { - , + , ); expect(callback).toHaveBeenCalledTimes(2); @@ -407,14 +480,14 @@ describe('Profiler', () => { Scheduler.unstable_advanceTime(5); // 0 -> 5 ReactTestRenderer.create( - <> + - , + , ); expect(callback).toHaveBeenCalledTimes(2); @@ -440,13 +513,13 @@ describe('Profiler', () => { Scheduler.unstable_advanceTime(5); // 0 -> 5 ReactTestRenderer.create( - <> + - , + , ); expect(callback).toHaveBeenCalledTimes(1); @@ -471,28 +544,18 @@ describe('Profiler', () => { } } - class Pure extends React.PureComponent { - render() { - return this.props.children; - } - } - const renderer = ReactTestRenderer.create( - - - -
- - + +
, ); // All profile callbacks are called for initial render - expect(callback).toHaveBeenCalledTimes(3); + expect(callback).toHaveBeenCalledTimes(2); callback.mockReset(); @@ -502,11 +565,11 @@ describe('Profiler', () => { }); }); - // Only call profile updates for paths that have re-rendered - // Since "inner" is beneath a pure component, it isn't called - expect(callback).toHaveBeenCalledTimes(2); - expect(callback.mock.calls[0][0]).toBe('middle'); - expect(callback.mock.calls[1][0]).toBe('outer'); + // Only call onRender for paths that have re-rendered. + // Since the Updater's props didn't change, + // React does not re-render its children. + expect(callback).toHaveBeenCalledTimes(1); + expect(callback.mock.calls[0][0]).toBe('outer'); }); it('decreases actual time but not base time when sCU prevents an update', () => { @@ -1455,11 +1518,11 @@ describe('Profiler', () => { render() { instance = this; return ( - <> + {this.state.count} - + ); } }