From 14f50ad1554f0adf20fa1b5bc62859ed32be0bc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 8 Apr 2024 15:40:11 -0400 Subject: [PATCH] [Flight] Allow lazily resolving outlined models (#28780) We used to assume that outlined models are emitted before the reference (which was true before Blobs). However, it still wasn't safe to assume that all the data will be available because an "import" (client reference) can be async and therefore if it's directly a child of an outlined model, it won't be able to update in place. This is a similar problem as the one hit by @unstubbable in #28669 with elements, but a little different since these don't follow the same way of wrapping. I don't love the structuring of this code which now needs to pass a first class mapper instead of just being known code. It also shares the host path which is just an identity function. It wouldn't necessarily pass my own review but I don't have a better one for now. I'd really prefer if this was done at a "row" level but that ends up creating even more code. Add test for Blob in FormData and async modules in Maps. --- .../react-client/src/ReactFlightClient.js | 183 ++++++++++-------- .../src/__tests__/ReactFlightDOMEdge-test.js | 78 ++++++++ .../react-server/src/ReactFlightServer.js | 27 ++- 3 files changed, 197 insertions(+), 91 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 446c529c67ba2..840b49fceae16 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -581,6 +581,8 @@ function createModelResolver( parentObject: Object, key: string, cyclic: boolean, + response: Response, + map: (response: Response, model: any) => T, ): (value: any) => void { let blocked; if (initializingChunkBlockedModel) { @@ -595,12 +597,12 @@ function createModelResolver( }; } return value => { - parentObject[key] = value; + parentObject[key] = map(response, value); // If this is the root object for a model reference, where `blocked.value` // is a stale `null`, the resolved value can be used directly. if (key === '' && blocked.value === null) { - blocked.value = value; + blocked.value = parentObject[key]; } blocked.deps--; @@ -651,24 +653,103 @@ function createServerReferenceProxy, T>( return proxy; } -function getOutlinedModel(response: Response, id: number): any { +function getOutlinedModel( + response: Response, + id: number, + parentObject: Object, + key: string, + map: (response: Response, model: any) => T, +): T { const chunk = getChunk(response, id); switch (chunk.status) { case RESOLVED_MODEL: initializeModelChunk(chunk); break; + case RESOLVED_MODULE: + initializeModuleChunk(chunk); + break; } // The status might have changed after initialization. switch (chunk.status) { - case INITIALIZED: { - return chunk.value; - } - // We always encode it first in the stream so it won't be pending. + case INITIALIZED: + const chunkValue = map(response, chunk.value); + if (__DEV__ && chunk._debugInfo) { + // If we have a direct reference to an object that was rendered by a synchronous + // server component, it might have some debug info about how it was rendered. + // We forward this to the underlying object. This might be a React Element or + // an Array fragment. + // If this was a string / number return value we lose the debug info. We choose + // that tradeoff to allow sync server components to return plain values and not + // use them as React Nodes necessarily. We could otherwise wrap them in a Lazy. + if ( + typeof chunkValue === 'object' && + chunkValue !== null && + (Array.isArray(chunkValue) || + chunkValue.$$typeof === REACT_ELEMENT_TYPE) && + !chunkValue._debugInfo + ) { + // We should maybe use a unique symbol for arrays but this is a React owned array. + // $FlowFixMe[prop-missing]: This should be added to elements. + Object.defineProperty((chunkValue: any), '_debugInfo', { + configurable: false, + enumerable: false, + writable: true, + value: chunk._debugInfo, + }); + } + } + return chunkValue; + case PENDING: + case BLOCKED: + case CYCLIC: + const parentChunk = initializingChunk; + chunk.then( + createModelResolver( + parentChunk, + parentObject, + key, + chunk.status === CYCLIC, + response, + map, + ), + createModelReject(parentChunk), + ); + return (null: any); default: throw chunk.reason; } } +function createMap( + response: Response, + model: Array<[any, any]>, +): Map { + return new Map(model); +} + +function createSet(response: Response, model: Array): Set { + return new Set(model); +} + +function createBlob(response: Response, model: Array): Blob { + return new Blob(model.slice(1), {type: model[0]}); +} + +function createFormData( + response: Response, + model: Array<[any, any]>, +): FormData { + const formData = new FormData(); + for (let i = 0; i < model.length; i++) { + formData.append(model[i][0], model[i][1]); + } + return formData; +} + +function createModel(response: Response, model: any): any { + return model; +} + function parseModelString( response: Response, parentObject: Object, @@ -710,8 +791,13 @@ function parseModelString( case 'F': { // Server Reference const id = parseInt(value.slice(2), 16); - const metadata = getOutlinedModel(response, id); - return createServerReferenceProxy(response, metadata); + return getOutlinedModel( + response, + id, + parentObject, + key, + createServerReferenceProxy, + ); } case 'T': { // Temporary Reference @@ -728,33 +814,31 @@ function parseModelString( case 'Q': { // Map const id = parseInt(value.slice(2), 16); - const data = getOutlinedModel(response, id); - return new Map(data); + return getOutlinedModel(response, id, parentObject, key, createMap); } case 'W': { // Set const id = parseInt(value.slice(2), 16); - const data = getOutlinedModel(response, id); - return new Set(data); + return getOutlinedModel(response, id, parentObject, key, createSet); } case 'B': { // Blob if (enableBinaryFlight) { const id = parseInt(value.slice(2), 16); - const data = getOutlinedModel(response, id); - return new Blob(data.slice(1), {type: data[0]}); + return getOutlinedModel(response, id, parentObject, key, createBlob); } return undefined; } case 'K': { // FormData const id = parseInt(value.slice(2), 16); - const data = getOutlinedModel(response, id); - const formData = new FormData(); - for (let i = 0; i < data.length; i++) { - formData.append(data[i][0], data[i][1]); - } - return formData; + return getOutlinedModel( + response, + id, + parentObject, + key, + createFormData, + ); } case 'I': { // $Infinity @@ -803,62 +887,7 @@ function parseModelString( default: { // We assume that anything else is a reference ID. const id = parseInt(value.slice(1), 16); - const chunk = getChunk(response, id); - switch (chunk.status) { - case RESOLVED_MODEL: - initializeModelChunk(chunk); - break; - case RESOLVED_MODULE: - initializeModuleChunk(chunk); - break; - } - // The status might have changed after initialization. - switch (chunk.status) { - case INITIALIZED: - const chunkValue = chunk.value; - if (__DEV__ && chunk._debugInfo) { - // If we have a direct reference to an object that was rendered by a synchronous - // server component, it might have some debug info about how it was rendered. - // We forward this to the underlying object. This might be a React Element or - // an Array fragment. - // If this was a string / number return value we lose the debug info. We choose - // that tradeoff to allow sync server components to return plain values and not - // use them as React Nodes necessarily. We could otherwise wrap them in a Lazy. - if ( - typeof chunkValue === 'object' && - chunkValue !== null && - (Array.isArray(chunkValue) || - chunkValue.$$typeof === REACT_ELEMENT_TYPE) && - !chunkValue._debugInfo - ) { - // We should maybe use a unique symbol for arrays but this is a React owned array. - // $FlowFixMe[prop-missing]: This should be added to elements. - Object.defineProperty(chunkValue, '_debugInfo', { - configurable: false, - enumerable: false, - writable: true, - value: chunk._debugInfo, - }); - } - } - return chunkValue; - case PENDING: - case BLOCKED: - case CYCLIC: - const parentChunk = initializingChunk; - chunk.then( - createModelResolver( - parentChunk, - parentObject, - key, - chunk.status === CYCLIC, - ), - createModelReject(parentChunk), - ); - return null; - default: - throw chunk.reason; - } + return getOutlinedModel(response, id, parentObject, key, createModel); } } } diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js index ada7bb35cae27..e7b4e06f49f87 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js @@ -18,6 +18,9 @@ global.TextDecoder = require('util').TextDecoder; if (typeof Blob === 'undefined') { global.Blob = require('buffer').Blob; } +if (typeof File === 'undefined') { + global.File = require('buffer').File; +} // Don't wait before processing work on the server. // TODO: we can replace this with FlightServer.act(). @@ -352,6 +355,81 @@ describe('ReactFlightDOMEdge', () => { expect(await result.arrayBuffer()).toEqual(await blob.arrayBuffer()); }); + if (typeof FormData !== 'undefined' && typeof File !== 'undefined') { + // @gate enableBinaryFlight + it('can transport FormData (blobs)', async () => { + const bytes = new Uint8Array([ + 123, 4, 10, 5, 100, 255, 244, 45, 56, 67, 43, 124, 67, 89, 100, 20, + ]); + const blob = new Blob([bytes, bytes], { + type: 'application/x-test', + }); + + const formData = new FormData(); + formData.append('hi', 'world'); + formData.append('file', blob, 'filename.test'); + + expect(formData.get('file') instanceof File).toBe(true); + expect(formData.get('file').name).toBe('filename.test'); + + const stream = passThrough( + ReactServerDOMServer.renderToReadableStream(formData), + ); + const result = await ReactServerDOMClient.createFromReadableStream( + stream, + { + ssrManifest: { + moduleMap: null, + moduleLoading: null, + }, + }, + ); + + expect(result instanceof FormData).toBe(true); + expect(result.get('hi')).toBe('world'); + const resultBlob = result.get('file'); + expect(resultBlob instanceof Blob).toBe(true); + expect(resultBlob.name).toBe('blob'); // We should not pass through the file name for security. + expect(resultBlob.size).toBe(bytes.length * 2); + expect(await resultBlob.arrayBuffer()).toEqual(await blob.arrayBuffer()); + }); + } + + it('can pass an async import that resolves later to an outline object like a Map', async () => { + let resolve; + const promise = new Promise(r => (resolve = r)); + + const asyncClient = clientExports(promise); + + // We await the value on the servers so it's an async value that the client should wait for + const awaitedValue = await asyncClient; + + const map = new Map(); + map.set('value', awaitedValue); + + const stream = passThrough( + ReactServerDOMServer.renderToReadableStream(map, webpackMap), + ); + + // Parsing the root blocks because the module hasn't loaded yet + const resultPromise = ReactServerDOMClient.createFromReadableStream( + stream, + { + ssrManifest: { + moduleMap: null, + moduleLoading: null, + }, + }, + ); + + // Afterwards we finally resolve the module value so it's available on the client + resolve('hello'); + + const result = await resultPromise; + expect(result instanceof Map).toBe(true); + expect(result.get('value')).toBe('hello'); + }); + it('warns if passing a this argument to bind() of a server reference', async () => { const ServerModule = serverExports({ greet: function () {}, diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 57079536b5758..5c14276f330b9 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -1239,27 +1239,25 @@ function serializeTypedArray( } function serializeBlob(request: Request, blob: Blob): string { - const id = request.nextChunkId++; - request.pendingChunks++; + const model: Array = [blob.type]; + const newTask = createTask( + request, + model, + null, + false, + request.abortableTasks, + ); const reader = blob.stream().getReader(); - const model: Array = [blob.type]; - function progress( entry: {done: false, value: Uint8Array} | {done: true, value: void}, ): Promise | void { if (entry.done) { - const blobId = outlineModel(request, model); - const blobReference = '$B' + blobId.toString(16); - const processedChunk = encodeReferenceChunk(request, id, blobReference); - request.completedRegularChunks.push(processedChunk); - if (request.destination !== null) { - flushCompletedChunks(request, request.destination); - } + pingTask(request, newTask); return; } - // TODO: Emit the chunk early and refer to it later. + // TODO: Emit the chunk early and refer to it later by dedupe. model.push(entry.value); // $FlowFixMe[incompatible-call] return reader.read().then(progress).catch(error); @@ -1267,7 +1265,8 @@ function serializeBlob(request: Request, blob: Blob): string { function error(reason: mixed) { const digest = logRecoverableError(request, reason); - emitErrorChunk(request, id, digest, reason); + emitErrorChunk(request, newTask.id, digest, reason); + request.abortableTasks.delete(newTask); if (request.destination !== null) { flushCompletedChunks(request, request.destination); } @@ -1275,7 +1274,7 @@ function serializeBlob(request: Request, blob: Blob): string { // $FlowFixMe[incompatible-call] reader.read().then(progress).catch(error); - return '$' + id.toString(16); + return '$B' + newTask.id.toString(16); } function escapeStringValue(value: string): string {