From f172fa74610df623f1e82997ba66337452deeaa1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 19 Oct 2023 13:41:16 -0400 Subject: [PATCH] [Flight] Detriplicate Objects (#27537) Now that we no longer support Server Context, we can now deduplicate objects. It's not completely safe for useId but only in the same way as it's not safe if you reuse elements on the client, so it's not a new issue. This also solves cyclic object references. The issue is that we prefer to inline objects into a plain JSON format when an object is not going to get reused. In this case the object doesn't have an id. We could potentially serialize a reference to an existing model + a path to it but it bloats the format and complicates the client. In a smarter flush phase like we have in Fizz we could choose to inline or outline depending on what we've discovered so far before a flush. We can't do that here since we use native stringify. However, even in that solution you might not know that you're going to discover the same object later so it's not perfect deduping anyway. Instead, I use a heuristic where I mark previously seen objects and if I ever see that object again, then I'll outline it. The idea is that most objects are just going to be emitted once and if it's more than once it's fairly likely you have a shared reference to it somewhere and it might be more than two. The third object gets deduplicated (or "detriplicated"). It's not a perfect heuristic because when we write the second object we will have already visited all the nested objects inside of it, which causes us to outline every nested object too even those weren't reference more than by that parent. Not sure how to solve for that. If we for some other reason outline an object such as if it suspends, then it's truly deduplicated since it already has an id. --- .../react-client/src/ReactFlightClient.js | 46 +++- .../src/__tests__/ReactFlight-test.js | 29 ++- .../src/__tests__/ReactFlightDOMEdge-test.js | 59 +++++ .../react-server/src/ReactFlightServer.js | 203 ++++++++++++++---- 4 files changed, 282 insertions(+), 55 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 3fb97089f5..8c584bbc59 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -72,6 +72,7 @@ type RowParserState = 0 | 1 | 2 | 3 | 4; const PENDING = 'pending'; const BLOCKED = 'blocked'; +const CYCLIC = 'cyclic'; const RESOLVED_MODEL = 'resolved_model'; const RESOLVED_MODULE = 'resolved_module'; const INITIALIZED = 'fulfilled'; @@ -91,6 +92,13 @@ type BlockedChunk = { _response: Response, then(resolve: (T) => mixed, reject: (mixed) => mixed): void, }; +type CyclicChunk = { + status: 'cyclic', + value: null | Array<(T) => mixed>, + reason: null | Array<(mixed) => mixed>, + _response: Response, + then(resolve: (T) => mixed, reject: (mixed) => mixed): void, +}; type ResolvedModelChunk = { status: 'resolved_model', value: UninitializedModel, @@ -122,6 +130,7 @@ type ErroredChunk = { type SomeChunk = | PendingChunk | BlockedChunk + | CyclicChunk | ResolvedModelChunk | ResolvedModuleChunk | InitializedChunk @@ -160,6 +169,7 @@ Chunk.prototype.then = function ( break; case PENDING: case BLOCKED: + case CYCLIC: if (resolve) { if (chunk.value === null) { chunk.value = ([]: Array<(T) => mixed>); @@ -211,6 +221,7 @@ function readChunk(chunk: SomeChunk): T { return chunk.value; case PENDING: case BLOCKED: + case CYCLIC: // eslint-disable-next-line no-throw-literal throw ((chunk: any): Thenable); default: @@ -259,6 +270,7 @@ function wakeChunkIfInitialized( break; case PENDING: case BLOCKED: + case CYCLIC: chunk.value = resolveListeners; chunk.reason = rejectListeners; break; @@ -365,8 +377,19 @@ function initializeModelChunk(chunk: ResolvedModelChunk): void { const prevBlocked = initializingChunkBlockedModel; initializingChunk = chunk; initializingChunkBlockedModel = null; + + const resolvedModel = chunk.value; + + // We go to the CYCLIC state until we've fully resolved this. + // We do this before parsing in case we try to initialize the same chunk + // while parsing the model. Such as in a cyclic reference. + const cyclicChunk: CyclicChunk = (chunk: any); + cyclicChunk.status = CYCLIC; + cyclicChunk.value = null; + cyclicChunk.reason = null; + try { - const value: T = parseModel(chunk._response, chunk.value); + const value: T = parseModel(chunk._response, resolvedModel); if ( initializingChunkBlockedModel !== null && initializingChunkBlockedModel.deps > 0 @@ -379,9 +402,13 @@ function initializeModelChunk(chunk: ResolvedModelChunk): void { blockedChunk.value = null; blockedChunk.reason = null; } else { + const resolveListeners = cyclicChunk.value; const initializedChunk: InitializedChunk = (chunk: any); initializedChunk.status = INITIALIZED; initializedChunk.value = value; + if (resolveListeners !== null) { + wakeChunk(resolveListeners, value); + } } } catch (error) { const erroredChunk: ErroredChunk = (chunk: any); @@ -491,15 +518,18 @@ function createModelResolver( chunk: SomeChunk, parentObject: Object, key: string, + cyclic: boolean, ): (value: any) => void { let blocked; if (initializingChunkBlockedModel) { blocked = initializingChunkBlockedModel; - blocked.deps++; + if (!cyclic) { + blocked.deps++; + } } else { blocked = initializingChunkBlockedModel = { - deps: 1, - value: null, + deps: cyclic ? 0 : 1, + value: (null: any), }; } return value => { @@ -673,9 +703,15 @@ function parseModelString( return chunk.value; case PENDING: case BLOCKED: + case CYCLIC: const parentChunk = initializingChunk; chunk.then( - createModelResolver(parentChunk, parentObject, key), + createModelResolver( + parentChunk, + parentObject, + key, + chunk.status === CYCLIC, + ), createModelReject(parentChunk), ); return null; diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index c68630e5b2..57c54a82b4 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -374,12 +374,13 @@ describe('ReactFlight', () => { }); it('can transport Map', async () => { - function ComponentClient({prop}) { + function ComponentClient({prop, selected}) { return ` map: ${prop instanceof Map} size: ${prop.size} greet: ${prop.get('hi').greet} content: ${JSON.stringify(Array.from(prop))} + selected: ${prop.get(selected)} `; } const Component = clientReference(ComponentClient); @@ -389,7 +390,7 @@ describe('ReactFlight', () => { ['hi', {greet: 'world'}], [objKey, 123], ]); - const model = ; + const model = ; const transport = ReactNoopFlightServer.render(model); @@ -402,23 +403,25 @@ describe('ReactFlight', () => { size: 2 greet: world content: [["hi",{"greet":"world"}],[{"obj":"key"},123]] + selected: 123 `); }); it('can transport Set', async () => { - function ComponentClient({prop}) { + function ComponentClient({prop, selected}) { return ` set: ${prop instanceof Set} size: ${prop.size} hi: ${prop.has('hi')} content: ${JSON.stringify(Array.from(prop))} + selected: ${prop.has(selected)} `; } const Component = clientReference(ComponentClient); const objKey = {obj: 'key'}; const set = new Set(['hi', objKey]); - const model = ; + const model = ; const transport = ReactNoopFlightServer.render(model); @@ -431,9 +434,27 @@ describe('ReactFlight', () => { size: 2 hi: true content: ["hi",{"obj":"key"}] + selected: true `); }); + it('can transport cyclic objects', async () => { + function ComponentClient({prop}) { + expect(prop.obj.obj.obj).toBe(prop.obj.obj); + } + const Component = clientReference(ComponentClient); + + const cyclic = {obj: null}; + cyclic.obj = cyclic; + const model = ; + + const transport = ReactNoopFlightServer.render(model); + + await act(async () => { + ReactNoop.render(await ReactNoopFlightClient.read(transport)); + }); + }); + it('can render a lazy component as a shared component on the server', async () => { function SharedComponent({text}) { return ( 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 a81ec52583..2f643fbbfb 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js @@ -183,6 +183,65 @@ describe('ReactFlightDOMEdge', () => { expect(result.text2).toBe(testString2); }); + it('should encode repeated objects in a compact format by deduping', async () => { + const obj = { + this: {is: 'a large objected'}, + with: {many: 'properties in it'}, + }; + const props = { + items: new Array(30).fill(obj), + }; + const stream = ReactServerDOMServer.renderToReadableStream(props); + const [stream1, stream2] = passThrough(stream).tee(); + + const serializedContent = await readResult(stream1); + expect(serializedContent.length).toBeLessThan(400); + + const result = await ReactServerDOMClient.createFromReadableStream( + stream2, + { + ssrManifest: { + moduleMap: null, + moduleLoading: null, + }, + }, + ); + // Should still match the result when parsed + expect(result).toEqual(props); + expect(result.items[5]).toBe(result.items[10]); // two random items are the same instance + // TODO: items[0] is not the same as the others in this case + }); + + it('should execute repeated server components only once', async () => { + const str = 'this is a long return value'; + let timesRendered = 0; + function ServerComponent() { + timesRendered++; + return str; + } + const element = ; + const children = new Array(30).fill(element); + const resolvedChildren = new Array(30).fill(str); + const stream = ReactServerDOMServer.renderToReadableStream(children); + const [stream1, stream2] = passThrough(stream).tee(); + + const serializedContent = await readResult(stream1); + expect(serializedContent.length).toBeLessThan(400); + expect(timesRendered).toBeLessThan(5); + + const result = await ReactServerDOMClient.createFromReadableStream( + stream2, + { + ssrManifest: { + moduleMap: null, + moduleLoading: null, + }, + }, + ); + // Should still match the result when parsed + expect(result).toEqual(resolvedChildren); + }); + // @gate enableBinaryFlight it('should be able to serialize any kind of typed array', async () => { const buffer = new Uint8Array([ diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 6c08dd9948..5da20fcdda 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -15,6 +15,7 @@ import { enableBinaryFlight, enablePostpone, enableTaint, + enableServerContext, } from 'shared/ReactFeatureFlags'; import { @@ -180,6 +181,8 @@ type Task = { thenableState: ThenableState | null, }; +interface Reference {} + export type Request = { status: 0 | 1 | 2, flushScheduled: boolean, @@ -200,6 +203,7 @@ export type Request = { writtenClientReferences: Map, writtenServerReferences: Map, number>, writtenProviders: Map, + writtenObjects: WeakMap, // -1 means "seen" but not outlined. identifierPrefix: string, identifierCount: number, taintCleanupQueue: Array, @@ -298,6 +302,7 @@ export function createRequest( writtenClientReferences: new Map(), writtenServerReferences: new Map(), writtenProviders: new Map(), + writtenObjects: new WeakMap(), identifierPrefix: identifierPrefix || '', identifierCount: 1, taintCleanupQueue: cleanupQueue, @@ -581,28 +586,31 @@ function attemptResolveElement( ); } case REACT_PROVIDER_TYPE: { - pushProvider(type._context, props.value); - if (__DEV__) { - const extraKeys = Object.keys(props).filter(value => { - if (value === 'children' || value === 'value') { - return false; + if (enableServerContext) { + pushProvider(type._context, props.value); + if (__DEV__) { + const extraKeys = Object.keys(props).filter(value => { + if (value === 'children' || value === 'value') { + return false; + } + return true; + }); + if (extraKeys.length !== 0) { + console.error( + 'ServerContext can only have a value prop and children. Found: %s', + JSON.stringify(extraKeys), + ); } - return true; - }); - if (extraKeys.length !== 0) { - console.error( - 'ServerContext can only have a value prop and children. Found: %s', - JSON.stringify(extraKeys), - ); } + return [ + REACT_ELEMENT_TYPE, + type, + key, + // Rely on __popProvider being serialized last to pop the provider. + {value: props.value, children: props.children, __pop: POP}, + ]; } - return [ - REACT_ELEMENT_TYPE, - type, - key, - // Rely on __popProvider being serialized last to pop the provider. - {value: props.value, children: props.children, __pop: POP}, - ]; + // Fallthrough } } } @@ -759,10 +767,14 @@ function serializeClientReference( function outlineModel(request: Request, value: any): number { request.pendingChunks++; - const outlinedId = request.nextChunkId++; - // We assume that this object doesn't suspend, but a child might. - emitModelChunk(request, outlinedId, value); - return outlinedId; + const newTask = createTask( + request, + value, + getActiveContext(), + request.abortableTasks, + ); + retryTask(request, newTask); + return newTask.id; } function serializeServerReference( @@ -810,12 +822,36 @@ function serializeMap( request: Request, map: Map, ): string { - const id = outlineModel(request, Array.from(map)); + const entries = Array.from(map); + for (let i = 0; i < entries.length; i++) { + const key = entries[i][0]; + if (typeof key === 'object' && key !== null) { + const writtenObjects = request.writtenObjects; + const existingId = writtenObjects.get(key); + if (existingId === undefined) { + // Mark all object keys as seen so that they're always outlined. + writtenObjects.set(key, -1); + } + } + } + const id = outlineModel(request, entries); return '$Q' + id.toString(16); } function serializeSet(request: Request, set: Set): string { - const id = outlineModel(request, Array.from(set)); + const entries = Array.from(set); + for (let i = 0; i < entries.length; i++) { + const key = entries[i]; + if (typeof key === 'object' && key !== null) { + const writtenObjects = request.writtenObjects; + const existingId = writtenObjects.get(key); + if (existingId === undefined) { + // Mark all object keys as seen so that they're always outlined. + writtenObjects.set(key, -1); + } + } + } + const id = outlineModel(request, entries); return '$W' + id.toString(16); } @@ -860,6 +896,7 @@ function escapeStringValue(value: string): string { let insideContextProps = null; let isInsideContextValue = false; +let modelRoot: null | ReactClientValue = false; function resolveModelToJSON( request: Request, @@ -913,6 +950,7 @@ function resolveModelToJSON( if (__DEV__) { if ( + enableServerContext && parent[0] === REACT_ELEMENT_TYPE && parent[1] && (parent[1]: any).$$typeof === REACT_PROVIDER_TYPE && @@ -934,7 +972,7 @@ function resolveModelToJSON( (value: any).$$typeof === REACT_LAZY_TYPE) ) { if (__DEV__) { - if (isInsideContextValue) { + if (enableServerContext && isInsideContextValue) { console.error('React elements are not allowed in ServerContext'); } } @@ -942,6 +980,28 @@ function resolveModelToJSON( try { switch ((value: any).$$typeof) { case REACT_ELEMENT_TYPE: { + const writtenObjects = request.writtenObjects; + const existingId = writtenObjects.get(value); + if (existingId !== undefined) { + if (existingId === -1) { + // Seen but not yet outlined. + const newId = outlineModel(request, value); + return serializeByValueID(newId); + } else if (modelRoot === value) { + // This is the ID we're currently emitting so we need to write it + // once but if we discover it again, we refer to it by id. + modelRoot = null; + } else { + // We've already emitted this as an outlined object, so we can + // just refer to that by its existing ID. + return serializeByValueID(existingId); + } + } else { + // This is the first time we've seen this object. We may never see it again + // so we'll inline it. Mark it as seen. If we see it again, we'll outline. + writtenObjects.set(value, -1); + } + // TODO: Concatenate keys of parents onto children. const element: React$Element = (value: any); // Attempt to render the Server Component. @@ -1022,31 +1082,70 @@ function resolveModelToJSON( } if (isClientReference(value)) { return serializeClientReference(request, parent, key, (value: any)); - // $FlowFixMe[method-unbinding] - } else if (typeof value.then === 'function') { + } + + const writtenObjects = request.writtenObjects; + const existingId = writtenObjects.get(value); + // $FlowFixMe[method-unbinding] + if (typeof value.then === 'function') { + if (existingId !== undefined) { + if (modelRoot === value) { + // This is the ID we're currently emitting so we need to write it + // once but if we discover it again, we refer to it by id. + modelRoot = null; + } else { + // We've seen this promise before, so we can just refer to the same result. + return serializePromiseID(existingId); + } + } // We assume that any object with a .then property is a "Thenable" type, // or a Promise type. Either of which can be represented by a Promise. const promiseId = serializeThenable(request, (value: any)); + writtenObjects.set(value, promiseId); return serializePromiseID(promiseId); - } else if ((value: any).$$typeof === REACT_PROVIDER_TYPE) { - const providerKey = ((value: any): ReactProviderType)._context - ._globalName; - const writtenProviders = request.writtenProviders; - let providerId = writtenProviders.get(key); - if (providerId === undefined) { - request.pendingChunks++; - providerId = request.nextChunkId++; - writtenProviders.set(providerKey, providerId); - emitProviderChunk(request, providerId, providerKey); + } + + if (enableServerContext) { + if ((value: any).$$typeof === REACT_PROVIDER_TYPE) { + const providerKey = ((value: any): ReactProviderType)._context + ._globalName; + const writtenProviders = request.writtenProviders; + let providerId = writtenProviders.get(key); + if (providerId === undefined) { + request.pendingChunks++; + providerId = request.nextChunkId++; + writtenProviders.set(providerKey, providerId); + emitProviderChunk(request, providerId, providerKey); + } + return serializeByValueID(providerId); + } else if (value === POP) { + popProvider(); + if (__DEV__) { + insideContextProps = null; + isInsideContextValue = false; + } + return (undefined: any); } - return serializeByValueID(providerId); - } else if (value === POP) { - popProvider(); - if (__DEV__) { - insideContextProps = null; - isInsideContextValue = false; + } + + if (existingId !== undefined) { + if (existingId === -1) { + // Seen but not yet outlined. + const newId = outlineModel(request, value); + return serializeByValueID(newId); + } else if (modelRoot === value) { + // This is the ID we're currently emitting so we need to write it + // once but if we discover it again, we refer to it by id. + modelRoot = null; + } else { + // We've already emitted this as an outlined object, so we can + // just refer to that by its existing ID. + return serializeByValueID(existingId); } - return (undefined: any); + } else { + // This is the first time we've seen this object. We may never see it again + // so we'll inline it. Mark it as seen. If we see it again, we'll outline. + writtenObjects.set(value, -1); } if (isArray(value)) { @@ -1401,6 +1500,10 @@ function emitModelChunk( id: number, model: ReactClientValue, ): void { + // Track the root so we know that we have to emit this object even though it + // already has an ID. This is needed because we might see this object twice + // in the same toJSON if it is cyclic. + modelRoot = model; // $FlowFixMe[incompatible-type] stringify can return null const json: string = stringify(model, request.toJSON); const row = id.toString(16) + ':' + json + '\n'; @@ -1422,6 +1525,8 @@ function retryTask(request: Request, task: Task): void { value !== null && (value: any).$$typeof === REACT_ELEMENT_TYPE ) { + request.writtenObjects.set(value, task.id); + // TODO: Concatenate keys of parents onto children. const element: React$Element = (value: any); @@ -1454,6 +1559,7 @@ function retryTask(request: Request, task: Task): void { value !== null && (value: any).$$typeof === REACT_ELEMENT_TYPE ) { + request.writtenObjects.set(value, task.id); // TODO: Concatenate keys of parents onto children. const nextElement: React$Element = (value: any); task.model = value; @@ -1468,6 +1574,11 @@ function retryTask(request: Request, task: Task): void { } } + // Track that this object is outlined and has an id. + if (typeof value === 'object' && value !== null) { + request.writtenObjects.set(value, task.id); + } + emitModelChunk(request, task.id, value); request.abortableTasks.delete(task); task.status = COMPLETED; @@ -1703,7 +1814,7 @@ export function abort(request: Request, reason: mixed): void { function importServerContexts( contexts?: Array<[string, ServerContextJSONValue]>, ) { - if (contexts) { + if (enableServerContext && contexts) { const prevContext = getActiveContext(); switchContext(rootContextSnapshot); for (let i = 0; i < contexts.length; i++) {