From d830cd998490c12164da6a9e1ddff56daff80030 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 4 May 2020 01:43:51 +0100 Subject: [PATCH] [Blocks] Fix stale data on updates (#18810) * [Blocks] Failing test for nested load * Simplify the test * Add a similar test that fails in PROD * Copy .type when cloning work in progress --- .../react-reconciler/src/ReactFiber.new.js | 4 + .../react-reconciler/src/ReactFiber.old.js | 4 + .../src/__tests__/ReactBlocks-test.js | 83 +++++++++++++++++++ 3 files changed, 91 insertions(+) diff --git a/packages/react-reconciler/src/ReactFiber.new.js b/packages/react-reconciler/src/ReactFiber.new.js index b428e11374..c3f8d2b169 100644 --- a/packages/react-reconciler/src/ReactFiber.new.js +++ b/packages/react-reconciler/src/ReactFiber.new.js @@ -282,6 +282,8 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber { current.alternate = workInProgress; } else { workInProgress.pendingProps = pendingProps; + // Needed because Blocks store data on type. + workInProgress.type = current.type; // We already have an alternate. // Reset the effect tag. @@ -415,6 +417,8 @@ export function resetWorkInProgress(workInProgress: Fiber, renderLanes: Lanes) { workInProgress.memoizedProps = current.memoizedProps; workInProgress.memoizedState = current.memoizedState; workInProgress.updateQueue = current.updateQueue; + // Needed because Blocks store data on type. + workInProgress.type = current.type; // Clone the dependencies object. This is mutated during the render phase, so // it cannot be shared with the current fiber. diff --git a/packages/react-reconciler/src/ReactFiber.old.js b/packages/react-reconciler/src/ReactFiber.old.js index 3ef1b33032..1ebb767ec6 100644 --- a/packages/react-reconciler/src/ReactFiber.old.js +++ b/packages/react-reconciler/src/ReactFiber.old.js @@ -277,6 +277,8 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber { current.alternate = workInProgress; } else { workInProgress.pendingProps = pendingProps; + // Needed because Blocks store data on type. + workInProgress.type = current.type; // We already have an alternate. // Reset the effect tag. @@ -413,6 +415,8 @@ export function resetWorkInProgress( workInProgress.memoizedProps = current.memoizedProps; workInProgress.memoizedState = current.memoizedState; workInProgress.updateQueue = current.updateQueue; + // Needed because Blocks store data on type. + workInProgress.type = current.type; // Clone the dependencies object. This is mutated during the render phase, so // it cannot be shared with the current fiber. diff --git a/packages/react-reconciler/src/__tests__/ReactBlocks-test.js b/packages/react-reconciler/src/__tests__/ReactBlocks-test.js index b5cc0b2128..56be938649 100644 --- a/packages/react-reconciler/src/__tests__/ReactBlocks-test.js +++ b/packages/react-reconciler/src/__tests__/ReactBlocks-test.js @@ -257,4 +257,87 @@ describe('ReactBlocks', () => { , ); }); + + // Regression test. + // @gate experimental + it('does not render stale data after ping', async () => { + function Child() { + return Name: {readString('Sebastian')}; + } + + const loadParent = block( + function Parent(props, data) { + return ( + + {data.name ? : Empty} + + ); + }, + function load(name) { + return {name}; + }, + ); + + function App({Page}) { + return ; + } + + await ReactNoop.act(async () => { + ReactNoop.render(); + }); + expect(ReactNoop).toMatchRenderedOutput(Empty); + + await ReactNoop.act(async () => { + ReactNoop.render(); + }); + await ReactNoop.act(async () => { + jest.advanceTimersByTime(1000); + }); + expect(ReactNoop).toMatchRenderedOutput(Name: Sebastian); + }); + + // Regression test. + // @gate experimental + it('does not render stale data after ping and setState', async () => { + function Child() { + return Name: {readString('Sebastian')}; + } + + let _setSuspend; + const loadParent = block( + function Parent(props, data) { + const [suspend, setSuspend] = useState(true); + _setSuspend = setSuspend; + if (!suspend) { + return {data.name}; + } + return ( + + {data.name ? : Empty} + + ); + }, + function load(name) { + return {name}; + }, + ); + + function App({Page}) { + return ; + } + + await ReactNoop.act(async () => { + ReactNoop.render(); + }); + expect(ReactNoop).toMatchRenderedOutput(Empty); + + await ReactNoop.act(async () => { + ReactNoop.render(); + }); + await ReactNoop.act(async () => { + _setSuspend(false); + jest.advanceTimersByTime(1000); + }); + expect(ReactNoop).toMatchRenderedOutput(Sebastian); + }); });