From 847f0649c18b95cdd8201236b229b5017536a900 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 17 Sep 2021 16:58:30 -0400 Subject: [PATCH] Convert useSES shim tests to use React DOM Idea is that eventually we'll run these tests against an actual build of React DOM 17 to test backwards compatibility. --- .../useSyncExternalStoreShared-test.js | 151 ++++++++++-------- 1 file changed, 82 insertions(+), 69 deletions(-) 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 93dd24c838231..d457b15758451 100644 --- a/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js +++ b/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js @@ -12,7 +12,7 @@ let useSyncExternalStore; let useSyncExternalStoreExtra; let React; -let ReactNoop; +let ReactDOM; let Scheduler; let act; let useState; @@ -27,11 +27,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { // Remove the built-in API from the React exports to force the package to // use the shim. - // TODO: Don't do this during a variant test run. That way these tests run - // against both the shim and the built-in implementation. - if (gate(flags => flags.variant)) { - // We'll use the variant flag to represent the native implementation - } else { + if (!gate(flags => flags.supportsNativeUseSyncExternalStore)) { // and the non-variant tests for the shim. // // Remove useSyncExternalStore from the React imports so that we use the @@ -55,7 +51,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { } React = require('react'); - ReactNoop = require('react-noop-renderer'); + ReactDOM = require('react-dom'); Scheduler = require('scheduler'); useState = React.useState; useEffect = React.useEffect; @@ -66,7 +62,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { // The internal act implementation doesn't batch updates by default, since // it's mostly used to test concurrent mode. But since these tests run // in both concurrent and legacy mode, I'm adding batching here. - act = cb => internalAct(() => ReactNoop.batchedUpdates(cb)); + act = cb => internalAct(() => ReactDOM.unstable_batchedUpdates(cb)); useSyncExternalStore = require('use-sync-external-store') .useSyncExternalStore; @@ -79,19 +75,24 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { return text; } - function createRoot(element) { + function createRoot(container) { // This wrapper function exists so we can test both legacy roots and // concurrent roots. - if (gate(flags => flags.variant)) { + if (gate(flags => flags.supportsNativeUseSyncExternalStore)) { // The native implementation only exists in 18+, so we test using // concurrent mode. To test the legacy root behavior in the native // implementation (which is supported in the sense that it needs to have // the correct behavior, despite the fact that the legacy root API // triggers a warning in 18), write a test that uses // createLegacyRoot directly. - return ReactNoop.createRoot(); + return ReactDOM.createRoot(container); } else { - return ReactNoop.createLegacyRoot(); + ReactDOM.render(null, container); + return { + render(children) { + ReactDOM.render(children, container); + }, + }; } } @@ -101,7 +102,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { return { set(text) { currentState = text; - ReactNoop.batchedUpdates(() => { + ReactDOM.unstable_batchedUpdates(() => { listeners.forEach(listener => listener()); }); }, @@ -126,17 +127,18 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { return ; } - const root = createRoot(); + const container = document.createElement('div'); + const root = createRoot(container); await act(() => root.render()); expect(Scheduler).toHaveYielded(['Initial']); - expect(root).toMatchRenderedOutput('Initial'); + expect(container.textContent).toEqual('Initial'); await act(() => { store.set('Updated'); }); expect(Scheduler).toHaveYielded(['Updated']); - expect(root).toMatchRenderedOutput('Updated'); + expect(container.textContent).toEqual('Updated'); }); test('skips re-rendering if nothing changes', () => { @@ -147,11 +149,12 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { return ; } - const root = createRoot(); + const container = document.createElement('div'); + const root = createRoot(container); act(() => root.render()); expect(Scheduler).toHaveYielded(['Initial']); - expect(root).toMatchRenderedOutput('Initial'); + expect(container.textContent).toEqual('Initial'); // Update to the same value act(() => { @@ -159,7 +162,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { }); // Should not re-render expect(Scheduler).toHaveYielded([]); - expect(root).toMatchRenderedOutput('Initial'); + expect(container.textContent).toEqual('Initial'); }); test('switch to a different store', async () => { @@ -174,21 +177,22 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { return ; } - const root = createRoot(); + const container = document.createElement('div'); + const root = createRoot(container); await act(() => root.render()); expect(Scheduler).toHaveYielded([0]); - expect(root).toMatchRenderedOutput('0'); + expect(container.textContent).toEqual('0'); await act(() => { storeA.set(1); }); expect(Scheduler).toHaveYielded([1]); - expect(root).toMatchRenderedOutput('1'); + expect(container.textContent).toEqual('1'); // Switch stores and update in the same batch act(() => { - ReactNoop.flushSync(() => { + ReactDOM.flushSync(() => { // This update will be disregarded storeA.set(2); setStore(storeB); @@ -196,7 +200,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { }); // Now reading from B instead of A expect(Scheduler).toHaveYielded([0]); - expect(root).toMatchRenderedOutput('0'); + expect(container.textContent).toEqual('0'); // Update A await act(() => { @@ -204,14 +208,14 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { }); // Nothing happened, because we're no longer subscribed to A expect(Scheduler).toHaveYielded([]); - expect(root).toMatchRenderedOutput('0'); + expect(container.textContent).toEqual('0'); // Update B await act(() => { storeB.set(1); }); expect(Scheduler).toHaveYielded([1]); - expect(root).toMatchRenderedOutput('1'); + expect(container.textContent).toEqual('1'); }); test('selecting a specific value inside getSnapshot', async () => { @@ -235,11 +239,12 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { ); } - const root = createRoot(); + const container = document.createElement('div'); + const root = createRoot(container); act(() => root.render()); expect(Scheduler).toHaveYielded(['A0', 'B0']); - expect(root).toMatchRenderedOutput('A0B0'); + expect(container.textContent).toEqual('A0B0'); // Update b but not a await act(() => { @@ -247,7 +252,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { }); // Only b re-renders expect(Scheduler).toHaveYielded(['B1']); - expect(root).toMatchRenderedOutput('A0B1'); + expect(container.textContent).toEqual('A0B1'); // Update a but not b await act(() => { @@ -255,12 +260,12 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { }); // Only a re-renders expect(Scheduler).toHaveYielded(['A1']); - expect(root).toMatchRenderedOutput('A1B1'); + expect(container.textContent).toEqual('A1B1'); }); // In React 18, you can't observe in between a sync render and its // passive effects, so this is only relevant to legacy roots - // @gate !variant + // @gate !supportsNativeUseSyncExternalStore test( "compares to current state before bailing out, even when there's a " + 'mutation in between the sync and passive effects', @@ -275,7 +280,8 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { return ; } - const root = createRoot(); + const container = document.createElement('div'); + const root = createRoot(container); act(() => root.render()); expect(Scheduler).toHaveYielded([0, 'Passive effect: 0']); @@ -287,7 +293,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { 1, // Passive effect hasn't fired yet ]); - expect(root).toMatchRenderedOutput('1'); + expect(container.textContent).toEqual('1'); // Flip the store state back to the previous value. store.set(0); @@ -300,7 +306,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { 0, ]); // Should flip back to 0 - expect(root).toMatchRenderedOutput('0'); + expect(container.textContent).toEqual('0'); }, ); @@ -344,10 +350,11 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { ); } - const root = createRoot(); + const container = document.createElement('div'); + const root = createRoot(container); act(() => root.render()); expect(Scheduler).toHaveYielded(['A1']); - expect(root).toMatchRenderedOutput('A1'); + expect(container.textContent).toEqual('A1'); act(() => { // Change getSnapshot and update the store in the same batch @@ -360,7 +367,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { // incorrectly bailed out here instead of re-rendering. 'B2', ]); - expect(root).toMatchRenderedOutput('B2'); + expect(container.textContent).toEqual('B2'); }); test('mutating the store in between render and commit when getSnapshot has _not_ changed', () => { @@ -401,10 +408,11 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { ); } - const root = createRoot(); + const container = document.createElement('div'); + const root = createRoot(container); act(() => root.render()); expect(Scheduler).toHaveYielded(['A1']); - expect(root).toMatchRenderedOutput('A1'); + expect(container.textContent).toEqual('A1'); // This will cause a layout effect, and in the layout effect we'll update // the store @@ -418,7 +426,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { 'Update B in commit phase', // No re-render ]); - expect(root).toMatchRenderedOutput('A1'); + expect(container.textContent).toEqual('A1'); }); test("does not bail out if the previous update hasn't finished yet", async () => { @@ -440,7 +448,8 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { return ; } - const root = createRoot(); + const container = document.createElement('div'); + const root = createRoot(container); act(() => root.render( <> @@ -450,13 +459,13 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { ), ); expect(Scheduler).toHaveYielded([0, 0]); - expect(root).toMatchRenderedOutput('00'); + expect(container.textContent).toEqual('00'); await act(() => { store.set(1); }); expect(Scheduler).toHaveYielded([1, 1, 'Reset back to 0', 0, 0]); - expect(root).toMatchRenderedOutput('00'); + expect(container.textContent).toEqual('00'); }); test('uses the latest getSnapshot, even if it changed in the same batch as a store update', () => { @@ -473,20 +482,21 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { return ; } - const root = createRoot(); + const container = document.createElement('div'); + const root = createRoot(container); act(() => root.render()); expect(Scheduler).toHaveYielded([0]); // Update the store and getSnapshot at the same time act(() => { - ReactNoop.flushSync(() => { + ReactDOM.flushSync(() => { setGetSnapshot(() => getSnapshotB); store.set({a: 1, b: 2}); }); }); // It should read from B instead of A expect(Scheduler).toHaveYielded([2]); - expect(root).toMatchRenderedOutput('2'); + expect(container.textContent).toEqual('2'); }); test('handles errors thrown by getSnapshot', async () => { @@ -521,7 +531,8 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { } const errorBoundary = React.createRef(null); - const root = createRoot(); + const container = document.createElement('div'); + const root = createRoot(container); act(() => root.render( @@ -530,13 +541,13 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { ), ); expect(Scheduler).toHaveYielded([0]); - expect(root).toMatchRenderedOutput('0'); + expect(container.textContent).toEqual('0'); // Update that throws in a getSnapshot. We can catch it with an error boundary. await act(() => { store.set({value: 1, throwInGetSnapshot: true, throwInIsEqual: false}); }); - if (gate(flags => flags.variant)) { + if (gate(flags => flags.supportsNativeUseSyncExternalStore)) { expect(Scheduler).toHaveYielded([ 'Error in getSnapshot', // In a concurrent root, React renders a second time to attempt to @@ -546,7 +557,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { } else { expect(Scheduler).toHaveYielded(['Error in getSnapshot']); } - expect(root).toMatchRenderedOutput('Error in getSnapshot'); + expect(container.textContent).toEqual('Error in getSnapshot'); }); test('Infinite loop if getSnapshot keeps returning new reference', () => { @@ -557,19 +568,18 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { return ; } - spyOnDev(console, 'error'); - const root = createRoot(); + const container = document.createElement('div'); + const root = createRoot(container); - expect(() => act(() => root.render())).toThrow( - 'Maximum update depth exceeded. This can happen when a component repeatedly ' + - 'calls setState inside componentWillUpdate or componentDidUpdate. React limits ' + - 'the number of nested updates to prevent infinite loops.', - ); - if (__DEV__) { - expect(console.error.calls.argsFor(0)[0]).toMatch( - 'The result of getSnapshot should be cached to avoid an infinite loop', + expect(() => { + expect(() => act(() => root.render())).toThrow( + 'Maximum update depth exceeded. This can happen when a component repeatedly ' + + 'calls setState inside componentWillUpdate or componentDidUpdate. React limits ' + + 'the number of nested updates to prevent infinite loops.', ); - } + }).toErrorDev( + 'The result of getSnapshot should be cached to avoid an infinite loop', + ); }); describe('extra features implemented in user-space', () => { @@ -594,11 +604,12 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { return ; } - const root = createRoot(); + const container = document.createElement('div'); + const root = createRoot(container); act(() => root.render()); expect(Scheduler).toHaveYielded(['App', 'Selector', 'A0']); - expect(root).toMatchRenderedOutput('A0'); + expect(container.textContent).toEqual('A0'); // Update the store await act(() => { @@ -612,7 +623,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { // the previous result without running the selector again 'A1', ]); - expect(root).toMatchRenderedOutput('A1'); + expect(container.textContent).toEqual('A1'); }); // The selector implementation uses the lazy ref initialization pattern @@ -652,11 +663,12 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { ); } - const root = createRoot(); + const container = document.createElement('div'); + const root = createRoot(container); act(() => root.render()); expect(Scheduler).toHaveYielded(['A0', 'B0']); - expect(root).toMatchRenderedOutput('A0B0'); + expect(container.textContent).toEqual('A0B0'); // Update b but not a await act(() => { @@ -664,7 +676,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { }); // Only b re-renders expect(Scheduler).toHaveYielded(['B1']); - expect(root).toMatchRenderedOutput('A0B1'); + expect(container.textContent).toEqual('A0B1'); // Update a but not b await act(() => { @@ -672,7 +684,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { }); // Only a re-renders expect(Scheduler).toHaveYielded(['A1']); - expect(root).toMatchRenderedOutput('A1B1'); + expect(container.textContent).toEqual('A1B1'); }); }); @@ -725,7 +737,8 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { ); } - const root = createRoot(); + const container = document.createElement('div'); + const root = createRoot(container); await act(() => { root.render(); });