Skip to content

Commit adb0bef

Browse files
committed
Suspend Thenable/Lazy if it's used in React.Children and unwrap
1 parent ba5e6a8 commit adb0bef

File tree

3 files changed

+124
-38
lines changed

3 files changed

+124
-38
lines changed

packages/react-reconciler/src/ReactFiberThenable.js

+12-12
Original file line numberDiff line numberDiff line change
@@ -212,19 +212,19 @@ export function trackUsedThenable<T>(
212212
}
213213
},
214214
);
215+
}
215216

216-
// Check one more time in case the thenable resolved synchronously.
217-
switch (thenable.status) {
218-
case 'fulfilled': {
219-
const fulfilledThenable: FulfilledThenable<T> = (thenable: any);
220-
return fulfilledThenable.value;
221-
}
222-
case 'rejected': {
223-
const rejectedThenable: RejectedThenable<T> = (thenable: any);
224-
const rejectedError = rejectedThenable.reason;
225-
checkIfUseWrappedInAsyncCatch(rejectedError);
226-
throw rejectedError;
227-
}
217+
// Check one more time in case the thenable resolved synchronously.
218+
switch (thenable.status) {
219+
case 'fulfilled': {
220+
const fulfilledThenable: FulfilledThenable<T> = (thenable: any);
221+
return fulfilledThenable.value;
222+
}
223+
case 'rejected': {
224+
const rejectedThenable: RejectedThenable<T> = (thenable: any);
225+
const rejectedError = rejectedThenable.reason;
226+
checkIfUseWrappedInAsyncCatch(rejectedError);
227+
throw rejectedError;
228228
}
229229
}
230230

packages/react/src/ReactChildren.js

+86-10
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,13 @@
77
* @flow
88
*/
99

10-
import type {ReactNodeList} from 'shared/ReactTypes';
10+
import type {
11+
ReactNodeList,
12+
Thenable,
13+
PendingThenable,
14+
FulfilledThenable,
15+
RejectedThenable,
16+
} from 'shared/ReactTypes';
1117

1218
import isArray from 'shared/isArray';
1319
import {
@@ -75,6 +81,68 @@ function getElementKey(element: any, index: number): string {
7581
return index.toString(36);
7682
}
7783

84+
function noop() {}
85+
86+
function resolveThenable<T>(thenable: Thenable<T>): T {
87+
switch (thenable.status) {
88+
case 'fulfilled': {
89+
const fulfilledValue: T = thenable.value;
90+
return fulfilledValue;
91+
}
92+
case 'rejected': {
93+
const rejectedError = thenable.reason;
94+
throw rejectedError;
95+
}
96+
default: {
97+
if (typeof thenable.status === 'string') {
98+
// Only instrument the thenable if the status if not defined. If
99+
// it's defined, but an unknown value, assume it's been instrumented by
100+
// some custom userspace implementation. We treat it as "pending".
101+
// Attach a dummy listener, to ensure that any lazy initialization can
102+
// happen. Flight lazily parses JSON when the value is actually awaited.
103+
thenable.then(noop, noop);
104+
} else {
105+
// This is an uncached thenable that we haven't seen before.
106+
107+
// TODO: Detect infinite ping loops caused by uncached promises.
108+
109+
const pendingThenable: PendingThenable<T> = (thenable: any);
110+
pendingThenable.status = 'pending';
111+
pendingThenable.then(
112+
fulfilledValue => {
113+
if (thenable.status === 'pending') {
114+
const fulfilledThenable: FulfilledThenable<T> = (thenable: any);
115+
fulfilledThenable.status = 'fulfilled';
116+
fulfilledThenable.value = fulfilledValue;
117+
}
118+
},
119+
(error: mixed) => {
120+
if (thenable.status === 'pending') {
121+
const rejectedThenable: RejectedThenable<T> = (thenable: any);
122+
rejectedThenable.status = 'rejected';
123+
rejectedThenable.reason = error;
124+
}
125+
},
126+
);
127+
}
128+
129+
// Check one more time in case the thenable resolved synchronously.
130+
switch (thenable.status) {
131+
case 'fulfilled': {
132+
const fulfilledThenable: FulfilledThenable<T> = (thenable: any);
133+
return fulfilledThenable.value;
134+
}
135+
case 'rejected': {
136+
const rejectedThenable: RejectedThenable<T> = (thenable: any);
137+
const rejectedError = rejectedThenable.reason;
138+
throw rejectedError;
139+
}
140+
}
141+
}
142+
}
143+
throw thenable;
144+
}
145+
78146
function mapIntoArray(
79147
children: ?ReactNodeList,
80148
array: Array<React$Node>,
@@ -106,9 +174,14 @@ function mapIntoArray(
106174
invokeCallback = true;
107175
break;
108176
case REACT_LAZY_TYPE:
109-
throw new Error(
110-
'Cannot render an Async Component, Promise or React.Lazy inside React.Children. ' +
111-
'We recommend not iterating over children and just rendering them plain.',
177+
const payload = (children: any)._payload;
178+
const init = (children: any)._init;
179+
return mapIntoArray(
180+
init(payload),
181+
array,
182+
escapedPrefix,
183+
nameSoFar,
184+
callback,
112185
);
113186
}
114187
}
@@ -211,16 +284,19 @@ function mapIntoArray(
211284
);
212285
}
213286
} else if (type === 'object') {
214-
// eslint-disable-next-line react-internal/safe-string-coercion
215-
const childrenString = String((children: any));
216-
217287
if (typeof (children: any).then === 'function') {
218-
throw new Error(
219-
'Cannot render an Async Component, Promise or React.Lazy inside React.Children. ' +
220-
'We recommend not iterating over children and just rendering them plain.',
288+
return mapIntoArray(
289+
resolveThenable((children: any)),
290+
array,
291+
escapedPrefix,
292+
nameSoFar,
293+
callback,
221294
);
222295
}
223296

297+
// eslint-disable-next-line react-internal/safe-string-coercion
298+
const childrenString = String((children: any));
299+
224300
throw new Error(
225301
`Objects are not valid as a React child (found: ${
226302
childrenString === '[object Object]'

packages/react/src/__tests__/ReactChildren-test.js

+26-16
Original file line numberDiff line numberDiff line change
@@ -948,26 +948,36 @@ describe('ReactChildren', () => {
948948
);
949949
});
950950

951-
it('should throw on React.lazy', async () => {
951+
it('should render React.lazy after suspending', async () => {
952952
const lazyElement = React.lazy(async () => ({default: <div />}));
953-
await expect(() => {
954-
React.Children.forEach([lazyElement], () => {}, null);
955-
}).toThrowError(
956-
'Cannot render an Async Component, Promise or React.Lazy inside React.Children. ' +
957-
'We recommend not iterating over children and just rendering them plain.',
958-
{withoutStack: true}, // There's nothing on the stack
959-
);
953+
function Component() {
954+
return React.Children.map([lazyElement], c =>
955+
React.cloneElement(c, {children: 'hi'}),
956+
);
957+
}
958+
const container = document.createElement('div');
959+
const root = ReactDOMClient.createRoot(container);
960+
await act(() => {
961+
root.render(<Component />);
962+
});
963+
964+
expect(container.innerHTML).toBe('<div>hi</div>');
960965
});
961966

962-
it('should throw on Promises', async () => {
967+
it('should render cached Promises after suspending', async () => {
963968
const promise = Promise.resolve(<div />);
964-
await expect(() => {
965-
React.Children.forEach([promise], () => {}, null);
966-
}).toThrowError(
967-
'Cannot render an Async Component, Promise or React.Lazy inside React.Children. ' +
968-
'We recommend not iterating over children and just rendering them plain.',
969-
{withoutStack: true}, // There's nothing on the stack
970-
);
969+
function Component() {
970+
return React.Children.map([promise], c =>
971+
React.cloneElement(c, {children: 'hi'}),
972+
);
973+
}
974+
const container = document.createElement('div');
975+
const root = ReactDOMClient.createRoot(container);
976+
await act(() => {
977+
root.render(<Component />);
978+
});
979+
980+
expect(container.innerHTML).toBe('<div>hi</div>');
971981
});
972982

973983
it('should throw on regex', () => {

0 commit comments

Comments
 (0)