From 51c6fdea64bee8d1edf4590ebd288df830a68609 Mon Sep 17 00:00:00 2001 From: Brian Vaughn <bvaughn@fb.com> Date: Mon, 4 Nov 2019 11:08:11 -0800 Subject: [PATCH 1/3] Bailout without entering work loop for roots without work This addresses an edge case where React currently does a no-op render and commits. This commit is unnecessary, and can confuse the DevTools Profiler. --- .../src/ReactFiberWorkLoop.js | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 85d678dec501f..c364606269034 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -661,6 +661,24 @@ function performConcurrentWorkOnRoot(root, didTimeout) { root !== workInProgressRoot || expirationTime !== renderExpirationTime ) { + // Before starting a fresh render, check if we can bailout on the entire tree. + // For non-root fibers, this bailout usually happens in the begin phase + // of the parent component, but roots are special because they don't + // have parents. + const remainingExpirationTimeOnRoot = + root.current.expirationTime > root.current.childExpirationTime + ? root.current.expirationTime + : root.current.childExpirationTime; + if (remainingExpirationTimeOnRoot < renderExpirationTime) { + // We can bailout without entering the work loop. + markRootFinishedAtTime( + root, + expirationTime, + remainingExpirationTimeOnRoot, + ); + return null; + } + prepareFreshStack(root, expirationTime); startWorkOnPendingInteractions(root, expirationTime); } @@ -997,6 +1015,24 @@ function performSyncWorkOnRoot(root) { root !== workInProgressRoot || expirationTime !== renderExpirationTime ) { + // Before starting a fresh render, check if we can bailout on the entire tree. + // For non-root fibers, this bailout usually happens in the begin phase + // of the parent component, but roots are special because they don't + // have parents. + const remainingExpirationTimeOnRoot = + root.current.expirationTime > root.current.childExpirationTime + ? root.current.expirationTime + : root.current.childExpirationTime; + if (remainingExpirationTimeOnRoot < renderExpirationTime) { + // We can bailout without entering the work loop. + markRootFinishedAtTime( + root, + expirationTime, + remainingExpirationTimeOnRoot, + ); + return null; + } + prepareFreshStack(root, expirationTime); startWorkOnPendingInteractions(root, expirationTime); } From 5fba3c2b19f59d0ca55d51e1aa1c85b1fc40be83 Mon Sep 17 00:00:00 2001 From: Brian Vaughn <bvaughn@fb.com> Date: Mon, 4 Nov 2019 11:25:40 -0800 Subject: [PATCH 2/3] Compare next exp time (rather than previous) when bailing out on a root --- packages/react-reconciler/src/ReactFiberWorkLoop.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index c364606269034..dc9bdc1584c5f 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -669,7 +669,7 @@ function performConcurrentWorkOnRoot(root, didTimeout) { root.current.expirationTime > root.current.childExpirationTime ? root.current.expirationTime : root.current.childExpirationTime; - if (remainingExpirationTimeOnRoot < renderExpirationTime) { + if (remainingExpirationTimeOnRoot < expirationTime) { // We can bailout without entering the work loop. markRootFinishedAtTime( root, @@ -1023,7 +1023,7 @@ function performSyncWorkOnRoot(root) { root.current.expirationTime > root.current.childExpirationTime ? root.current.expirationTime : root.current.childExpirationTime; - if (remainingExpirationTimeOnRoot < renderExpirationTime) { + if (remainingExpirationTimeOnRoot < expirationTime) { // We can bailout without entering the work loop. markRootFinishedAtTime( root, From a14fdcf42407814c91f05a67bc8b4c046fd2dbb5 Mon Sep 17 00:00:00 2001 From: Brian Vaughn <bvaughn@fb.com> Date: Mon, 4 Nov 2019 13:32:53 -0800 Subject: [PATCH 3/3] Wrap refs-test updates in act() --- packages/react-dom/src/__tests__/refs-test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/react-dom/src/__tests__/refs-test.js b/packages/react-dom/src/__tests__/refs-test.js index 7cc764bdefdac..aaa1b0358a122 100644 --- a/packages/react-dom/src/__tests__/refs-test.js +++ b/packages/react-dom/src/__tests__/refs-test.js @@ -140,18 +140,18 @@ describe('reactiverefs', () => { expectClickLogsLengthToBe(testRefsComponent, 1); // After clicking the reset, there should still only be one click log ref. - testRefsComponent.refs.resetDiv.click(); + ReactTestUtils.act(() => testRefsComponent.refs.resetDiv.click()); expectClickLogsLengthToBe(testRefsComponent, 1); // Begin incrementing clicks (and therefore refs). - clickIncrementer.click(); + ReactTestUtils.act(() => clickIncrementer.click()); expectClickLogsLengthToBe(testRefsComponent, 2); - clickIncrementer.click(); + ReactTestUtils.act(() => clickIncrementer.click()); expectClickLogsLengthToBe(testRefsComponent, 3); // Now reset again - testRefsComponent.refs.resetDiv.click(); + ReactTestUtils.act(() => testRefsComponent.refs.resetDiv.click()); expectClickLogsLengthToBe(testRefsComponent, 1); }); });