Skip to content

[SuspenseList] Don't treat the last row in hidden as deleted if already mounted #17206

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions packages/react-reconciler/src/ReactFiberCompleteWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -1098,10 +1098,11 @@ function completeWork(
// This might have been modified.
if (
renderState.tail === null &&
renderState.tailMode === 'hidden'
renderState.tailMode === 'hidden' &&
!renderedTail.alternate
) {
// We need to delete the row we just rendered.
// Reset the effect list to what it w as before we rendered this
// Reset the effect list to what it was before we rendered this
// child. The nested children have already appended themselves.
let lastEffect = (workInProgress.lastEffect =
renderState.lastEffect);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2023,7 +2023,7 @@ describe('ReactSuspenseList', () => {
);
});

it('eventually resolves two nested forwards suspense list with a hidden tail', async () => {
it('eventually resolves two nested forwards suspense lists with a hidden tail', async () => {
let B = createAsyncText('B');

function Foo({showB}) {
Expand Down Expand Up @@ -2135,4 +2135,92 @@ describe('ReactSuspenseList', () => {
</div>,
);
});

it('is able to re-suspend the last rows during an update with hidden', async () => {
let AsyncB = createAsyncText('B');

let setAsyncB;

function B() {
let [shouldBeAsync, setAsync] = React.useState(false);
setAsyncB = setAsync;

return shouldBeAsync ? (
<Suspense fallback={<Text text="Loading B" />}>
<AsyncB />
</Suspense>
) : (
<Text text="Sync B" />
);
}

function Foo({updateList}) {
return (
<SuspenseList revealOrder="forwards" tail="hidden">
<Suspense key="A" fallback={<Text text="Loading A" />}>
<Text text="A" />
</Suspense>
<B key="B" updateList={updateList} />
</SuspenseList>
);
}

ReactNoop.render(<Foo />);

expect(Scheduler).toFlushAndYield(['A', 'Sync B']);

expect(ReactNoop).toMatchRenderedOutput(
<>
<span>A</span>
<span>Sync B</span>
</>,
);

let previousInst = setAsyncB;

// During an update we suspend on B.
ReactNoop.act(() => setAsyncB(true));

expect(Scheduler).toHaveYielded([
'Suspend! [B]',
'Loading B',
// The second pass is the "force hide" pass
'Loading B',
]);

expect(ReactNoop).toMatchRenderedOutput(
<>
<span>A</span>
<span>Loading B</span>
</>,
);

// Before we resolve we'll rerender the whole list.
// This should leave the tree intact.
ReactNoop.act(() => ReactNoop.render(<Foo updateList={true} />));

expect(Scheduler).toHaveYielded(['A', 'Suspend! [B]', 'Loading B']);

expect(ReactNoop).toMatchRenderedOutput(
<>
<span>A</span>
<span>Loading B</span>
</>,
);

await AsyncB.resolve();

expect(Scheduler).toFlushAndYield(['B']);

expect(ReactNoop).toMatchRenderedOutput(
<>
<span>A</span>
<span>B</span>
</>,
);

// This should be the same instance. I.e. it didn't
// remount.
expect(previousInst).toBe(setAsyncB);
});
});