Skip to content
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

[Flight / Flight Reply] Encode Iterator separately from Iterable #28854

Merged
merged 2 commits into from
Apr 21, 2024
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
16 changes: 16 additions & 0 deletions packages/react-client/src/ReactFlightClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -824,6 +824,11 @@ function createFormData(
return formData;
}

function extractIterator(response: Response, model: Array<any>): Iterator<any> {
// $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;
}
Expand Down Expand Up @@ -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;
Expand Down
23 changes: 22 additions & 1 deletion packages/react-client/src/ReactFlightReplyClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,10 @@ export type ReactServerValue =
| null
| void
| bigint
| $AsyncIterable<ReactServerValue, ReactServerValue, void>
| $AsyncIterator<ReactServerValue, ReactServerValue, void>
| Iterable<ReactServerValue>
| Iterator<ReactServerValue>
| Array<ReactServerValue>
| Map<ReactServerValue, ReactServerValue>
| Set<ReactServerValue>
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
18 changes: 18 additions & 0 deletions packages/react-client/src/__tests__/ReactFlight-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,24 @@ describe('ReactFlight', () => {
expect(ReactNoop).toMatchRenderedOutput(<span>ABC</span>);
});

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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
6 changes: 6 additions & 0 deletions packages/react-server/src/ReactFlightReplyServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
18 changes: 17 additions & 1 deletion packages/react-server/src/ReactFlightServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,9 @@ export type ReactClientValue =
| bigint
| ReadableStream
| $AsyncIterable<ReactClientValue, ReactClientValue, void>
| $AsyncIterator<ReactClientValue, ReactClientValue, void>
| Iterable<ReactClientValue>
| Iterator<ReactClientValue>
| Array<ReactClientValue>
| Map<ReactClientValue, ReactClientValue>
| Set<ReactClientValue>
Expand Down Expand Up @@ -1458,6 +1460,14 @@ function serializeSet(request: Request, set: Set<ReactClientValue>): string {
return '$W' + id.toString(16);
}

function serializeIterator(
request: Request,
iterator: Iterator<ReactClientValue>,
): string {
const id = outlineModel(request, Array.from(iterator));
return '$i' + id.toString(16);
}

function serializeTypedArray(
request: Request,
tag: string,
Expand Down Expand Up @@ -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?
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kind of annoying because it would require more code. We could potentially just not support it in the AsyncIterable case too.

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) {
Expand Down