From 0f44e9fb9854a3abf4a3e835fa9577edb9d3f67d Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 18 Apr 2019 16:09:45 +0100 Subject: [PATCH 1/3] Move sync stress tests in their own file --- .../__snapshots__/storeStress-test.js.snap | 239 ------------------ .../storeStressSync-test.js.snap | 239 ++++++++++++++++++ ...Stress-test.js => storeStressSync-test.js} | 37 +-- 3 files changed, 243 insertions(+), 272 deletions(-) delete mode 100644 src/__tests__/__snapshots__/storeStress-test.js.snap create mode 100644 src/__tests__/__snapshots__/storeStressSync-test.js.snap rename src/__tests__/{storeStress-test.js => storeStressSync-test.js} (94%) diff --git a/src/__tests__/__snapshots__/storeStress-test.js.snap b/src/__tests__/__snapshots__/storeStress-test.js.snap deleted file mode 100644 index 9deb51fb38..0000000000 --- a/src/__tests__/__snapshots__/storeStress-test.js.snap +++ /dev/null @@ -1,239 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`StoreStress should handle a stress test for Suspense 1`] = ` -[root] - ▾ - - ▾ - - -`; - -exports[`StoreStress should handle a stress test for Suspense 2`] = ` -[root] - ▾ - - ▾ - - -`; - -exports[`StoreStress should handle a stress test for Suspense 3`] = ` -[root] - ▾ - - ▾ - - - - -`; - -exports[`StoreStress should handle a stress test for Suspense 4`] = ` -[root] - ▾ - - ▾ - - - - -`; - -exports[`StoreStress should handle a stress test for Suspense 5`] = ` -[root] - ▾ - - ▾ - - - -`; - -exports[`StoreStress should handle a stress test for Suspense 6`] = ` -[root] - ▾ - - ▾ - - - -`; - -exports[`StoreStress should handle a stress test for Suspense 7`] = ` -[root] - ▾ - - ▾ - - - -`; - -exports[`StoreStress should handle a stress test for Suspense 8`] = ` -[root] - ▾ - - ▾ - - - -`; - -exports[`StoreStress should handle a stress test for Suspense 9`] = ` -[root] - ▾ - - ▾ - - -`; - -exports[`StoreStress should handle a stress test for Suspense 10`] = ` -[root] - ▾ - - - -`; - -exports[`StoreStress should handle a stress test for Suspense 11`] = ` -[root] - ▾ - - ▾ - - -`; - -exports[`StoreStress should handle a stress test for Suspense 12`] = ` -[root] - ▾ - - ▾ - - -`; - -exports[`StoreStress should handle a stress test with different tree operations: 1: abcde 1`] = ` -[root] - ▾ - - - - - -`; - -exports[`StoreStress should handle a stress test with different tree operations: 2: abxde 1`] = ` -[root] - ▾ - - - ▾ - - - -`; - -exports[`StoreStress should handle stress test with reordering 1`] = ` -[root] - ▾ - -`; - -exports[`StoreStress should handle stress test with reordering 2`] = ` -[root] - ▾ - -`; - -exports[`StoreStress should handle stress test with reordering 3`] = ` -[root] - ▾ - -`; - -exports[`StoreStress should handle stress test with reordering 4`] = ` -[root] - ▾ - -`; - -exports[`StoreStress should handle stress test with reordering 5`] = ` -[root] - ▾ - -`; - -exports[`StoreStress should handle stress test with reordering 6`] = ` -[root] - ▾ - -`; - -exports[`StoreStress should handle stress test with reordering 7`] = ` -[root] - ▾ - -`; - -exports[`StoreStress should handle stress test with reordering 8`] = ` -[root] - ▾ - -`; - -exports[`StoreStress should handle stress test with reordering 9`] = ` -[root] - ▾ - -`; - -exports[`StoreStress should handle stress test with reordering 10`] = ` -[root] - ▾ - -`; - -exports[`StoreStress should handle stress test with reordering 11`] = ` -[root] - ▾ - - -`; - -exports[`StoreStress should handle stress test with reordering 12`] = ` -[root] - ▾ - - -`; - -exports[`StoreStress should handle stress test with reordering 13`] = ` -[root] - ▾ - - -`; - -exports[`StoreStress should handle stress test with reordering 14`] = ` -[root] - ▾ - - -`; - -exports[`StoreStress should handle stress test with reordering 15`] = ` -[root] - ▾ - - -`; - -exports[`StoreStress should handle stress test with reordering 16`] = ` -[root] - ▾ - - -`; diff --git a/src/__tests__/__snapshots__/storeStressSync-test.js.snap b/src/__tests__/__snapshots__/storeStressSync-test.js.snap new file mode 100644 index 0000000000..d4a4f7e466 --- /dev/null +++ b/src/__tests__/__snapshots__/storeStressSync-test.js.snap @@ -0,0 +1,239 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`StoreStress (Sync Mode) should handle a stress test for Suspense (Sync Mode) 1`] = ` +[root] + ▾ + + ▾ + + +`; + +exports[`StoreStress (Sync Mode) should handle a stress test for Suspense (Sync Mode) 2`] = ` +[root] + ▾ + + ▾ + + +`; + +exports[`StoreStress (Sync Mode) should handle a stress test for Suspense (Sync Mode) 3`] = ` +[root] + ▾ + + ▾ + + + + +`; + +exports[`StoreStress (Sync Mode) should handle a stress test for Suspense (Sync Mode) 4`] = ` +[root] + ▾ + + ▾ + + + + +`; + +exports[`StoreStress (Sync Mode) should handle a stress test for Suspense (Sync Mode) 5`] = ` +[root] + ▾ + + ▾ + + + +`; + +exports[`StoreStress (Sync Mode) should handle a stress test for Suspense (Sync Mode) 6`] = ` +[root] + ▾ + + ▾ + + + +`; + +exports[`StoreStress (Sync Mode) should handle a stress test for Suspense (Sync Mode) 7`] = ` +[root] + ▾ + + ▾ + + + +`; + +exports[`StoreStress (Sync Mode) should handle a stress test for Suspense (Sync Mode) 8`] = ` +[root] + ▾ + + ▾ + + + +`; + +exports[`StoreStress (Sync Mode) should handle a stress test for Suspense (Sync Mode) 9`] = ` +[root] + ▾ + + ▾ + + +`; + +exports[`StoreStress (Sync Mode) should handle a stress test for Suspense (Sync Mode) 10`] = ` +[root] + ▾ + + + +`; + +exports[`StoreStress (Sync Mode) should handle a stress test for Suspense (Sync Mode) 11`] = ` +[root] + ▾ + + ▾ + + +`; + +exports[`StoreStress (Sync Mode) should handle a stress test for Suspense (Sync Mode) 12`] = ` +[root] + ▾ + + ▾ + + +`; + +exports[`StoreStress (Sync Mode) should handle a stress test with different tree operations (Sync Mode): 1: abcde 1`] = ` +[root] + ▾ + + + + + +`; + +exports[`StoreStress (Sync Mode) should handle a stress test with different tree operations (Sync Mode): 2: abxde 1`] = ` +[root] + ▾ + + + ▾ + + + +`; + +exports[`StoreStress (Sync Mode) should handle stress test with reordering (Sync Mode) 1`] = ` +[root] + ▾ + +`; + +exports[`StoreStress (Sync Mode) should handle stress test with reordering (Sync Mode) 2`] = ` +[root] + ▾ + +`; + +exports[`StoreStress (Sync Mode) should handle stress test with reordering (Sync Mode) 3`] = ` +[root] + ▾ + +`; + +exports[`StoreStress (Sync Mode) should handle stress test with reordering (Sync Mode) 4`] = ` +[root] + ▾ + +`; + +exports[`StoreStress (Sync Mode) should handle stress test with reordering (Sync Mode) 5`] = ` +[root] + ▾ + +`; + +exports[`StoreStress (Sync Mode) should handle stress test with reordering (Sync Mode) 6`] = ` +[root] + ▾ + +`; + +exports[`StoreStress (Sync Mode) should handle stress test with reordering (Sync Mode) 7`] = ` +[root] + ▾ + +`; + +exports[`StoreStress (Sync Mode) should handle stress test with reordering (Sync Mode) 8`] = ` +[root] + ▾ + +`; + +exports[`StoreStress (Sync Mode) should handle stress test with reordering (Sync Mode) 9`] = ` +[root] + ▾ + +`; + +exports[`StoreStress (Sync Mode) should handle stress test with reordering (Sync Mode) 10`] = ` +[root] + ▾ + +`; + +exports[`StoreStress (Sync Mode) should handle stress test with reordering (Sync Mode) 11`] = ` +[root] + ▾ + + +`; + +exports[`StoreStress (Sync Mode) should handle stress test with reordering (Sync Mode) 12`] = ` +[root] + ▾ + + +`; + +exports[`StoreStress (Sync Mode) should handle stress test with reordering (Sync Mode) 13`] = ` +[root] + ▾ + + +`; + +exports[`StoreStress (Sync Mode) should handle stress test with reordering (Sync Mode) 14`] = ` +[root] + ▾ + + +`; + +exports[`StoreStress (Sync Mode) should handle stress test with reordering (Sync Mode) 15`] = ` +[root] + ▾ + + +`; + +exports[`StoreStress (Sync Mode) should handle stress test with reordering (Sync Mode) 16`] = ` +[root] + ▾ + + +`; diff --git a/src/__tests__/storeStress-test.js b/src/__tests__/storeStressSync-test.js similarity index 94% rename from src/__tests__/storeStress-test.js rename to src/__tests__/storeStressSync-test.js index 12e111adf5..30f48d9a26 100644 --- a/src/__tests__/storeStress-test.js +++ b/src/__tests__/storeStressSync-test.js @@ -1,6 +1,6 @@ // @flow -describe('StoreStress', () => { +describe('StoreStress (Sync Mode)', () => { let React; let ReactDOM; let TestUtils; @@ -29,7 +29,7 @@ describe('StoreStress', () => { // This is a stress test for the tree mount/update/unmount traversal. // It renders different trees that should produce the same output. - it('should handle a stress test with different tree operations', () => { + it('should handle a stress test with different tree operations (Sync Mode)', () => { let setShowX; const A = () => 'a'; const B = () => 'b'; @@ -168,37 +168,9 @@ describe('StoreStress', () => { } act(() => ReactDOM.unmountComponentAtNode(container)); expect(print(store)).toBe(''); - - // 7. Same as the previous step, but for Concurrent Mode. - container = document.createElement('div'); - // $FlowFixMe - let root = ReactDOM.unstable_createRoot(container); - for (let i = 0; i < cases.length; i++) { - // Verify mounting 'abcde'. - act(() => root.render({cases[i]})); - expect(container.textContent).toMatch('abcde'); - expect(print(store)).toEqual(snapshotForABCDE); - - // Verify switching to 'abxde'. - act(() => { - setShowX(true); - }); - expect(container.textContent).toMatch('abxde'); - expect(print(store)).toBe(snapshotForABXDE); - - // Verify switching back to 'abcde'. - act(() => { - setShowX(false); - }); - expect(container.textContent).toMatch('abcde'); - expect(print(store)).toBe(snapshotForABCDE); - // Don't unmount. Reuse the container between iterations. - } - act(() => root.unmount()); - expect(print(store)).toBe(''); }); - it('should handle stress test with reordering', () => { + it('should handle stress test with reordering (Sync Mode)', () => { const A = () => 'a'; const B = () => 'b'; const C = () => 'c'; @@ -298,7 +270,7 @@ describe('StoreStress', () => { } }); - it('should handle a stress test for Suspense', async () => { + it('should handle a stress test for Suspense (Sync Mode)', async () => { const A = () => 'a'; const B = () => 'b'; const C = () => 'c'; @@ -690,6 +662,5 @@ describe('StoreStress', () => { expect(print(store)).toBe(''); } } - // TODO: Test Concurrent Mode }); }); From 24309fde06f0603f968532d4e41ec421e3434861 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 18 Apr 2019 16:10:44 +0100 Subject: [PATCH 2/3] Add failing Concurrent Mode stress tests --- .../storeStressTestConcurrent-test.js.snap | 239 +++++++ .../storeStressTestConcurrent-test.js | 671 ++++++++++++++++++ 2 files changed, 910 insertions(+) create mode 100644 src/__tests__/__snapshots__/storeStressTestConcurrent-test.js.snap create mode 100644 src/__tests__/storeStressTestConcurrent-test.js diff --git a/src/__tests__/__snapshots__/storeStressTestConcurrent-test.js.snap b/src/__tests__/__snapshots__/storeStressTestConcurrent-test.js.snap new file mode 100644 index 0000000000..22f2fdf406 --- /dev/null +++ b/src/__tests__/__snapshots__/storeStressTestConcurrent-test.js.snap @@ -0,0 +1,239 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`StoreStressConcurrent should handle a stress test for Suspense (Concurrent Mode) 1`] = ` +[root] + ▾ + + ▾ + + +`; + +exports[`StoreStressConcurrent should handle a stress test for Suspense (Concurrent Mode) 2`] = ` +[root] + ▾ + + ▾ + + +`; + +exports[`StoreStressConcurrent should handle a stress test for Suspense (Concurrent Mode) 3`] = ` +[root] + ▾ + + ▾ + + + + +`; + +exports[`StoreStressConcurrent should handle a stress test for Suspense (Concurrent Mode) 4`] = ` +[root] + ▾ + + ▾ + + + + +`; + +exports[`StoreStressConcurrent should handle a stress test for Suspense (Concurrent Mode) 5`] = ` +[root] + ▾ + + ▾ + + + +`; + +exports[`StoreStressConcurrent should handle a stress test for Suspense (Concurrent Mode) 6`] = ` +[root] + ▾ + + ▾ + + + +`; + +exports[`StoreStressConcurrent should handle a stress test for Suspense (Concurrent Mode) 7`] = ` +[root] + ▾ + + ▾ + + + +`; + +exports[`StoreStressConcurrent should handle a stress test for Suspense (Concurrent Mode) 8`] = ` +[root] + ▾ + + ▾ + + + +`; + +exports[`StoreStressConcurrent should handle a stress test for Suspense (Concurrent Mode) 9`] = ` +[root] + ▾ + + ▾ + + +`; + +exports[`StoreStressConcurrent should handle a stress test for Suspense (Concurrent Mode) 10`] = ` +[root] + ▾ + + + +`; + +exports[`StoreStressConcurrent should handle a stress test for Suspense (Concurrent Mode) 11`] = ` +[root] + ▾ + + ▾ + + +`; + +exports[`StoreStressConcurrent should handle a stress test for Suspense (Concurrent Mode) 12`] = ` +[root] + ▾ + + ▾ + + +`; + +exports[`StoreStressConcurrent should handle a stress test with different tree operations (Concurrent Mode): 1: abcde 1`] = ` +[root] + ▾ + + + + + +`; + +exports[`StoreStressConcurrent should handle a stress test with different tree operations (Concurrent Mode): 2: abxde 1`] = ` +[root] + ▾ + + + ▾ + + + +`; + +exports[`StoreStressConcurrent should handle stress test with reordering (Concurrent Mode) 1`] = ` +[root] + ▾ + +`; + +exports[`StoreStressConcurrent should handle stress test with reordering (Concurrent Mode) 2`] = ` +[root] + ▾ + +`; + +exports[`StoreStressConcurrent should handle stress test with reordering (Concurrent Mode) 3`] = ` +[root] + ▾ + +`; + +exports[`StoreStressConcurrent should handle stress test with reordering (Concurrent Mode) 4`] = ` +[root] + ▾ + +`; + +exports[`StoreStressConcurrent should handle stress test with reordering (Concurrent Mode) 5`] = ` +[root] + ▾ + +`; + +exports[`StoreStressConcurrent should handle stress test with reordering (Concurrent Mode) 6`] = ` +[root] + ▾ + +`; + +exports[`StoreStressConcurrent should handle stress test with reordering (Concurrent Mode) 7`] = ` +[root] + ▾ + +`; + +exports[`StoreStressConcurrent should handle stress test with reordering (Concurrent Mode) 8`] = ` +[root] + ▾ + +`; + +exports[`StoreStressConcurrent should handle stress test with reordering (Concurrent Mode) 9`] = ` +[root] + ▾ + +`; + +exports[`StoreStressConcurrent should handle stress test with reordering (Concurrent Mode) 10`] = ` +[root] + ▾ + +`; + +exports[`StoreStressConcurrent should handle stress test with reordering (Concurrent Mode) 11`] = ` +[root] + ▾ + + +`; + +exports[`StoreStressConcurrent should handle stress test with reordering (Concurrent Mode) 12`] = ` +[root] + ▾ + + +`; + +exports[`StoreStressConcurrent should handle stress test with reordering (Concurrent Mode) 13`] = ` +[root] + ▾ + + +`; + +exports[`StoreStressConcurrent should handle stress test with reordering (Concurrent Mode) 14`] = ` +[root] + ▾ + + +`; + +exports[`StoreStressConcurrent should handle stress test with reordering (Concurrent Mode) 15`] = ` +[root] + ▾ + + +`; + +exports[`StoreStressConcurrent should handle stress test with reordering (Concurrent Mode) 16`] = ` +[root] + ▾ + + +`; diff --git a/src/__tests__/storeStressTestConcurrent-test.js b/src/__tests__/storeStressTestConcurrent-test.js new file mode 100644 index 0000000000..6bfd867337 --- /dev/null +++ b/src/__tests__/storeStressTestConcurrent-test.js @@ -0,0 +1,671 @@ +// @flow + +describe('StoreStressConcurrent', () => { + let React; + let ReactDOM; + let TestUtils; + let bridge; + let store; + let print; + + const act = (callback: Function) => { + TestUtils.act(() => { + callback(); + }); + jest.advanceTimersByTime(1000); // Flush rendering and Suspense + jest.runAllTimers(); // Flush Bridge operations + }; + + beforeEach(() => { + bridge = global.bridge; + store = global.store; + store.collapseNodesByDefault = false; + + React = require('react'); + ReactDOM = require('react-dom'); + TestUtils = require('react-dom/test-utils'); + + print = require('./storeSerializer').print; + }); + + // This is a stress test for the tree mount/update/unmount traversal. + // It renders different trees that should produce the same output. + it('should handle a stress test with different tree operations (Concurrent Mode)', () => { + let setShowX; + const A = () => 'a'; + const B = () => 'b'; + const C = () => { + // We'll be manually flipping this component back and forth in the test. + // We only do this for a single node in order to verify that DevTools + // can handle a subtree switching alternates while other subtrees are memoized. + let [showX, _setShowX] = React.useState(false); + setShowX = _setShowX; + return showX ? : 'c'; + }; + const D = () => 'd'; + const E = () => 'e'; + const X = () => 'x'; + const a = ; + const b = ; + const c = ; + const d = ; + const e = ; + + function Parent({ children }) { + return children; + } + + // 1. Render a normal version of [a, b, c, d, e]. + let container = document.createElement('div'); + // $FlowFixMe + let root = ReactDOM.unstable_createRoot(container); + act(() => root.render({[a, b, c, d, e]})); + expect(store).toMatchSnapshot('1: abcde'); + expect(container.textContent).toMatch('abcde'); + const snapshotForABCDE = print(store); + + // 2. Render a version where renders an child instead of 'c'. + // This is how we'll test an update to a single component. + act(() => { + setShowX(true); + }); + expect(store).toMatchSnapshot('2: abxde'); + expect(container.textContent).toMatch('abxde'); + const snapshotForABXDE = print(store); + + // 3. Verify flipping it back produces the original result. + act(() => { + setShowX(false); + }); + expect(container.textContent).toMatch('abcde'); + expect(print(store)).toBe(snapshotForABCDE); + + // 4. Clean up. + act(() => root.unmount()); + expect(print(store)).toBe(''); + + // Now comes the interesting part. + // All of these cases are equivalent to [a, b, c, d, e] in output. + // We'll verify that DevTools produces the same snapshots for them. + // These cases are picked so that rendering them sequentially in the same + // container results in a combination of mounts, updates, unmounts, and reorders. + // prettier-ignore + let cases = [ + [a, b, c, d, e], + [[a], b, c, d, e], + [[a, b], c, d, e], + [[a, b], c, [d, e]], + [[a, b], c, [d, '', e]], + [[a], b, c, d, [e]], + [a, b, [[c]], d, e], + [[a, ''], [b], [c], [d], [e]], + [a, b, [c, [d, ['', e]]]], + [a, b, c, d, e], + [
{a}
, b, c, d, e], + [
{a}{b}
, c, d, e], + [
{a}{b}
, c,
{d}{e}
], + [
{a}{b}
, c,
{d}{e}
], + [
{a}{b}
, c,
{d}{e}
], + [
{a}{b}
, c,
{d}{e}
], + [{a}, b, c, d, [e]], + [a, b, {c}, d, e], + [
{a}
, [b], {c}, [d],
{e}
], + [a, b, [c,
{d}{e}
], ''], + [a, [[]], b, c, [d, [[]], e]], + [[[a, b, c, d], e]], + [a, b, c, d, e] + ]; + + // 5. Test fresh mount for each case. + for (let i = 0; i < cases.length; i++) { + // Ensure fresh mount. + container = document.createElement('div'); + // $FlowFixMe + root = ReactDOM.unstable_createRoot(container); + + // Verify mounting 'abcde'. + act(() => root.render({cases[i]})); + expect(container.textContent).toMatch('abcde'); + expect(print(store)).toEqual(snapshotForABCDE); + + // Verify switching to 'abxde'. + act(() => { + setShowX(true); + }); + expect(container.textContent).toMatch('abxde'); + expect(print(store)).toBe(snapshotForABXDE); + + // Verify switching back to 'abcde'. + act(() => { + setShowX(false); + }); + expect(container.textContent).toMatch('abcde'); + expect(print(store)).toBe(snapshotForABCDE); + + // Clean up. + act(() => root.unmount()); + expect(print(store)).toBe(''); + } + + // 6. Verify *updates* by reusing the container between iterations. + // There'll be no unmounting until the very end. + container = document.createElement('div'); + // $FlowFixMe + root = ReactDOM.unstable_createRoot(container); + for (let i = 0; i < cases.length; i++) { + // Verify mounting 'abcde'. + act(() => root.render({cases[i]})); + expect(container.textContent).toMatch('abcde'); + expect(print(store)).toEqual(snapshotForABCDE); + + // Verify switching to 'abxde'. + act(() => { + setShowX(true); + }); + expect(container.textContent).toMatch('abxde'); + expect(print(store)).toBe(snapshotForABXDE); + + // Verify switching back to 'abcde'. + act(() => { + setShowX(false); + }); + expect(container.textContent).toMatch('abcde'); + expect(print(store)).toBe(snapshotForABCDE); + // Don't unmount. Reuse the container between iterations. + } + act(() => root.unmount()); + expect(print(store)).toBe(''); + }); + + it('should handle stress test with reordering (Concurrent Mode)', () => { + const A = () => 'a'; + const B = () => 'b'; + const C = () => 'c'; + const D = () => 'd'; + const E = () => 'e'; + const a = ; + const b = ; + const c = ; + const d = ; + const e = ; + + // prettier-ignore + let steps = [ + a, + b, + c, + d, + e, + [a], + [b], + [c], + [d], + [e], + [a, b], + [b, a], + [b, c], + [c, b], + [a, c], + [c, a], + ]; + + const Root = ({ children }) => { + return children; + }; + + // 1. Capture the expected render result. + let snapshots = []; + let container = document.createElement('div'); + // $FlowFixMe + let root = ReactDOM.unstable_createRoot(container); + for (let i = 0; i < steps.length; i++) { + act(() => root.render({steps[i]})); + // We snapshot each step once so it doesn't regress. + expect(store).toMatchSnapshot(); + snapshots.push(print(store)); + act(() => root.unmount()); + expect(print(store)).toBe(''); + } + + // 2. Verify that we can update from every step to every other step and back. + for (let i = 0; i < steps.length; i++) { + for (let j = 0; j < steps.length; j++) { + let container = document.createElement('div'); + // $FlowFixMe + let root = ReactDOM.unstable_createRoot(container); + act(() => root.render({steps[i]})); + expect(print(store)).toMatch(snapshots[i]); + act(() => root.render({steps[j]})); + expect(print(store)).toMatch(snapshots[j]); + act(() => root.render({steps[i]})); + expect(print(store)).toMatch(snapshots[i]); + act(() => root.unmount()); + expect(print(store)).toBe(''); + } + } + + // 3. Same test as above, but this time we wrap children in a host component. + for (let i = 0; i < steps.length; i++) { + for (let j = 0; j < steps.length; j++) { + let container = document.createElement('div'); + // $FlowFixMe + let root = ReactDOM.unstable_createRoot(container); + act(() => + root.render( + +
{steps[i]}
+ + ) + ); + expect(print(store)).toMatch(snapshots[i]); + act(() => + root.render( + +
{steps[j]}
+ + ) + ); + expect(print(store)).toMatch(snapshots[j]); + act(() => + root.render( + +
{steps[i]}
+ + ) + ); + expect(print(store)).toMatch(snapshots[i]); + act(() => root.unmount()); + expect(print(store)).toBe(''); + } + } + }); + + it('should handle a stress test for Suspense (Concurrent Mode)', async () => { + const A = () => 'a'; + const B = () => 'b'; + const C = () => 'c'; + const X = () => 'x'; + const Y = () => 'y'; + const Z = () => 'z'; + const a = ; + const b = ; + const c = ; + const z = ; + + // prettier-ignore + const steps = [ + a, + [a], + [a, b, c], + [c, b, a], + [c, null, a], + {c}{a}, +
{c}{a}
, +
{a}{b}
, + [[a]], + null, + b, + a + ]; + + const Never = () => { + throw new Promise(() => {}); + }; + + const Root = ({ children }) => { + return children; + }; + + // 1. For each step, check Suspense can render them as initial primary content. + // This is the only step where we use Jest snapshots. + let snapshots = []; + let container = document.createElement('div'); + // $FlowFixMe + let root = ReactDOM.unstable_createRoot(container); + for (let i = 0; i < steps.length; i++) { + act(() => + root.render( + + + {steps[i]} + + + ) + ); + // We snapshot each step once so it doesn't regress. + expect(store).toMatchSnapshot(); + snapshots.push(print(store)); + act(() => root.unmount()); + expect(print(store)).toBe(''); + } + + // 2. Verify check Suspense can render same steps as initial fallback content. + for (let i = 0; i < steps.length; i++) { + act(() => + root.render( + + + + + + + + + + ) + ); + expect(print(store)).toEqual(snapshots[i]); + act(() => root.unmount()); + expect(print(store)).toBe(''); + } + + // 3. Verify we can update from each step to each step in primary mode. + for (let i = 0; i < steps.length; i++) { + for (let j = 0; j < steps.length; j++) { + // Always start with a fresh container and steps[i]. + container = document.createElement('div'); + // $FlowFixMe + root = ReactDOM.unstable_createRoot(container); + act(() => + root.render( + + + {steps[i]} + + + ) + ); + expect(print(store)).toEqual(snapshots[i]); + // Re-render with steps[j]. + act(() => + root.render( + + + {steps[j]} + + + ) + ); + // Verify the successful transition to steps[j]. + expect(print(store)).toEqual(snapshots[j]); + // Check that we can transition back again. + act(() => + root.render( + + + {steps[i]} + + + ) + ); + expect(print(store)).toEqual(snapshots[i]); + // Clean up after every iteration. + act(() => root.unmount()); + expect(print(store)).toBe(''); + } + } + + // 4. Verify we can update from each step to each step in fallback mode. + for (let i = 0; i < steps.length; i++) { + for (let j = 0; j < steps.length; j++) { + // Always start with a fresh container and steps[i]. + container = document.createElement('div'); + // $FlowFixMe + root = ReactDOM.unstable_createRoot(container); + act(() => + root.render( + + + + + + + + + + ) + ); + expect(print(store)).toEqual(snapshots[i]); + // Re-render with steps[j]. + act(() => + root.render( + + + + + + + + + + ) + ); + // Verify the successful transition to steps[j]. + expect(print(store)).toEqual(snapshots[j]); + // Check that we can transition back again. + act(() => + root.render( + + + + + + + + + + ) + ); + expect(print(store)).toEqual(snapshots[i]); + // Clean up after every iteration. + act(() => root.unmount()); + expect(print(store)).toBe(''); + } + } + + // 5. Verify we can update from each step to each step when moving primary -> fallback. + for (let i = 0; i < steps.length; i++) { + for (let j = 0; j < steps.length; j++) { + // Always start with a fresh container and steps[i]. + container = document.createElement('div'); + // $FlowFixMe + root = ReactDOM.unstable_createRoot(container); + act(() => + root.render( + + + {steps[i]} + + + ) + ); + expect(print(store)).toEqual(snapshots[i]); + // Re-render with steps[j]. + act(() => + root.render( + + + + + + + + + + ) + ); + // Verify the successful transition to steps[j]. + expect(print(store)).toEqual(snapshots[j]); + // Check that we can transition back again. + act(() => + root.render( + + + {steps[i]} + + + ) + ); + expect(print(store)).toEqual(snapshots[i]); + // Clean up after every iteration. + act(() => root.unmount()); + expect(print(store)).toBe(''); + } + } + + // 6. Verify we can update from each step to each step when moving fallback -> primary. + for (let i = 0; i < steps.length; i++) { + for (let j = 0; j < steps.length; j++) { + // Always start with a fresh container and steps[i]. + container = document.createElement('div'); + // $FlowFixMe + root = ReactDOM.unstable_createRoot(container); + act(() => + root.render( + + + + + + + + + + ) + ); + expect(print(store)).toEqual(snapshots[i]); + // Re-render with steps[j]. + act(() => + root.render( + + + {steps[j]} + + + ) + ); + // Verify the successful transition to steps[j]. + expect(print(store)).toEqual(snapshots[j]); + // Check that we can transition back again. + act(() => + root.render( + + + + + + + + + + ) + ); + expect(print(store)).toEqual(snapshots[i]); + // Clean up after every iteration. + act(() => root.unmount()); + expect(print(store)).toBe(''); + } + } + + // 7. Verify we can update from each step to each step when toggling Suspense. + for (let i = 0; i < steps.length; i++) { + for (let j = 0; j < steps.length; j++) { + // Always start with a fresh container and steps[i]. + container = document.createElement('div'); + // $FlowFixMe + root = ReactDOM.unstable_createRoot(container); + act(() => + root.render( + + + {steps[i]} + + + ) + ); + + // We get ID from the index in the tree above: + // Root, X, Suspense, ... + // ^ (index is 2) + const suspenseID = store.getElementIDAtIndex(2); + + // Force fallback. + expect(print(store)).toEqual(snapshots[i]); + act(() => { + const suspenseID = store.getElementIDAtIndex(2); + bridge.send('overrideSuspense', { + id: suspenseID, + rendererID: store.getRendererIDForElement(suspenseID), + forceFallback: true, + }); + }); + expect(print(store)).toEqual(snapshots[j]); + + // Stop forcing fallback. + act(() => { + bridge.send('overrideSuspense', { + id: suspenseID, + rendererID: store.getRendererIDForElement(suspenseID), + forceFallback: false, + }); + }); + expect(print(store)).toEqual(snapshots[i]); + + // Trigger actual fallback. + act(() => + root.render( + + + + + + + + + + ) + ); + expect(print(store)).toEqual(snapshots[j]); + + // Force fallback while we're in fallback mode. + act(() => { + bridge.send('overrideSuspense', { + id: suspenseID, + rendererID: store.getRendererIDForElement(suspenseID), + forceFallback: true, + }); + }); + // Keep seeing fallback content. + expect(print(store)).toEqual(snapshots[j]); + + // Switch to primary mode. + act(() => + root.render( + + + {steps[i]} + + + ) + ); + // Fallback is still forced though. + expect(print(store)).toEqual(snapshots[j]); + + // Stop forcing fallback. This reverts to primary content. + act(() => { + bridge.send('overrideSuspense', { + id: suspenseID, + rendererID: store.getRendererIDForElement(suspenseID), + forceFallback: false, + }); + }); + // Now we see primary content. + expect(print(store)).toEqual(snapshots[i]); + + // Clean up after every iteration. + act(() => root.unmount()); + expect(print(store)).toBe(''); + } + } + }); +}); From 6a13ffd2314275b9e1ae8eda5acd614d22edc15a Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 18 Apr 2019 16:19:09 +0100 Subject: [PATCH 3/3] Traverse the previous current tree when switching from primary to fallback --- src/backend/renderer.js | 29 ++++++------ src/constants.js | 1 - src/devtools/store.js | 46 ------------------- .../views/Profiler/CommitTreeBuilder.js | 32 ------------- 4 files changed, 14 insertions(+), 94 deletions(-) diff --git a/src/backend/renderer.js b/src/backend/renderer.js index 870ceece79..71c10da392 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -22,7 +22,6 @@ import { TREE_OPERATION_ADD, TREE_OPERATION_REMOVE, TREE_OPERATION_RESET_CHILDREN, - TREE_OPERATION_RECURSIVE_REMOVE_CHILDREN, TREE_OPERATION_UPDATE_TREE_BASE_DURATION, } from '../constants'; import { getUID } from '../utils'; @@ -772,15 +771,6 @@ export function attach( } } - function recordRecursiveRemoveChildren(fiber) { - const primaryFiber = getPrimaryFiber(fiber); - const id = getFiberID(primaryFiber); - beginNextOperation(2); - nextOperation[0] = TREE_OPERATION_RECURSIVE_REMOVE_CHILDREN; - nextOperation[1] = id; - endNextOperation(false); - } - function mountFiberRecursively( fiber: Fiber, parentFiber: Fiber | null, @@ -828,6 +818,18 @@ export function attach( } } + function unmountFiberChildrenRecursively(fiber: Fiber) { + if (__DEBUG__) { + debug('unmountFiberChildrenRecursively()', fiber); + } + let child = fiber.child; + while (child !== null) { + recordUnmount(child); + unmountFiberChildrenRecursively(child); + child = child.sibling; + } + } + function recordTreeDuration(fiber: Fiber) { const id = getFiberID(getPrimaryFiber(fiber)); const { actualDuration, treeBaseDuration } = fiber; @@ -955,11 +957,8 @@ export function attach( // Primary -> Fallback: // 1. Hide primary set // This is not a real unmount, so it won't get reported by React. - // By this point it's *too late* to find the previous primary child set - // so we'll just tell the store to "forget" about those children. - // They might "resurface" later when we switch to primary content, - // but from the store's point of view they will be a new tree. - recordRecursiveRemoveChildren(nextFiber); + // We need to manually walk the previous tree and record unmounts. + unmountFiberChildrenRecursively(prevFiber); // 2. Mount fallback set const nextFallbackChildSet = nextFiber.child.sibling; mountFiberRecursively(nextFallbackChildSet, nextFiber, true); diff --git a/src/constants.js b/src/constants.js index d4ba283319..ef69e21760 100644 --- a/src/constants.js +++ b/src/constants.js @@ -4,7 +4,6 @@ export const TREE_OPERATION_ADD = 1; export const TREE_OPERATION_REMOVE = 2; export const TREE_OPERATION_RESET_CHILDREN = 3; export const TREE_OPERATION_UPDATE_TREE_BASE_DURATION = 4; -export const TREE_OPERATION_RECURSIVE_REMOVE_CHILDREN = 5; export const LOCAL_STORAGE_RELOAD_AND_PROFILE_KEY = 'React::DevTools::reloadAndProfile'; diff --git a/src/devtools/store.js b/src/devtools/store.js index 1e37ffcc1e..d1bc5884e1 100644 --- a/src/devtools/store.js +++ b/src/devtools/store.js @@ -5,7 +5,6 @@ import memoize from 'memoize-one'; import throttle from 'lodash.throttle'; import { TREE_OPERATION_ADD, - TREE_OPERATION_RECURSIVE_REMOVE_CHILDREN, TREE_OPERATION_REMOVE, TREE_OPERATION_RESET_CHILDREN, TREE_OPERATION_UPDATE_TREE_BASE_DURATION, @@ -695,51 +694,6 @@ export default class Store extends EventEmitter { weightDelta = 1; } break; - case TREE_OPERATION_RECURSIVE_REMOVE_CHILDREN: { - id = ((operations[i + 1]: any): number); - - if (!this._idToElement.has(id)) { - throw new Error( - 'Store does not contain fiber ' + - id + - '. This is a bug in React DevTools.' - ); - } - - i = i + 2; - - let justRemovedIDs = []; - const recursivelyRemove = childID => { - justRemovedIDs.push(childID); - const child = this._idToElement.get(childID); - if (!child) { - throw new Error( - 'Store does not contain fiber ' + - childID + - '. This is a bug in React DevTools.' - ); - } - this._idToElement.delete(childID); - child.children.forEach(recursivelyRemove); - }; - - // Track removed items so search results can be updated - const oldRemovedElementIDs = removedElementIDs; - removedElementIDs = new Uint32Array( - removedElementIDs.length + justRemovedIDs.length - ); - removedElementIDs.set(oldRemovedElementIDs); - let startIndex = oldRemovedElementIDs.length; - for (let j = 0; j < justRemovedIDs.length; j++) { - removedElementIDs[startIndex + j] = oldRemovedElementIDs[j]; - } - - parentElement = ((this._idToElement.get(id): any): Element); - parentElement.children.forEach(recursivelyRemove); - parentElement.children = []; - weightDelta = -parentElement.weight + 1; - break; - } case TREE_OPERATION_REMOVE: { id = ((operations[i + 1]: any): number); diff --git a/src/devtools/views/Profiler/CommitTreeBuilder.js b/src/devtools/views/Profiler/CommitTreeBuilder.js index 2629fbeedb..b077f51e3b 100644 --- a/src/devtools/views/Profiler/CommitTreeBuilder.js +++ b/src/devtools/views/Profiler/CommitTreeBuilder.js @@ -3,7 +3,6 @@ import { __DEBUG__, TREE_OPERATION_ADD, - TREE_OPERATION_RECURSIVE_REMOVE_CHILDREN, TREE_OPERATION_REMOVE, TREE_OPERATION_RESET_CHILDREN, TREE_OPERATION_UPDATE_TREE_BASE_DURATION, @@ -256,37 +255,6 @@ function updateTree( nodes.set(id, node); } break; - case TREE_OPERATION_RECURSIVE_REMOVE_CHILDREN: - id = ((operations[i + 1]: any): number); - - i = i + 2; - - if (!nodes.has(id)) { - throw new Error( - 'Commit tree does not contain fiber ' + - id + - '. This is a bug in React DevTools.' - ); - } - - node = getClonedNode(id); - - const recursivelyRemove = childID => { - if (!nodes.has(id)) { - throw new Error( - 'Commit tree does not contain fiber ' + - id + - '. This is a bug in React DevTools.' - ); - } - const child = getClonedNode(childID); - nodes.delete(childID); - child.children.forEach(recursivelyRemove); - }; - - node.children.forEach(recursivelyRemove); - node.children = []; - break; case TREE_OPERATION_REMOVE: id = ((operations[i + 1]: any): number);