From 42eff4bc78a3636953c55096ca0b220c611a74cc Mon Sep 17 00:00:00 2001 From: Ricky Date: Wed, 10 Apr 2024 10:33:41 -0400 Subject: [PATCH] [tests] Fix assertions not flushed before act (#28745) Fixes some easy cases blocking https://github.com/facebook/react/pull/28737, I'll follow up with more complex/interesting cases in other PRs. --- .../src/__tests__/ReactCompositeComponent-test.js | 6 ++---- .../src/__tests__/ReactDOMFizzDeferredValue-test.js | 2 ++ .../src/__tests__/ReactDOMNativeEventHeuristic-test.js | 3 ++- packages/react-dom/src/__tests__/ReactDOMRoot-test.js | 3 ++- .../src/__tests__/ReactDOMSuspensePlaceholder-test.js | 6 ++++-- .../src/__tests__/ReactEmptyComponent-test.js | 9 +++------ .../src/__tests__/ReactUpdaters-test.internal.js | 8 ++++++-- .../src/__tests__/useSubscription-test.js | 10 +++------- .../src/__tests__/useSyncExternalStoreShared-test.js | 5 +++++ 9 files changed, 29 insertions(+), 23 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js index e098c93ca7a18..4630f9ddbc163 100644 --- a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js @@ -913,15 +913,13 @@ describe('ReactCompositeComponent', () => { await act(() => { root.render(); }); + + assertLog(['A componentWillMount', 'A render', 'A componentDidMount']); await act(() => { root.render(); }); assertLog([ - 'A componentWillMount', - 'A render', - 'A componentDidMount', - 'B componentWillMount', 'B render', 'A componentWillUnmount', diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzDeferredValue-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzDeferredValue-test.js index 0b3335dfbacb8..fbfb00df87a1d 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzDeferredValue-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzDeferredValue-test.js @@ -111,9 +111,11 @@ describe('ReactDOMFizzForm', () => { await readIntoContainer(stream); expect(container.textContent).toEqual('Loading...'); + assertLog(['Loading...']); // After hydration, it's updated to the final value await act(() => ReactDOMClient.hydrateRoot(container, )); expect(container.textContent).toEqual('Final'); + assertLog(['Loading...', 'Final']); }, ); diff --git a/packages/react-dom/src/__tests__/ReactDOMNativeEventHeuristic-test.js b/packages/react-dom/src/__tests__/ReactDOMNativeEventHeuristic-test.js index 6b445f9cf8d61..13b754ecf050f 100644 --- a/packages/react-dom/src/__tests__/ReactDOMNativeEventHeuristic-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMNativeEventHeuristic-test.js @@ -297,6 +297,7 @@ describe('ReactDOMNativeEventHeuristic-test', () => { }); expect(container.textContent).toEqual('not hovered'); + assertLog(['not hovered']); await act(async () => { // Note: React does not use native mouseenter/mouseleave events // but we should still correctly determine their priority. @@ -308,7 +309,7 @@ describe('ReactDOMNativeEventHeuristic-test', () => { dispatchAndSetCurrentEvent(target.current, mouseEnterEvent); // Since mouse end is not discrete, should not have updated yet - assertLog(['not hovered']); + assertLog([]); expect(container.textContent).toEqual('not hovered'); await waitFor(['hovered']); diff --git a/packages/react-dom/src/__tests__/ReactDOMRoot-test.js b/packages/react-dom/src/__tests__/ReactDOMRoot-test.js index 45f6fc095c2d2..f36f45a1e28fd 100644 --- a/packages/react-dom/src/__tests__/ReactDOMRoot-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMRoot-test.js @@ -342,12 +342,13 @@ describe('ReactDOMRoot', () => { root.render(); }); + assertLog(['a']); expect(container.textContent).toEqual('a'); await act(async () => { root.render(); - assertLog(['a']); + assertLog([]); expect(container.textContent).toEqual('a'); await waitFor(['b']); diff --git a/packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js b/packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js index 69789ad482349..61047322f9d34 100644 --- a/packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js @@ -17,6 +17,7 @@ let Suspense; let Scheduler; let act; let textCache; +let assertLog; describe('ReactDOMSuspensePlaceholder', () => { let container; @@ -31,6 +32,7 @@ describe('ReactDOMSuspensePlaceholder', () => { ReactDOMClient = require('react-dom/client'); Scheduler = require('scheduler'); act = require('internal-test-utils').act; + assertLog = require('internal-test-utils').assertLog; Suspense = React.Suspense; container = document.createElement('div'); document.body.appendChild(container); @@ -157,11 +159,11 @@ describe('ReactDOMSuspensePlaceholder', () => { }); expect(container.textContent).toEqual('Loading...'); - + assertLog(['A', 'Suspend! [B]', 'Loading...']); await act(() => { resolveText('B'); }); - + assertLog(['A', 'B', 'C']); expect(container.textContent).toEqual('ABC'); }); diff --git a/packages/react-dom/src/__tests__/ReactEmptyComponent-test.js b/packages/react-dom/src/__tests__/ReactEmptyComponent-test.js index 0cb47904b6f56..de22540c5432f 100644 --- a/packages/react-dom/src/__tests__/ReactEmptyComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactEmptyComponent-test.js @@ -115,17 +115,14 @@ describe('ReactEmptyComponent', () => { root1.render(instance1); }); + assertLog(['mount undefined', 'update DIV']); + const root2 = ReactDOMClient.createRoot(container2); await act(() => { root2.render(instance2); }); - assertLog([ - 'mount undefined', - 'update DIV', - 'mount DIV', - 'update undefined', - ]); + assertLog(['mount DIV', 'update undefined']); }); it('should be able to switch in a list of children', async () => { diff --git a/packages/react-reconciler/src/__tests__/ReactUpdaters-test.internal.js b/packages/react-reconciler/src/__tests__/ReactUpdaters-test.internal.js index a14e29daaf332..3f7fcd28aec3d 100644 --- a/packages/react-reconciler/src/__tests__/ReactUpdaters-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactUpdaters-test.internal.js @@ -112,11 +112,13 @@ describe('updaters', () => { root.render(); }); expect(allSchedulerTags).toEqual([[HostRoot]]); + assertLog(['onCommitRoot']); await act(() => { root.render(); }); expect(allSchedulerTags).toEqual([[HostRoot], [HostRoot]]); + assertLog(['onCommitRoot']); }); it('should report a function component as the scheduler for a hooks update', async () => { @@ -148,12 +150,13 @@ describe('updaters', () => { expect(scheduleForA).not.toBeNull(); expect(scheduleForB).not.toBeNull(); expect(allSchedulerTypes).toEqual([[null]]); + assertLog(['onCommitRoot']); await act(() => { scheduleForA(); }); expect(allSchedulerTypes).toEqual([[null], [SchedulingComponentA]]); - + assertLog(['onCommitRoot']); await act(() => { scheduleForB(); }); @@ -162,6 +165,7 @@ describe('updaters', () => { [SchedulingComponentA], [SchedulingComponentB], ]); + assertLog(['onCommitRoot']); }); it('should report a class component as the scheduler for a setState update', async () => { @@ -180,7 +184,7 @@ describe('updaters', () => { root.render(); }); expect(allSchedulerTypes).toEqual([[null]]); - + assertLog(['onCommitRoot']); expect(instance).not.toBeNull(); await act(() => { instance.setState({}); diff --git a/packages/use-subscription/src/__tests__/useSubscription-test.js b/packages/use-subscription/src/__tests__/useSubscription-test.js index aff10a7967a33..d54d806354b68 100644 --- a/packages/use-subscription/src/__tests__/useSubscription-test.js +++ b/packages/use-subscription/src/__tests__/useSubscription-test.js @@ -338,6 +338,8 @@ describe('useSubscription', () => { observableB.next('b-3'); }); + assertLog(['Grandchild: b-0', 'Child: b-3', 'Grandchild: b-3']); + // Update again await act(() => root.render()); @@ -345,13 +347,7 @@ describe('useSubscription', () => { // We expect the last emitted update to be rendered (because of the commit phase value check) // But the intermediate ones should be ignored, // And the final rendered output should be the higher-priority observable. - assertLog([ - 'Grandchild: b-0', - 'Child: b-3', - 'Grandchild: b-3', - 'Child: a-0', - 'Grandchild: a-0', - ]); + assertLog(['Child: a-0', 'Grandchild: a-0']); expect(log).toEqual([ 'Parent.componentDidMount', 'Parent.componentDidUpdate', diff --git a/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js b/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js index b8f66d15c8907..e01ef57c10a64 100644 --- a/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js +++ b/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js @@ -660,14 +660,17 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { // Object.is algorithm to compare values. await act(() => root.render()); expect(container.textContent).toEqual('NaN'); + assertLog([NaN]); // Update to real number await act(() => store.set(123)); expect(container.textContent).toEqual('123'); + assertLog([123]); // Update back to NaN await act(() => store.set('not a number')); expect(container.textContent).toEqual('NaN'); + assertLog([NaN]); }); describe('extra features implemented in user-space', () => { @@ -968,6 +971,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { ), ); + assertLog(['A']); expect(container.textContent).toEqual('A'); if (__DEV__ && gate(flags => flags.enableUseSyncExternalStoreShim)) { @@ -1017,6 +1021,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { ), ); + assertLog(['A']); expect(container.textContent).toEqual('A'); if (__DEV__ && gate(flags => flags.enableUseSyncExternalStoreShim)) {