Skip to content

Commit 5b889b4

Browse files
committed
Fix ref counting bug and retry of thrown promises
1 parent 07edd52 commit 5b889b4

File tree

2 files changed

+56
-23
lines changed

2 files changed

+56
-23
lines changed

packages/react-client/src/ReactFlightReplyClient.js

+19-22
Original file line numberDiff line numberDiff line change
@@ -238,18 +238,17 @@ export function processReply(
238238
// Upgrade to use FormData to allow us to stream this value.
239239
formData = new FormData();
240240
}
241+
pendingParts++;
241242
try {
242243
const resolvedModel = init(payload);
243244
// We always outline this as a separate part even though we could inline it
244245
// because it ensures a more deterministic encoding.
245-
pendingParts++;
246246
const lazyId = nextPartId++;
247247
const partJSON = JSON.stringify(resolvedModel, resolveToJSON);
248248
// $FlowFixMe[incompatible-type] We know it's not null because we assigned it above.
249249
const data: FormData = formData;
250250
// eslint-disable-next-line react-internal/safe-string-coercion
251251
data.append(formFieldPrefix + lazyId, partJSON);
252-
pendingParts--;
253252
return serializeByValueID(lazyId);
254253
} catch (x) {
255254
if (
@@ -261,35 +260,33 @@ export function processReply(
261260
pendingParts++;
262261
const lazyId = nextPartId++;
263262
const thenable: Thenable<any> = (x: any);
264-
thenable.then(
265-
partValue => {
266-
try {
267-
const partJSON = JSON.stringify(partValue, resolveToJSON);
268-
// $FlowFixMe[incompatible-type] We know it's not null because we assigned it above.
269-
const data: FormData = formData;
270-
// eslint-disable-next-line react-internal/safe-string-coercion
271-
data.append(formFieldPrefix + lazyId, partJSON);
272-
pendingParts--;
273-
if (pendingParts === 0) {
274-
resolve(data);
275-
}
276-
} catch (reason) {
277-
reject(reason);
263+
const retry = function() {
264+
// While the first promise resolved, its value isn't necessarily what we'll
265+
// resolve into because we might suspend again.
266+
try {
267+
const partJSON = JSON.stringify(value, resolveToJSON);
268+
// $FlowFixMe[incompatible-type] We know it's not null because we assigned it above.
269+
const data: FormData = formData;
270+
// eslint-disable-next-line react-internal/safe-string-coercion
271+
data.append(formFieldPrefix + lazyId, partJSON);
272+
pendingParts--;
273+
if (pendingParts === 0) {
274+
resolve(data);
278275
}
279-
},
280-
reason => {
281-
// In the future we could consider serializing this as an error
282-
// that throws on the server instead.
276+
} catch (reason) {
283277
reject(reason);
284-
},
285-
);
278+
}
279+
}
280+
thenable.then(retry, retry);
286281
return serializeByValueID(lazyId);
287282
} else {
288283
// In the future we could consider serializing this as an error
289284
// that throws on the server instead.
290285
reject(x);
291286
return null;
292287
}
288+
} finally {
289+
pendingParts--;
293290
}
294291
}
295292
}

packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMReply-test.js

+37-1
Original file line numberDiff line numberDiff line change
@@ -257,11 +257,47 @@ describe('ReactFlightDOMReply', () => {
257257
let resolve;
258258
const lazy = React.lazy(() => new Promise(r => (resolve = r)));
259259
const bodyPromise = ReactServerDOMClient.encodeReply({lazy: lazy});
260-
resolve('Hi');
260+
resolve({default: 'Hi'});
261261
const result = await ReactServerDOMServer.decodeReply(await bodyPromise);
262262
expect(result.lazy).toBe('Hi');
263263
});
264264

265+
it('resolves a proxy throwing a promise inside React.lazy', async () => {
266+
let resolve1;
267+
let resolve2;
268+
const lazy = React.lazy(() => new Promise(r => (resolve1 = r)));
269+
const promise = new Promise(r => (resolve2 = r));
270+
const bodyPromise1 = ReactServerDOMClient.encodeReply({lazy: lazy});
271+
const target = {value: ''};
272+
let loaded = false;
273+
const proxy = new Proxy(target, {
274+
get(targetObj, prop, receiver) {
275+
if (prop === 'value') {
276+
if (!loaded) {
277+
throw promise;
278+
}
279+
return 'Hello';
280+
}
281+
return targetObj[prop];
282+
},
283+
});
284+
await resolve1({default: proxy});
285+
286+
// Encode it again so that we have an already initialized lazy
287+
// This is now already resolved but the proxy inside isn't. This ensures
288+
// we trigger the retry code path.
289+
const bodyPromise2 = ReactServerDOMClient.encodeReply({lazy: lazy});
290+
291+
// Then resolve the inner thrown promise.
292+
loaded = true;
293+
await resolve2('Hello');
294+
295+
const result1 = await ReactServerDOMServer.decodeReply(await bodyPromise1);
296+
expect(await result1.lazy.value).toBe('Hello');
297+
const result2 = await ReactServerDOMServer.decodeReply(await bodyPromise2);
298+
expect(await result2.lazy.value).toBe('Hello');
299+
});
300+
265301
it('errors when called with JSX by default', async () => {
266302
let error;
267303
try {

0 commit comments

Comments
 (0)