From 6328da10b96aabb42e2ee6e696af0f3761515cfd Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 29 Aug 2023 17:13:00 -0400 Subject: [PATCH] useFormState: Emit comment to mark whether state matches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A planned feature of useFormState is that if page load is the result of an MPA-style form submission — i.e. a form was submitted before it was hydrated, using Server Actions — the state should transfer to the next page. I haven't implemented that part yet, but as a prerequisite, we need some way for Fizz to indicate whether a useFormState hook was rendered using the "postback" state. That way we can do all state matching logic on the server without having to replicate it on the client, too. The approach here is to emit a comment node for each useFormState hook. We use one of two comment types: `` for a normal useFormState hook, and `` for a hook that was rendered using the postback state. React will read these markers during hydration. This is similar to how we encode Suspense boundaries. Again, the actual matching algorithm is not yet implemented — for now, the "not matching" marker is always emitted. We can optimize this further by not emitting any markers for a render that is not the result of a form postback, which I'll do in subsequent PRs. --- .../src/client/ReactFiberConfigDOM.js | 33 ++++- .../src/server/ReactFizzConfigDOM.js | 15 +++ .../src/server/ReactFizzConfigDOMLegacy.js | 2 + .../src/__tests__/ReactDOMFizzServer-test.js | 119 ++++++++++++++++++ .../src/ReactFiberConfigWithNoHydration.js | 2 + .../react-reconciler/src/ReactFiberHooks.js | 14 ++- .../src/ReactFiberHydrationContext.js | 27 ++++ .../src/forks/ReactFiberConfig.custom.js | 2 + packages/react-server/src/ReactFizzHooks.js | 39 +++++- .../src/forks/ReactFizzConfig.custom.js | 4 + 10 files changed, 252 insertions(+), 5 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 9e7894cda68ba..0493fbd21a34d 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -160,7 +160,12 @@ export type TextInstance = Text; export interface SuspenseInstance extends Comment { _reactRetry?: () => void; } -export type HydratableInstance = Instance | TextInstance | SuspenseInstance; +type FormStateMarkerInstance = Comment; +export type HydratableInstance = + | Instance + | TextInstance + | SuspenseInstance + | FormStateMarkerInstance; export type PublicInstance = Element | Text; export type HostContextDev = { context: HostContextProd, @@ -187,6 +192,8 @@ const SUSPENSE_START_DATA = '$'; const SUSPENSE_END_DATA = '/$'; const SUSPENSE_PENDING_START_DATA = '$?'; const SUSPENSE_FALLBACK_START_DATA = '$!'; +const FORM_STATE_IS_MATCHING = '!F'; +const FORM_STATE_IS_NOT_MATCHING = 'F'; const STYLE = 'style'; @@ -1283,6 +1290,26 @@ export function registerSuspenseInstanceRetry( instance._reactRetry = callback; } +export function canHydrateFormStateMarker( + instance: HydratableInstance, +): null | FormStateMarkerInstance { + const nodeData = (instance: any).data; + if ( + nodeData === FORM_STATE_IS_MATCHING || + nodeData === FORM_STATE_IS_NOT_MATCHING + ) { + const markerInstance: FormStateMarkerInstance = (instance: any); + return markerInstance; + } + return null; +} + +export function isFormStateMarkerMatching( + markerInstance: FormStateMarkerInstance, +): boolean { + return markerInstance.data === FORM_STATE_IS_MATCHING; +} + function getNextHydratable(node: ?Node) { // Skip non-hydratable nodes. for (; node != null; node = ((node: any): Node).nextSibling) { @@ -1295,7 +1322,9 @@ function getNextHydratable(node: ?Node) { if ( nodeData === SUSPENSE_START_DATA || nodeData === SUSPENSE_FALLBACK_START_DATA || - nodeData === SUSPENSE_PENDING_START_DATA + nodeData === SUSPENSE_PENDING_START_DATA || + nodeData === FORM_STATE_IS_MATCHING || + nodeData === FORM_STATE_IS_NOT_MATCHING ) { break; } diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js index 0eb1c51f72622..9a5260efdf92e 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js @@ -1525,6 +1525,21 @@ function injectFormReplayingRuntime( } } +const formStateMarkerIsMatching = stringToPrecomputedChunk(''); +const formStateMarkerIsNotMatching = stringToPrecomputedChunk(''); + +export function pushFormStateMarkerIsMatching( + target: Array, +) { + target.push(formStateMarkerIsMatching); +} + +export function pushFormStateMarkerIsNotMatching( + target: Array, +) { + target.push(formStateMarkerIsNotMatching); +} + function pushStartForm( target: Array, props: Object, diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js index ee3db635ff98c..41ae968dd4cb3 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js @@ -101,6 +101,8 @@ export { pushEndInstance, pushStartCompletedSuspenseBoundary, pushEndCompletedSuspenseBoundary, + pushFormStateMarkerIsMatching, + pushFormStateMarkerIsNotMatching, writeStartSegment, writeEndSegment, writeCompletedSegmentInstruction, diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index fb3a2ae86ee5a..477f20215d497 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -28,6 +28,7 @@ let SuspenseList; let useSyncExternalStore; let useSyncExternalStoreWithSelector; let use; +let useFormState; let PropTypes; let textCache; let writable; @@ -83,6 +84,7 @@ describe('ReactDOMFizzServer', () => { if (gate(flags => flags.enableSuspenseList)) { SuspenseList = React.unstable_SuspenseList; } + useFormState = ReactDOM.experimental_useFormState; PropTypes = require('prop-types'); @@ -5911,6 +5913,123 @@ describe('ReactDOMFizzServer', () => { expect(getVisibleChildren(container)).toEqual('Hi'); }); + // @gate enableFormActions + // @gate enableAsyncActions + it('useFormState hydrates without a mismatch', async () => { + // This is testing an implementation detail: useFormState emits comment + // nodes into the SSR stream, so this checks that they are handled correctly + // during hydration. + + async function action(state) { + return state; + } + + const childRef = React.createRef(null); + function Form() { + const [state] = useFormState(action, 0); + const text = `Child: ${state}`; + return ( +
+ {text} +
+ ); + } + + function App() { + return ( +
+
+
+
+ Sibling +
+ ); + } + + await act(() => { + const {pipe} = renderToPipeableStream(); + pipe(writable); + }); + expect(getVisibleChildren(container)).toEqual( +
+
+
Child: 0
+
+ Sibling +
, + ); + const child = document.getElementById('child'); + + // Confirm that it hydrates correctly + await clientAct(() => { + ReactDOMClient.hydrateRoot(container, ); + }); + expect(childRef.current).toBe(child); + }); + + // @gate enableFormActions + // @gate enableAsyncActions + it("useFormState hydrates without a mismatch if there's a render phase update", async () => { + async function action(state) { + return state; + } + + const childRef = React.createRef(null); + function Form() { + const [localState, setLocalState] = React.useState(0); + if (localState < 3) { + setLocalState(localState + 1); + } + + // Because of the render phase update above, this component is evaluated + // multiple times (even during SSR), but it should only emit a single + // marker per useFormState instance. + const [formState] = useFormState(action, 0); + const text = `${readText('Child')}:${formState}:${localState}`; + return ( +
+ {text} +
+ ); + } + + function App() { + return ( +
+ + + + Sibling +
+ ); + } + + await act(() => { + const {pipe} = renderToPipeableStream(); + pipe(writable); + }); + expect(getVisibleChildren(container)).toEqual( +
+ Loading...Sibling +
, + ); + + await act(() => resolveText('Child')); + expect(getVisibleChildren(container)).toEqual( +
+
Child:0:3
+ Sibling +
, + ); + const child = document.getElementById('child'); + + // Confirm that it hydrates correctly + await clientAct(() => { + ReactDOMClient.hydrateRoot(container, ); + }); + expect(childRef.current).toBe(child); + }); + describe('useEffectEvent', () => { // @gate enableUseEffectEventHook it('can server render a component with useEffectEvent', async () => { diff --git a/packages/react-reconciler/src/ReactFiberConfigWithNoHydration.js b/packages/react-reconciler/src/ReactFiberConfigWithNoHydration.js index f467cbb86b733..7e2b181f4a623 100644 --- a/packages/react-reconciler/src/ReactFiberConfigWithNoHydration.js +++ b/packages/react-reconciler/src/ReactFiberConfigWithNoHydration.js @@ -26,6 +26,8 @@ export const isSuspenseInstancePending = shim; export const isSuspenseInstanceFallback = shim; export const getSuspenseInstanceFallbackErrorDetails = shim; export const registerSuspenseInstanceRetry = shim; +export const canHydrateFormStateMarker = shim; +export const isFormStateMarkerMatching = shim; export const getNextHydratableSibling = shim; export const getFirstHydratableChild = shim; export const getFirstHydratableChildWithinContainer = shim; diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 92864e038b2d2..f8f935846f86f 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -111,7 +111,10 @@ import { markWorkInProgressReceivedUpdate, checkIfWorkInProgressReceivedUpdate, } from './ReactFiberBeginWork'; -import {getIsHydrating} from './ReactFiberHydrationContext'; +import { + getIsHydrating, + tryToClaimNextHydratableFormMarkerInstance, +} from './ReactFiberHydrationContext'; import {logStateUpdateScheduled} from './DebugTracing'; import { markStateUpdateScheduled, @@ -2010,6 +2013,12 @@ function mountFormState( initialState: S, permalink?: string, ): [S, (P) => void] { + if (getIsHydrating()) { + // TODO: If this function returns true, it means we should use the form + // state passed to hydrateRoot instead of initialState. + tryToClaimNextHydratableFormMarkerInstance(currentlyRenderingFiber); + } + // State hook. The state is stored in a thenable which is then unwrapped by // the `use` algorithm during render. const stateHook = mountWorkInProgressHook(); @@ -2145,7 +2154,8 @@ function rerenderFormState( } // This is a mount. No updates to process. - const state = stateHook.memoizedState; + const thenable: Thenable = stateHook.memoizedState; + const state = useThenable(thenable); const actionQueueHook = updateWorkInProgressHook(); const actionQueue = actionQueueHook.queue; diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.js b/packages/react-reconciler/src/ReactFiberHydrationContext.js index 3453fcb451631..ed7f39c92f11b 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.js @@ -76,6 +76,8 @@ import { canHydrateInstance, canHydrateTextInstance, canHydrateSuspenseInstance, + canHydrateFormStateMarker, + isFormStateMarkerMatching, isHydratableText, } from './ReactFiberConfig'; import {OffscreenLane} from './ReactFiberLane'; @@ -595,6 +597,31 @@ function tryToClaimNextHydratableSuspenseInstance(fiber: Fiber): void { } } +export function tryToClaimNextHydratableFormMarkerInstance( + fiber: Fiber, +): boolean { + if (!isHydrating) { + return false; + } + if (nextHydratableInstance) { + const markerInstance = canHydrateFormStateMarker(nextHydratableInstance); + if (markerInstance) { + // Found the marker instance. + nextHydratableInstance = getNextHydratableSibling(markerInstance); + // Return true if this marker instance should use the state passed + // to hydrateRoot. + // TODO: As an optimization, Fizz should only emit these markers if form + // state is passed at the root. + return isFormStateMarkerMatching(markerInstance); + } + } + // Should have found a marker instance. Throw an error to trigger client + // rendering. We don't bother to check if we're in a concurrent root because + // useFormState is a new API, so backwards compat is not an issue. + throwOnHydrationMismatch(fiber); + return false; +} + function prepareToHydrateHostInstance( fiber: Fiber, hostContext: HostContext, diff --git a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js index 2fa2f733ead75..13b7a8bd96049 100644 --- a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js +++ b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js @@ -142,6 +142,8 @@ export const getSuspenseInstanceFallbackErrorDetails = $$$config.getSuspenseInstanceFallbackErrorDetails; export const registerSuspenseInstanceRetry = $$$config.registerSuspenseInstanceRetry; +export const canHydrateFormStateMarker = $$$config.canHydrateFormStateMarker; +export const isFormStateMarkerMatching = $$$config.isFormStateMarkerMatching; export const getNextHydratableSibling = $$$config.getNextHydratableSibling; export const getFirstHydratableChild = $$$config.getFirstHydratableChild; export const getFirstHydratableChildWithinContainer = diff --git a/packages/react-server/src/ReactFizzHooks.js b/packages/react-server/src/ReactFizzHooks.js index 377c21a9fe637..dc2a2392df84f 100644 --- a/packages/react-server/src/ReactFizzHooks.js +++ b/packages/react-server/src/ReactFizzHooks.js @@ -21,12 +21,20 @@ import type {ResumableState} from './ReactFizzConfig'; import type {Task} from './ReactFizzServer'; import type {ThenableState} from './ReactFizzThenable'; import type {TransitionStatus} from './ReactFizzConfig'; +import type { + Chunk, + PrecomputedChunk, +} from 'react-server/src/ReactServerStreamConfig'; import {readContext as readContextImpl} from './ReactFizzNewContext'; import {getTreeId} from './ReactFizzTreeContext'; import {createThenableState, trackUsedThenable} from './ReactFizzThenable'; -import {makeId, NotPendingTransition} from './ReactFizzConfig'; +import { + makeId, + NotPendingTransition, + pushFormStateMarkerIsNotMatching, +} from './ReactFizzConfig'; import { enableCache, @@ -72,6 +80,9 @@ let isReRender: boolean = false; let didScheduleRenderPhaseUpdate: boolean = false; // Counts the number of useId hooks in this component let localIdCounter: number = 0; +// Chunks that should be pushed to the stream once the component +// finishes rendering. +let bufferedChunks: Array | null = null; // Counts the number of use(thenable) calls in this component let thenableIndexCounter: number = 0; let thenableState: ThenableState | null = null; @@ -208,6 +219,7 @@ export function prepareToUseHooks( // workInProgressHook = null; localIdCounter = 0; + bufferedChunks = null; thenableIndexCounter = 0; thenableState = prevThenableState; } @@ -228,6 +240,7 @@ export function finishHooks( // restarting until no more updates are scheduled. didScheduleRenderPhaseUpdate = false; localIdCounter = 0; + bufferedChunks = null; thenableIndexCounter = 0; numberOfReRenders += 1; @@ -236,6 +249,18 @@ export function finishHooks( children = Component(props, refOrContext); } + + if (bufferedChunks !== null) { + // This component emitted some chunks. They are buffered until it finishes + // rendering successfully. We can flush them now. + const task: Task = (currentlyRenderingTask: any); + const target = task.blockedSegment.chunks; + // Not sure why Flow complains. If I change it to spread instead of apply, + // it checks fine. + // $FlowFixMe[method-unbinding] + target.push.apply(target, bufferedChunks); + } + resetHooksState(); return children; } @@ -559,6 +584,18 @@ function useFormState( ): [S, (P) => void] { resolveCurrentlyRenderingComponent(); + // Emit a marker that indicates whether we rendered using the form state + // passed at the root. We buffer these until the component has finished + // rendering, because a later hook might trigger a local re-render. + // TODO: Matching not yet implemented. For now, we always emit a "not + // matching" marker. + // TODO: As an optimization, Fizz should only emit these markers if form + // state is passed at the root. + if (bufferedChunks === null) { + bufferedChunks = []; + } + pushFormStateMarkerIsNotMatching(bufferedChunks); + // Bind the initial state to the first argument of the action. // TODO: Use the keypath (or permalink) to check if there's matching state // from the previous page. diff --git a/packages/react-server/src/forks/ReactFizzConfig.custom.js b/packages/react-server/src/forks/ReactFizzConfig.custom.js index 73af2ba7cb5c3..8b3e2f8bc348b 100644 --- a/packages/react-server/src/forks/ReactFizzConfig.custom.js +++ b/packages/react-server/src/forks/ReactFizzConfig.custom.js @@ -53,6 +53,10 @@ export const pushStartCompletedSuspenseBoundary = export const pushEndCompletedSuspenseBoundary = $$$config.pushEndCompletedSuspenseBoundary; export const pushSegmentFinale = $$$config.pushSegmentFinale; +export const pushFormStateIsMatchingMarker = + $$$config.pushFormStateIsMatchingMarker; +export const pushFormStateIsNotMatchingMarker = + $$$config.pushFormStateIsNotMatchingMarker; export const writeCompletedRoot = $$$config.writeCompletedRoot; export const writePlaceholder = $$$config.writePlaceholder; export const writeStartCompletedSuspenseBoundary =