Skip to content

Commit cfefc81

Browse files
authored
* Failing test for facebook#18657 * Remove incorrect priority check I think this was just poor factoring on my part in facebook#18411. Honestly it doesn't make much sense to me, but my best guess is that I must have thought that when `baseTime > currentChildExpirationTime`, the function would fall through to the `currentChildExpirationTime < renderExpirationTime` branch below. Really I think just made an oopsie. Regardless, this logic is galaxy brainéd. A goal of the Lanes refactor I'm working on is to make these types of checks -- is there remaining work in this tree? -- a lot easier to think about. Hopefully.
1 parent e7163a9 commit cfefc81

File tree

3 files changed

+51
-10
lines changed

3 files changed

+51
-10
lines changed

packages/react-reconciler/src/ReactFiberBeginWork.new.js

+1-5
Original file line numberDiff line numberDiff line change
@@ -1681,11 +1681,7 @@ function getRemainingWorkInPrimaryTree(
16811681
// This boundary already timed out. Check if this render includes the level
16821682
// that previously suspended.
16831683
const baseTime = currentSuspenseState.baseTime;
1684-
if (
1685-
baseTime !== NoWork &&
1686-
baseTime < renderExpirationTime &&
1687-
baseTime > currentChildExpirationTime
1688-
) {
1684+
if (baseTime !== NoWork && baseTime < renderExpirationTime) {
16891685
// There's pending work at a lower level that might now be unblocked.
16901686
return baseTime;
16911687
}

packages/react-reconciler/src/ReactFiberBeginWork.old.js

+1-5
Original file line numberDiff line numberDiff line change
@@ -1681,11 +1681,7 @@ function getRemainingWorkInPrimaryTree(
16811681
// This boundary already timed out. Check if this render includes the level
16821682
// that previously suspended.
16831683
const baseTime = currentSuspenseState.baseTime;
1684-
if (
1685-
baseTime !== NoWork &&
1686-
baseTime < renderExpirationTime &&
1687-
baseTime > currentChildExpirationTime
1688-
) {
1684+
if (baseTime !== NoWork && baseTime < renderExpirationTime) {
16891685
// There's pending work at a lower level that might now be unblocked.
16901686
return baseTime;
16911687
}

packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js

+49
Original file line numberDiff line numberDiff line change
@@ -3869,4 +3869,53 @@ describe('ReactSuspenseWithNoopRenderer', () => {
38693869
expect(root).toMatchRenderedOutput(<span prop="b" />);
38703870
});
38713871
});
3872+
3873+
it('regression: #18657', async () => {
3874+
const {useState} = React;
3875+
3876+
let setText;
3877+
function App() {
3878+
const [text, _setText] = useState('A');
3879+
setText = _setText;
3880+
return <AsyncText text={text} />;
3881+
}
3882+
3883+
const root = ReactNoop.createRoot();
3884+
await ReactNoop.act(async () => {
3885+
await resolveText('A');
3886+
root.render(
3887+
<Suspense fallback={<Text text="Loading..." />}>
3888+
<App />
3889+
</Suspense>,
3890+
);
3891+
});
3892+
expect(Scheduler).toHaveYielded(['A']);
3893+
expect(root).toMatchRenderedOutput(<span prop="A" />);
3894+
3895+
await ReactNoop.act(async () => {
3896+
setText('B');
3897+
Scheduler.unstable_runWithPriority(
3898+
Scheduler.unstable_IdlePriority,
3899+
() => {
3900+
setText('B');
3901+
},
3902+
);
3903+
// Suspend the first update. The second update doesn't run because it has
3904+
// Idle priority.
3905+
expect(Scheduler).toFlushAndYield(['Suspend! [B]', 'Loading...']);
3906+
3907+
// Commit the fallback. Now we'll try working on Idle.
3908+
jest.runAllTimers();
3909+
3910+
// It also suspends.
3911+
expect(Scheduler).toFlushAndYield(['Suspend! [B]']);
3912+
});
3913+
3914+
await ReactNoop.act(async () => {
3915+
setText('B');
3916+
await resolveText('B');
3917+
});
3918+
expect(Scheduler).toHaveYielded(['Promise resolved [B]', 'B']);
3919+
expect(root).toMatchRenderedOutput(<span prop="B" />);
3920+
});
38723921
});

0 commit comments

Comments
 (0)