From b1dea262517b1bad173c658cf2c8bba37ed5d5c0 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 16 Apr 2019 16:57:08 +0100 Subject: [PATCH] Fix the remaining issue when primary child is null This lets us unlock the rest of the Suspense test. --- src/__tests__/store-test.js | 256 ++++++++++++++++++------------------ src/backend/renderer.js | 4 +- 2 files changed, 132 insertions(+), 128 deletions(-) diff --git a/src/__tests__/store-test.js b/src/__tests__/store-test.js index a457dd57e3..f060f9c282 100644 --- a/src/__tests__/store-test.js +++ b/src/__tests__/store-test.js @@ -637,144 +637,146 @@ describe('Store', () => { } } - // TODO: fix the bugs // 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'); - // act(() => ReactDOM.render( - // - // - // - // - // - // - // - // - // , - // container - // )); - // expect(print(store)).toEqual(snapshots[i]); - // // Re-render with steps[j]. - // act(() => ReactDOM.render( - // - // - // - // {steps[j]} - // - // - // , - // container - // )); - // // Verify the successful transition to steps[j]. - // expect(print(store)).toEqual(snapshots[j]); - // // Clean up after every iteration. - // act(() => ReactDOM.unmountComponentAtNode(container)); - // expect(print(store)).toBe(''); - // } - // } + 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'); + act(() => + ReactDOM.render( + + + + + + + + + , + container + ) + ); + expect(print(store)).toEqual(snapshots[i]); + // Re-render with steps[j]. + act(() => + ReactDOM.render( + + + {steps[j]} + + , + container + ) + ); + // Verify the successful transition to steps[j]. + expect(print(store)).toEqual(snapshots[j]); + // Clean up after every iteration. + act(() => ReactDOM.unmountComponentAtNode(container)); + expect(print(store)).toBe(''); + } + } - // TODO: fix the bugs // 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'); - // act(() => ReactDOM.render( - // - // - // - // {steps[i]} - // - // - // , - // container - // )); + 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'); + act(() => + ReactDOM.render( + + + {steps[i]} + + , + container + ) + ); - // // We get ID from the index in the tree above: - // // Root, X, Suspense, ... - // // ^ (index is 2) - // const suspenseID = store.getElementIDAtIndex(2); + // 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]); + // 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]); + // 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(() => ReactDOM.render( - // - // - // - // - // - // - // - // - // , - // container - // )); - // expect(print(store)).toEqual(snapshots[j]); + // Trigger actual fallback. + act(() => + ReactDOM.render( + + + + + + + + + , + container + ) + ); + 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]); + // 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(() => ReactDOM.render( - // - // - // - // {steps[i]} - // - // - // , - // container - // )); - // // Fallback is still forced though. - // expect(print(store)).toEqual(snapshots[j]); + // Switch to primary mode. + act(() => + ReactDOM.render( + + + {steps[i]} + + , + container + ) + ); + // 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]); + // 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(() => ReactDOM.unmountComponentAtNode(container)); - // expect(print(store)).toBe(''); - // } - // } + // Clean up after every iteration. + act(() => ReactDOM.unmountComponentAtNode(container)); + expect(print(store)).toBe(''); + } + } // TODO: // Test Concurrent Mode diff --git a/src/backend/renderer.js b/src/backend/renderer.js index 9895adafb6..ac79265d04 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -947,7 +947,9 @@ export function attach( // Note: don't emulate fallback unmount because React actually did it. // 2. Mount primary set const nextPrimaryChildSet = nextFiber.child; - mountFiberRecursively(nextPrimaryChildSet, nextFiber, true); + if (nextPrimaryChildSet !== null) { + mountFiberRecursively(nextPrimaryChildSet, nextFiber, true); + } shouldResetChildren = true; } else if (!prevDidTimeout && nextDidTimeOut) { // Primary -> Fallback: