Skip to content

Commit 3926e24

Browse files
authored
Fix ViewTransition null stateNode with SuspenseList (facebook#35520)
I was experimenting with animations in SuspenseList and hit a crash using ViewTransition as a direct child with `revealOrder="together"` ``` TypeError: Cannot read properties of null (reading 'autoName') 33 | return props.name; 34 | } > 35 | if (instance.autoName !== null) { | ^ 36 | return instance.autoName; 37 | } ``` When ViewTransition is direct child of SuspenseList, the second render pass calls resetChildFibers, setting stateNode to null. Other fibers create stateNode in completeWork. ViewTransition does not, so stateNode is lost. Followed the pattern used for Offscreen to update stateNode in beginWork if it is null. Also added a regression test.
1 parent 6baff7a commit 3926e24

File tree

2 files changed

+192
-0
lines changed

2 files changed

+192
-0
lines changed
Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
/**
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @emails react-core
8+
*/
9+
10+
'use strict';
11+
12+
let React;
13+
let ReactDOMClient;
14+
let Suspense;
15+
let SuspenseList;
16+
let ViewTransition;
17+
let act;
18+
let assertLog;
19+
let Scheduler;
20+
let textCache;
21+
22+
describe('ReactDOMViewTransition', () => {
23+
let container;
24+
25+
beforeEach(() => {
26+
jest.resetModules();
27+
React = require('react');
28+
ReactDOMClient = require('react-dom/client');
29+
Scheduler = require('scheduler');
30+
act = require('internal-test-utils').act;
31+
assertLog = require('internal-test-utils').assertLog;
32+
Suspense = React.Suspense;
33+
ViewTransition = React.ViewTransition;
34+
if (gate(flags => flags.enableSuspenseList)) {
35+
SuspenseList = React.unstable_SuspenseList;
36+
}
37+
container = document.createElement('div');
38+
document.body.appendChild(container);
39+
40+
textCache = new Map();
41+
});
42+
43+
afterEach(() => {
44+
document.body.removeChild(container);
45+
});
46+
47+
function resolveText(text) {
48+
const record = textCache.get(text);
49+
if (record === undefined) {
50+
const newRecord = {
51+
status: 'resolved',
52+
value: text,
53+
};
54+
textCache.set(text, newRecord);
55+
} else if (record.status === 'pending') {
56+
const thenable = record.value;
57+
record.status = 'resolved';
58+
record.value = text;
59+
thenable.pings.forEach(t => t());
60+
}
61+
}
62+
63+
function readText(text) {
64+
const record = textCache.get(text);
65+
if (record !== undefined) {
66+
switch (record.status) {
67+
case 'pending':
68+
Scheduler.log(`Suspend! [${text}]`);
69+
throw record.value;
70+
case 'rejected':
71+
throw record.value;
72+
case 'resolved':
73+
return record.value;
74+
}
75+
} else {
76+
Scheduler.log(`Suspend! [${text}]`);
77+
const thenable = {
78+
pings: [],
79+
then(resolve) {
80+
if (newRecord.status === 'pending') {
81+
thenable.pings.push(resolve);
82+
} else {
83+
Promise.resolve().then(() => resolve(newRecord.value));
84+
}
85+
},
86+
};
87+
88+
const newRecord = {
89+
status: 'pending',
90+
value: thenable,
91+
};
92+
textCache.set(text, newRecord);
93+
94+
throw thenable;
95+
}
96+
}
97+
98+
function Text({text}) {
99+
Scheduler.log(text);
100+
return text;
101+
}
102+
103+
function AsyncText({text}) {
104+
readText(text);
105+
Scheduler.log(text);
106+
return text;
107+
}
108+
109+
// @gate enableSuspenseList
110+
it('handles ViewTransition wrapping Suspense inside SuspenseList', async () => {
111+
function Card({id}) {
112+
return (
113+
<div>
114+
<AsyncText text={`Card ${id}`} />
115+
</div>
116+
);
117+
}
118+
119+
function CardSkeleton({n}) {
120+
return <Text text={`Skeleton ${n}`} />;
121+
}
122+
123+
function App() {
124+
return (
125+
<div>
126+
<SuspenseList revealOrder="together">
127+
<ViewTransition>
128+
<Suspense fallback={<CardSkeleton n={1} />}>
129+
<Card id={1} />
130+
</Suspense>
131+
</ViewTransition>
132+
<ViewTransition>
133+
<Suspense fallback={<CardSkeleton n={2} />}>
134+
<Card id={2} />
135+
</Suspense>
136+
</ViewTransition>
137+
<ViewTransition>
138+
<Suspense fallback={<CardSkeleton n={3} />}>
139+
<Card id={3} />
140+
</Suspense>
141+
</ViewTransition>
142+
</SuspenseList>
143+
</div>
144+
);
145+
}
146+
147+
const root = ReactDOMClient.createRoot(container);
148+
149+
// Initial render - all cards should suspend
150+
await act(() => {
151+
root.render(<App />);
152+
});
153+
154+
assertLog([
155+
'Suspend! [Card 1]',
156+
'Skeleton 1',
157+
'Suspend! [Card 2]',
158+
'Skeleton 2',
159+
'Suspend! [Card 3]',
160+
'Skeleton 3',
161+
'Skeleton 1',
162+
'Skeleton 2',
163+
'Skeleton 3',
164+
]);
165+
166+
await act(() => {
167+
resolveText('Card 1');
168+
resolveText('Card 2');
169+
resolveText('Card 3');
170+
});
171+
172+
assertLog(['Card 1', 'Card 2', 'Card 3']);
173+
174+
// All cards should be visible
175+
expect(container.textContent).toContain('Card 1');
176+
expect(container.textContent).toContain('Card 2');
177+
expect(container.textContent).toContain('Card 3');
178+
});
179+
});

packages/react-reconciler/src/ReactFiberBeginWork.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import type {
4646
import type {UpdateQueue} from './ReactFiberClassUpdateQueue';
4747
import type {RootState} from './ReactFiberRoot';
4848
import type {TracingMarkerInstance} from './ReactFiberTracingMarkerComponent';
49+
import type {ViewTransitionState} from './ReactFiberViewTransitionComponent';
4950

5051
import {
5152
markComponentRenderStarted,
@@ -3566,6 +3567,18 @@ function updateViewTransition(
35663567
workInProgress: Fiber,
35673568
renderLanes: Lanes,
35683569
) {
3570+
if (workInProgress.stateNode === null) {
3571+
// We previously reset the work-in-progress.
3572+
// We need to create a new ViewTransitionState instance.
3573+
const instance: ViewTransitionState = {
3574+
autoName: null,
3575+
paired: null,
3576+
clones: null,
3577+
ref: null,
3578+
};
3579+
workInProgress.stateNode = instance;
3580+
}
3581+
35693582
const pendingProps: ViewTransitionProps = workInProgress.pendingProps;
35703583
if (pendingProps.name != null && pendingProps.name !== 'auto') {
35713584
// Explicitly named boundary. We track it so that we can pair it up with another explicit

0 commit comments

Comments
 (0)