Skip to content

Commit

Permalink
Failing test: Updates "un-committed" when rebasing
Browse files Browse the repository at this point in the history
Adds a failing test case where an update that was committed is
later skipped over during a rebase. This should never happen.
  • Loading branch information
acdlite committed Dec 5, 2019
1 parent acfe4b2 commit b867f5c
Showing 1 changed file with 64 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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(<App />);
});
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');
});
});

0 comments on commit b867f5c

Please sign in to comment.