Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion packages/react-reconciler/src/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
115 changes: 89 additions & 26 deletions packages/react/src/__tests__/ReactProfiler-test.internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<React.Fragment>
<React.Profiler id="test" onRender={callback}>
<DoesNotUpdate />
</React.Profiler>
<ProfilerSibling />
</React.Fragment>
);
}

const renderer = ReactTestRenderer.create(<App />);

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(<App />);

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();

Expand Down Expand Up @@ -372,15 +445,15 @@ describe('Profiler', () => {
Scheduler.unstable_advanceTime(5); // 0 -> 5

ReactTestRenderer.create(
<>
<React.Fragment>
<React.Profiler id="parent" onRender={callback}>
<AdvanceTime byAmount={10}>
<React.Profiler id="child" onRender={callback}>
<AdvanceTime byAmount={20} />
</React.Profiler>
</AdvanceTime>
</React.Profiler>
</>,
</React.Fragment>,
);

expect(callback).toHaveBeenCalledTimes(2);
Expand All @@ -407,14 +480,14 @@ describe('Profiler', () => {
Scheduler.unstable_advanceTime(5); // 0 -> 5

ReactTestRenderer.create(
<>
<React.Fragment>
<React.Profiler id="first" onRender={callback}>
<AdvanceTime byAmount={20} />
</React.Profiler>
<React.Profiler id="second" onRender={callback}>
<AdvanceTime byAmount={5} />
</React.Profiler>
</>,
</React.Fragment>,
);

expect(callback).toHaveBeenCalledTimes(2);
Expand All @@ -440,13 +513,13 @@ describe('Profiler', () => {
Scheduler.unstable_advanceTime(5); // 0 -> 5

ReactTestRenderer.create(
<>
<React.Fragment>
<AdvanceTime byAmount={20} />
<React.Profiler id="test" onRender={callback}>
<AdvanceTime byAmount={5} />
</React.Profiler>
<AdvanceTime byAmount={20} />
</>,
</React.Fragment>,
);

expect(callback).toHaveBeenCalledTimes(1);
Expand All @@ -471,28 +544,18 @@ describe('Profiler', () => {
}
}

class Pure extends React.PureComponent {
render() {
return this.props.children;
}
}

const renderer = ReactTestRenderer.create(
<React.Profiler id="outer" onRender={callback}>
<Updater>
<React.Profiler id="middle" onRender={callback}>
<Pure>
<React.Profiler id="inner" onRender={callback}>
<div />
</React.Profiler>
</Pure>
<React.Profiler id="inner" onRender={callback}>
<div />
</React.Profiler>
</Updater>
</React.Profiler>,
);

// All profile callbacks are called for initial render
expect(callback).toHaveBeenCalledTimes(3);
expect(callback).toHaveBeenCalledTimes(2);

callback.mockReset();

Expand All @@ -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', () => {
Expand Down Expand Up @@ -1455,11 +1518,11 @@ describe('Profiler', () => {
render() {
instance = this;
return (
<>
<React.Fragment>
<Yield value="first" />
{this.state.count}
<Yield value="last" />
</>
</React.Fragment>
);
}
}
Expand Down