From 084d1beb2bbb9e136a8abd93a1b6760e16544f66 Mon Sep 17 00:00:00 2001 From: Hendrik Liebau Date: Mon, 1 Apr 2024 18:37:27 +0200 Subject: [PATCH] [Flight] Update stale blocked values in `createModelResolver` (#28669) Alternative to #28620. Instead of emitting lazy references to not-yet-emitted models in the Flight Server, this fixes the observed issue in https://github.com/unstubbable/ai-rsc-test/pull/1 by adjusting the lazy model resolution in the Flight Client to update stale blocked root models, before assigning them as chunk values. In addition, the element props are not outlined anymore in the Flight Server to avoid having to also handle their staleness in blocked elements. fixes #28595 --- .../react-client/src/ReactFlightClient.js | 7 ++ .../src/__tests__/ReactFlightDOM-test.js | 104 ++++++++++++++++++ .../react-server/src/ReactFlightServer.js | 30 ++--- 3 files changed, 128 insertions(+), 13 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 435ee06aac44b..b30f9d34d8189 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -584,6 +584,13 @@ function createModelResolver( } return value => { parentObject[key] = 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.deps--; if (blocked.deps === 0) { if (chunk.status !== BLOCKED) { diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js index 9ff119ac1419d..b792844a0b49f 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js @@ -812,6 +812,110 @@ describe('ReactFlightDOM', () => { expect(reportedErrors).toEqual([]); }); + it('should handle streaming async server components', async () => { + const reportedErrors = []; + + const Row = async ({current, next}) => { + const chunk = await next; + + if (chunk.done) { + return chunk.value; + } + + return ( + + + + ); + }; + + function createResolvablePromise() { + let _resolve, _reject; + + const promise = new Promise((resolve, reject) => { + _resolve = resolve; + _reject = reject; + }); + + return {promise, resolve: _resolve, reject: _reject}; + } + + function createSuspendedChunk(initialValue) { + const {promise, resolve, reject} = createResolvablePromise(); + + return { + row: ( + + + + ), + resolve, + reject, + }; + } + + function makeDelayedText() { + const {promise, resolve, reject} = createResolvablePromise(); + async function DelayedText() { + const data = await promise; + return
{data}
; + } + return [DelayedText, resolve, reject]; + } + + const [Posts, resolvePostsData] = makeDelayedText(); + const [Photos, resolvePhotosData] = makeDelayedText(); + const suspendedChunk = createSuspendedChunk(

loading

); + const {writable, readable} = getTestStream(); + const {pipe} = ReactServerDOMServer.renderToPipeableStream( + suspendedChunk.row, + webpackMap, + { + onError(error) { + reportedErrors.push(error); + }, + }, + ); + pipe(writable); + const response = ReactServerDOMClient.createFromReadableStream(readable); + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + + function ClientRoot() { + return use(response); + } + + await act(() => { + root.render(); + }); + + expect(container.innerHTML).toBe('

loading

'); + + const donePromise = createResolvablePromise(); + + const value = ( + loading posts and photos

}> + + + + ); + + await act(async () => { + suspendedChunk.resolve({value, done: false, next: donePromise.promise}); + donePromise.resolve({value, done: true}); + }); + + expect(container.innerHTML).toBe('

loading posts and photos

'); + + await act(async () => { + await resolvePostsData('posts'); + await resolvePhotosData('photos'); + }); + + expect(container.innerHTML).toBe('
posts
photos
'); + expect(reportedErrors).toEqual([]); + }); + it('should preserve state of client components on refetch', async () => { // Client diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index d253db44c4a15..ad0a28f37175c 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -243,11 +243,16 @@ export type ReactClientValue = type ReactClientObject = {+[key: string]: ReactClientValue}; +// task status const PENDING = 0; const COMPLETED = 1; const ABORTED = 3; const ERRORED = 4; +// object reference status +const SEEN_BUT_NOT_YET_OUTLINED = -1; +const NEVER_OUTLINED = -2; + type Task = { id: number, status: 0 | 1 | 3 | 4, @@ -280,7 +285,7 @@ export type Request = { writtenSymbols: Map, writtenClientReferences: Map, writtenServerReferences: Map, number>, - writtenObjects: WeakMap, // -1 means "seen" but not outlined. + writtenObjects: WeakMap, identifierPrefix: string, identifierCount: number, taintCleanupQueue: Array, @@ -1125,8 +1130,7 @@ function serializeMap( 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); + writtenObjects.set(key, SEEN_BUT_NOT_YET_OUTLINED); } } } @@ -1142,8 +1146,7 @@ function serializeSet(request: Request, set: Set): string { 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); + writtenObjects.set(key, SEEN_BUT_NOT_YET_OUTLINED); } } } @@ -1328,8 +1331,7 @@ function renderModelDestructive( // 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 if (existingId === -1) { - // Seen but not yet outlined. + } else if (existingId === SEEN_BUT_NOT_YET_OUTLINED) { // TODO: If we throw here we can treat this as suspending which causes an outline // but that is able to reuse the same task if we're already in one but then that // will be a lazy future value rather than guaranteed to exist but maybe that's good. @@ -1348,7 +1350,10 @@ function renderModelDestructive( } 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); + writtenObjects.set(value, SEEN_BUT_NOT_YET_OUTLINED); + // The element's props are marked as "never outlined" so that they are inlined into + // the same row as the element itself. + writtenObjects.set((value: any).props, NEVER_OUTLINED); } const element: React$Element = (value: any); @@ -1477,11 +1482,10 @@ function renderModelDestructive( // 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 if (existingId === -1) { - // Seen but not yet outlined. + } else if (existingId === SEEN_BUT_NOT_YET_OUTLINED) { const newId = outlineModel(request, (value: any)); return serializeByValueID(newId); - } else { + } else if (existingId !== NEVER_OUTLINED) { // We've already emitted this as an outlined object, so we can // just refer to that by its existing ID. return serializeByValueID(existingId); @@ -1489,7 +1493,7 @@ function renderModelDestructive( } 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); + writtenObjects.set(value, SEEN_BUT_NOT_YET_OUTLINED); } if (isArray(value)) { @@ -2007,7 +2011,7 @@ function renderConsoleValue( return serializeInfinitePromise(); } - if (existingId !== undefined && existingId !== -1) { + if (existingId !== undefined && existingId >= 0) { // We've already emitted this as a real object, so we can // just refer to that by its existing ID. return serializeByValueID(existingId);