From e80cdb02902c0a6a7e02b72efe7b7190898e49c2 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 25 Jan 2024 15:24:04 -0500 Subject: [PATCH] Don't dedupe Elements if they're in a non-default Context If an element gets wrapped in a different server component then that has a different keyPath context and the element might end up with a different key. So we don't use the deduping mechanism if we're already inside a Server Component parent with a key or otherwise. Only the simple case gets deduped. The props of a client element are still deduped though if they're the same instance. --- .../src/__tests__/ReactFlightDOMEdge-test.js | 31 ++++++++++++++++++ .../react-server/src/ReactFlightServer.js | 32 ++++++++++++++----- .../ReactFlightServerConfigBundlerCustom.js | 6 ++-- 3 files changed, 59 insertions(+), 10 deletions(-) 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 5d966a16ded59..13b9a16338827 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js @@ -225,6 +225,37 @@ describe('ReactFlightDOMEdge', () => { 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); + }); + + it('should execute repeated host components only once', async () => { + const div =
this is a long return value
; + let timesRendered = 0; + function ServerComponent() { + timesRendered++; + return div; + } + const element = ; + const children = new Array(30).fill(element); + const resolvedChildren = new Array(30).fill(div); + 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); diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index bdbc26189a2f8..edb88c6948367 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -787,10 +787,6 @@ function createTask( abortSet: Set, ): Task { const id = request.nextChunkId++; - if (typeof model === 'object' && model !== null) { - // Register this model as having the ID we're about to write. - request.writtenObjects.set(model, id); - } const task: Task = { id, status: PENDING, @@ -988,7 +984,17 @@ function serializeClientReference( } } -function outlineModel(request: Request, value: ReactClientValue): number { +function outlineModel( + request: Request, + value: + | ClientReference + | ServerReference + | Iterable + | Array + | Map + | Set + | ReactClientObject, +): number { request.pendingChunks++; const newTask = createTask( request, @@ -998,6 +1004,7 @@ function outlineModel(request: Request, value: ReactClientValue): number { rootContextSnapshot, // Therefore we don't pass any contextual information along. request.abortableTasks, ); + request.writtenObjects.set(value, newTask.id); retryTask(request, newTask); return newTask.id; } @@ -1251,9 +1258,18 @@ function renderModelDestructive( const writtenObjects = request.writtenObjects; const existingId = writtenObjects.get(value); if (existingId !== undefined) { - if (existingId === -1) { + if ( + enableServerComponentKeys && + (task.keyPath !== null || + task.implicitSlot || + task.context !== rootContextSnapshot) + ) { + // If we're in some kind of context we can't reuse the result of this render or + // previous renders of this element. We only reuse elements if they're not wrapped + // by another Server Component. + } else if (existingId === -1) { // Seen but not yet outlined. - const newId = outlineModel(request, value); + const newId = outlineModel(request, (value: any)); return serializeByValueID(newId); } else if (modelRoot === value) { // This is the ID we're currently emitting so we need to write it @@ -1360,7 +1376,7 @@ function renderModelDestructive( if (existingId !== undefined) { if (existingId === -1) { // Seen but not yet outlined. - const newId = outlineModel(request, value); + const newId = outlineModel(request, (value: any)); return serializeByValueID(newId); } else if (modelRoot === value) { // This is the ID we're currently emitting so we need to write it diff --git a/packages/react-server/src/ReactFlightServerConfigBundlerCustom.js b/packages/react-server/src/ReactFlightServerConfigBundlerCustom.js index b1fbc93ee61b1..d30365a73211d 100644 --- a/packages/react-server/src/ReactFlightServerConfigBundlerCustom.js +++ b/packages/react-server/src/ReactFlightServerConfigBundlerCustom.js @@ -9,9 +9,11 @@ declare var $$$config: any; +interface Reference {} + export opaque type ClientManifest = mixed; -export opaque type ClientReference = mixed; // eslint-disable-line no-unused-vars -export opaque type ServerReference = mixed; // eslint-disable-line no-unused-vars +export opaque type ClientReference: Reference = Reference; // eslint-disable-line no-unused-vars +export opaque type ServerReference: Reference = Reference; // eslint-disable-line no-unused-vars export opaque type ClientReferenceMetadata: any = mixed; export opaque type ServerReferenceId: any = mixed; export opaque type ClientReferenceKey: any = mixed;