From b867f5c765b2a4a639066a41d5cd2a549e72f15d Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 21 Nov 2019 14:20:47 -0800 Subject: [PATCH] Failing test: Updates "un-committed" when rebasing Adds a failing test case where an update that was committed is later skipped over during a rebase. This should never happen. --- .../ReactIncrementalUpdates-test.internal.js | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js index 7ae9874105dce..095f3d8979a6a 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js @@ -653,4 +653,68 @@ describe('ReactIncrementalUpdates', () => { expect(Scheduler).toFlushAndYield(['Commit: goodbye']); }); }); + + it('when rebasing, does not exclude updates that were already committed, regardless of priority', async () => { + const {useState, useLayoutEffect} = React; + + let pushToLog; + function App() { + const [log, setLog] = useState(''); + pushToLog = msg => { + setLog(prevLog => prevLog + msg); + }; + + useLayoutEffect( + () => { + Scheduler.unstable_yieldValue('Committed: ' + log); + if (log === 'B') { + // Right after B commits, schedule additional updates. + Scheduler.unstable_runWithPriority( + Scheduler.unstable_UserBlockingPriority, + () => { + pushToLog('C'); + }, + ); + setLog(prevLog => prevLog + 'D'); + } + }, + [log], + ); + + return log; + } + + const root = ReactNoop.createRoot(); + await ReactNoop.act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded(['Committed: ']); + expect(root).toMatchRenderedOutput(''); + + await ReactNoop.act(async () => { + pushToLog('A'); + Scheduler.unstable_runWithPriority( + Scheduler.unstable_UserBlockingPriority, + () => { + pushToLog('B'); + }, + ); + }); + expect(Scheduler).toHaveYielded([ + // A and B are pending. B is higher priority, so we'll render that first. + 'Committed: B', + // Because A comes first in the queue, we're now in rebase mode. B must + // be rebased on top of A. Also, in a layout effect, we received two new + // updates: C and D. C is user-blocking and D is synchronous. + // + // First render the synchronous update. What we're testing here is that + // B *is not dropped* even though it has lower than sync priority. That's + // because we already committed it. However, this render should not + // include C, because that update wasn't already committed. + 'Committed: BD', + 'Committed: BCD', + 'Committed: ABCD', + ]); + expect(root).toMatchRenderedOutput('ABCD'); + }); });