From b22813679b72aa40dd51cf9f408d4ac8449c99d7 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sat, 14 Dec 2024 16:45:18 -0500 Subject: [PATCH] Update test split into two tasks The ReactDOMSelect has an overlapping act error but I don't know why that wasn't erroring before. I also don't get why waitFor isn't enough for useSubscription. Nor why ReactExpiration changes. What does the additionalLogsAfterAttemptingToYield helper do? --- .../src/__tests__/ReactDOMSelect-test.js | 4 +-- ...DOMServerPartialHydration-test.internal.js | 4 +++ .../src/__tests__/ReactDeferredValue-test.js | 4 +++ .../src/__tests__/ReactExpiration-test.js | 12 ++++++-- .../ReactHooksWithNoopRenderer-test.js | 24 +++++++++++----- .../ReactSiblingPrerendering-test.js | 16 +++++++++++ .../ReactSuspenseWithNoopRenderer-test.js | 4 +++ .../src/__tests__/ReactUpdatePriority-test.js | 28 +++++++++++++------ .../src/__tests__/useSubscription-test.js | 4 ++- 9 files changed, 78 insertions(+), 22 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMSelect-test.js b/packages/react-dom/src/__tests__/ReactDOMSelect-test.js index 72ebfd1bbe67b..e450cf9a8166e 100644 --- a/packages/react-dom/src/__tests__/ReactDOMSelect-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMSelect-test.js @@ -911,9 +911,7 @@ describe('ReactDOMSelect', () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); async function changeView() { - await act(() => { - root.unmount(); - }); + root.unmount(); } const stub = ( diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js index e9354b0bf259d..9615c34256c3c 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js @@ -3743,6 +3743,10 @@ describe('ReactDOMServerPartialHydration', () => { await waitForPaint(['App']); expect(visibleRef.current).toBe(visibleSpan); + if (gate(flags => flags.enableYieldingBeforePassive)) { + // Passive effects. + await waitForPaint([]); + } // Subsequently, the hidden child is prerendered on the client await waitForPaint(['HiddenChild']); expect(container).toMatchInlineSnapshot(` diff --git a/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js b/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js index 8ca142cb50517..bbd01cd551e86 100644 --- a/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js +++ b/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js @@ -753,6 +753,10 @@ describe('ReactDeferredValue', () => { revealContent(); // Because the preview state was already prerendered, we can reveal it // without any addditional work. + if (gate(flags => flags.enableYieldingBeforePassive)) { + // Passive effects. + await waitForPaint([]); + } await waitForPaint([]); expect(root).toMatchRenderedOutput(
Preview [B]
); }); diff --git a/packages/react-reconciler/src/__tests__/ReactExpiration-test.js b/packages/react-reconciler/src/__tests__/ReactExpiration-test.js index aef06288667db..5b09d7d0b8821 100644 --- a/packages/react-reconciler/src/__tests__/ReactExpiration-test.js +++ b/packages/react-reconciler/src/__tests__/ReactExpiration-test.js @@ -755,10 +755,16 @@ describe('ReactExpiration', () => { // The update finishes without yielding. But it does not flush the effect. await waitFor(['B1'], { - additionalLogsAfterAttemptingToYield: ['C1'], + additionalLogsAfterAttemptingToYield: gate( + flags => flags.enableYieldingBeforePassive, + ) + ? ['C1', 'Effect: 1'] + : ['C1'], }); }); - // The effect flushes after paint. - assertLog(['Effect: 1']); + if (!gate(flags => flags.enableYieldingBeforePassive)) { + // The effect flushes after paint. + assertLog(['Effect: 1']); + } }); }); diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index a43b6c124df7d..dcb362669928e 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -2695,13 +2695,23 @@ describe('ReactHooksWithNoopRenderer', () => { React.startTransition(() => { ReactNoop.render(); }); - await waitForPaint([ - 'Create passive [current: 0]', - 'Destroy insertion [current: 0]', - 'Create insertion [current: 0]', - 'Destroy layout [current: 1]', - 'Create layout [current: 1]', - ]); + if (gate(flags => flags.enableYieldingBeforePassive)) { + await waitForPaint(['Create passive [current: 0]']); + await waitForPaint([ + 'Destroy insertion [current: 0]', + 'Create insertion [current: 0]', + 'Destroy layout [current: 1]', + 'Create layout [current: 1]', + ]); + } else { + await waitForPaint([ + 'Create passive [current: 0]', + 'Destroy insertion [current: 0]', + 'Create insertion [current: 0]', + 'Destroy layout [current: 1]', + 'Create layout [current: 1]', + ]); + } expect(committedText).toEqual('1'); }); assertLog([ diff --git a/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js b/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js index 235e8df2201de..ceb32160976e0 100644 --- a/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js @@ -200,6 +200,10 @@ describe('ReactSiblingPrerendering', () => { await waitForPaint(['A']); expect(root).toMatchRenderedOutput('A'); + if (gate(flags => flags.enableYieldingBeforePassive)) { + // Passive effects. + await waitForPaint([]); + } // The second render is a prerender of the hidden content. await waitForPaint([ 'Suspend! [B]', @@ -237,6 +241,10 @@ describe('ReactSiblingPrerendering', () => { // Immediately after the fallback commits, retry the boundary again. This // time we include B, since we're not blocking the fallback from showing. if (gate('enableSiblingPrerendering')) { + if (gate(flags => flags.enableYieldingBeforePassive)) { + // Passive effects. + await waitForPaint([]); + } await waitForPaint(['Suspend! [A]', 'Suspend! [B]']); } }); @@ -452,6 +460,10 @@ describe('ReactSiblingPrerendering', () => { , ); + if (gate(flags => flags.enableYieldingBeforePassive)) { + // Passive effects. + await waitForPaint([]); + } // Immediately after the fallback commits, retry the boundary again. // Because the promise for A resolved, this is a normal render, _not_ // a prerender. So when we proceed to B, and B suspends, we unwind again @@ -471,6 +483,10 @@ describe('ReactSiblingPrerendering', () => { , ); + if (gate(flags => flags.enableYieldingBeforePassive)) { + // Passive effects. + await waitForPaint([]); + } // Now we can proceed to prerendering C. if (gate('enableSiblingPrerendering')) { await waitForPaint(['Suspend! [B]', 'Suspend! [C]']); diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js index 39cfebec3487e..0dcf0eb4705ee 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js @@ -1901,6 +1901,10 @@ describe('ReactSuspenseWithNoopRenderer', () => { // be throttled because the fallback would have appeared too recently. Scheduler.unstable_advanceTime(10000); jest.advanceTimersByTime(10000); + if (gate(flags => flags.enableYieldingBeforePassive)) { + // Passive effects. + await waitForPaint([]); + } await waitForPaint(['A']); expect(ReactNoop).toMatchRenderedOutput( <> diff --git a/packages/react-reconciler/src/__tests__/ReactUpdatePriority-test.js b/packages/react-reconciler/src/__tests__/ReactUpdatePriority-test.js index aa146558917c1..b2eebfa51611e 100644 --- a/packages/react-reconciler/src/__tests__/ReactUpdatePriority-test.js +++ b/packages/react-reconciler/src/__tests__/ReactUpdatePriority-test.js @@ -81,14 +81,26 @@ describe('ReactUpdatePriority', () => { // Schedule another update at default priority setDefaultState(2); - // The default update flushes first, because - await waitForPaint([ - // Idle update is scheduled - 'Idle update', - - // The default update flushes first, without including the idle update - 'Idle: 1, Default: 2', - ]); + if (gate(flags => flags.enableYieldingBeforePassive)) { + // The default update flushes first, because + await waitForPaint([ + // Idle update is scheduled + 'Idle update', + ]); + await waitForPaint([ + // The default update flushes first, without including the idle update + 'Idle: 1, Default: 2', + ]); + } else { + // The default update flushes first, because + await waitForPaint([ + // Idle update is scheduled + 'Idle update', + + // The default update flushes first, without including the idle update + 'Idle: 1, Default: 2', + ]); + } }); // Now the idle update has flushed assertLog(['Idle: 2, Default: 2']); diff --git a/packages/use-subscription/src/__tests__/useSubscription-test.js b/packages/use-subscription/src/__tests__/useSubscription-test.js index 4ecc405789f91..6642ab35afb1f 100644 --- a/packages/use-subscription/src/__tests__/useSubscription-test.js +++ b/packages/use-subscription/src/__tests__/useSubscription-test.js @@ -19,6 +19,7 @@ let ReplaySubject; let assertLog; let waitForAll; let waitFor; +let waitForPaint; describe('useSubscription', () => { beforeEach(() => { @@ -37,6 +38,7 @@ describe('useSubscription', () => { const InternalTestUtils = require('internal-test-utils'); waitForAll = InternalTestUtils.waitForAll; + waitForPaint = InternalTestUtils.waitForPaint; assertLog = InternalTestUtils.assertLog; waitFor = InternalTestUtils.waitFor; }); @@ -595,7 +597,7 @@ describe('useSubscription', () => { React.startTransition(() => { mutate('C'); }); - await waitFor(['render:first:C', 'render:second:C']); + await waitForPaint(['render:first:C', 'render:second:C']); React.startTransition(() => { mutate('D'); });