From a64c6bf1d085ef0d23d682f3b5d9547524d299b4 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 17 Apr 2024 12:28:25 -0400 Subject: [PATCH 1/2] Encode Iterator separately from Iterable in Flight Just giving it a special tag and indirect reference for this edge case. --- packages/react-client/src/ReactFlightClient.js | 16 ++++++++++++++++ .../src/__tests__/ReactFlight-test.js | 18 ++++++++++++++++++ packages/react-server/src/ReactFlightServer.js | 18 +++++++++++++++++- 3 files changed, 51 insertions(+), 1 deletion(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 1fb6ca6c2b0d1..fe50a309a8e40 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -824,6 +824,11 @@ function createFormData( return formData; } +function extractIterator(response: Response, model: Array): Iterator { + // $FlowFixMe[incompatible-use]: This uses raw Symbols because we're extracting from a native array. + return model[Symbol.iterator](); +} + function createModel(response: Response, model: any): any { return model; } @@ -918,6 +923,17 @@ function parseModelString( createFormData, ); } + case 'i': { + // Iterator + const id = parseInt(value.slice(2), 16); + return getOutlinedModel( + response, + id, + parentObject, + key, + extractIterator, + ); + } case 'I': { // $Infinity return Infinity; diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index 4b8fb5c3c43c9..b17273ddd01c2 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -277,6 +277,24 @@ describe('ReactFlight', () => { expect(ReactNoop).toMatchRenderedOutput(ABC); }); + it('can render an iterator as a single shot iterator', async () => { + const iterator = (function* () { + yield 'A'; + yield 'B'; + yield 'C'; + })(); + + const transport = ReactNoopFlightServer.render(iterator); + const result = await ReactNoopFlightClient.read(transport); + + // The iterator should be the same as itself. + expect(result[Symbol.iterator]()).toBe(result); + + expect(Array.from(result)).toEqual(['A', 'B', 'C']); + // We've already consumed this iterator. + expect(Array.from(result)).toEqual([]); + }); + it('can render undefined', async () => { function Undefined() { return undefined; diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 254e26e675bd0..c98bd1ad15a1b 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -242,7 +242,9 @@ export type ReactClientValue = | bigint | ReadableStream | $AsyncIterable + | $AsyncIterator | Iterable + | Iterator | Array | Map | Set @@ -1458,6 +1460,14 @@ function serializeSet(request: Request, set: Set): string { return '$W' + id.toString(16); } +function serializeIterator( + request: Request, + iterator: Iterator, +): string { + const id = outlineModel(request, Array.from(iterator)); + return '$i' + id.toString(16); +} + function serializeTypedArray( request: Request, tag: string, @@ -1911,7 +1921,13 @@ function renderModelDestructive( const iteratorFn = getIteratorFn(value); if (iteratorFn) { - return renderFragment(request, task, Array.from((value: any))); + // TODO: Should we serialize the return value as well like we do for AsyncIterables? + const iterator = iteratorFn.call(value); + if (iterator === value) { + // Iterator, not Iterable + return serializeIterator(request, (iterator: any)); + } + return renderFragment(request, task, Array.from((iterator: any))); } if (enableFlightReadableStream) { From 2f20c26883d2c957f83bcde388e995be457abf49 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 17 Apr 2024 12:28:34 -0400 Subject: [PATCH 2/2] Encode Iterator separately from Iterable in Flight Reply --- .../src/ReactFlightReplyClient.js | 23 ++++++++++++++- .../src/__tests__/ReactFlightDOMReply-test.js | 29 +++++++++++++++++++ .../src/ReactFlightReplyServer.js | 6 ++++ 3 files changed, 57 insertions(+), 1 deletion(-) diff --git a/packages/react-client/src/ReactFlightReplyClient.js b/packages/react-client/src/ReactFlightReplyClient.js index d90a3a509b0b2..eb562dd7b23c9 100644 --- a/packages/react-client/src/ReactFlightReplyClient.js +++ b/packages/react-client/src/ReactFlightReplyClient.js @@ -81,7 +81,10 @@ export type ReactServerValue = | null | void | bigint + | $AsyncIterable + | $AsyncIterator | Iterable + | Iterator | Array | Map | Set @@ -157,6 +160,10 @@ function serializeBlobID(id: number): string { return '$B' + id.toString(16); } +function serializeIteratorID(id: number): string { + return '$i' + id.toString(16); +} + function escapeStringValue(value: string): string { if (value[0] === '$') { // We need to escape $ prefixed strings since we use those to encode @@ -448,7 +455,21 @@ export function processReply( const iteratorFn = getIteratorFn(value); if (iteratorFn) { - return Array.from((value: any)); + const iterator = iteratorFn.call(value); + if (iterator === value) { + // Iterator, not Iterable + const partJSON = JSON.stringify( + Array.from((iterator: any)), + resolveToJSON, + ); + if (formData === null) { + formData = new FormData(); + } + const iteratorId = nextPartId++; + formData.append(formFieldPrefix + iteratorId, partJSON); + return serializeIteratorID(iteratorId); + } + return Array.from((iterator: any)); } // Verify that this is a simple plain object. diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMReply-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMReply-test.js index 48092c97a4485..62948f275298c 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMReply-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMReply-test.js @@ -97,6 +97,35 @@ describe('ReactFlightDOMReply', () => { items.push(item); } expect(items).toEqual(['A', 'B', 'C']); + + // Multipass + const items2 = []; + // eslint-disable-next-line no-for-of-loops/no-for-of-loops + for (const item of iterable) { + items2.push(item); + } + expect(items2).toEqual(['A', 'B', 'C']); + }); + + it('can pass an iterator as a reply', async () => { + const iterator = (function* () { + yield 'A'; + yield 'B'; + yield 'C'; + })(); + + const body = await ReactServerDOMClient.encodeReply(iterator); + const result = await ReactServerDOMServer.decodeReply( + body, + webpackServerMap, + ); + + // The iterator should be the same as itself. + expect(result[Symbol.iterator]()).toBe(result); + + expect(Array.from(result)).toEqual(['A', 'B', 'C']); + // We've already consumed this iterator. + expect(Array.from(result)).toEqual([]); }); it('can pass weird numbers as a reply', async () => { diff --git a/packages/react-server/src/ReactFlightReplyServer.js b/packages/react-server/src/ReactFlightReplyServer.js index e1933b17db4c3..4a447a5ee18da 100644 --- a/packages/react-server/src/ReactFlightReplyServer.js +++ b/packages/react-server/src/ReactFlightReplyServer.js @@ -477,6 +477,12 @@ function parseModelString( }); return data; } + case 'i': { + // Iterator + const id = parseInt(value.slice(2), 16); + const data = getOutlinedModel(response, id); + return data[Symbol.iterator](); + } case 'I': { // $Infinity return Infinity;