Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

useFormState: Emit comment to mark whether state matches #27307

Merged
merged 1 commit into from
Sep 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 45 additions & 2 deletions packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ import {
enableTrustedTypesIntegration,
diffInCommitPhase,
enableFormActions,
enableAsyncActions,
} from 'shared/ReactFeatureFlags';
import {
HostComponent,
Expand Down Expand Up @@ -160,7 +161,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,
Expand All @@ -187,6 +193,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';

Expand Down Expand Up @@ -1283,6 +1291,37 @@ export function registerSuspenseInstanceRetry(
instance._reactRetry = callback;
}

export function canHydrateFormStateMarker(
instance: HydratableInstance,
inRootOrSingleton: boolean,
): null | FormStateMarkerInstance {
while (instance.nodeType !== COMMENT_NODE) {
if (!inRootOrSingleton || !enableHostSingletons) {
return null;
}
const nextInstance = getNextHydratableSibling(instance);
if (nextInstance === null) {
return null;
}
instance = nextInstance;
}
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) {
Expand All @@ -1295,7 +1334,11 @@ function getNextHydratable(node: ?Node) {
if (
nodeData === SUSPENSE_START_DATA ||
nodeData === SUSPENSE_FALLBACK_START_DATA ||
nodeData === SUSPENSE_PENDING_START_DATA
nodeData === SUSPENSE_PENDING_START_DATA ||
(enableFormActions &&
enableAsyncActions &&
(nodeData === FORM_STATE_IS_MATCHING ||
nodeData === FORM_STATE_IS_NOT_MATCHING))
) {
break;
}
Expand Down
15 changes: 15 additions & 0 deletions packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -1519,6 +1519,21 @@ function injectFormReplayingRuntime(
}
}

const formStateMarkerIsMatching = stringToPrecomputedChunk('<!--F!-->');
const formStateMarkerIsNotMatching = stringToPrecomputedChunk('<!--F-->');

export function pushFormStateMarkerIsMatching(
target: Array<Chunk | PrecomputedChunk>,
) {
target.push(formStateMarkerIsMatching);
}

export function pushFormStateMarkerIsNotMatching(
target: Array<Chunk | PrecomputedChunk>,
) {
target.push(formStateMarkerIsNotMatching);
}

function pushStartForm(
target: Array<Chunk | PrecomputedChunk>,
props: Object,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ export {
pushEndInstance,
pushStartCompletedSuspenseBoundary,
pushEndCompletedSuspenseBoundary,
pushFormStateMarkerIsMatching,
pushFormStateMarkerIsNotMatching,
writeStartSegment,
writeEndSegment,
writeCompletedSegmentInstruction,
Expand Down
119 changes: 119 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ let SuspenseList;
let useSyncExternalStore;
let useSyncExternalStoreWithSelector;
let use;
let useFormState;
let PropTypes;
let textCache;
let writable;
Expand Down Expand Up @@ -88,6 +89,7 @@ describe('ReactDOMFizzServer', () => {
if (gate(flags => flags.enableSuspenseList)) {
SuspenseList = React.unstable_SuspenseList;
}
useFormState = ReactDOM.experimental_useFormState;

PropTypes = require('prop-types');

Expand Down Expand Up @@ -5876,6 +5878,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 (
<div id="child" ref={childRef}>
{text}
</div>
);
}

function App() {
return (
<div>
<div>
<Form />
</div>
<span>Sibling</span>
</div>
);
}

await act(() => {
const {pipe} = renderToPipeableStream(<App />);
pipe(writable);
});
expect(getVisibleChildren(container)).toEqual(
<div>
<div>
<div id="child">Child: 0</div>
</div>
<span>Sibling</span>
</div>,
);
const child = document.getElementById('child');

// Confirm that it hydrates correctly
await clientAct(() => {
ReactDOMClient.hydrateRoot(container, <App />);
});
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 (
<div id="child" ref={childRef}>
{text}
</div>
);
}

function App() {
return (
<div>
<Suspense fallback="Loading...">
<Form />
</Suspense>
<span>Sibling</span>
</div>
);
}

await act(() => {
const {pipe} = renderToPipeableStream(<App />);
pipe(writable);
});
expect(getVisibleChildren(container)).toEqual(
<div>
Loading...<span>Sibling</span>
</div>,
);

await act(() => resolveText('Child'));
expect(getVisibleChildren(container)).toEqual(
<div>
<div id="child">Child:0:3</div>
<span>Sibling</span>
</div>,
);
const child = document.getElementById('child');

// Confirm that it hydrates correctly
await clientAct(() => {
ReactDOMClient.hydrateRoot(container, <App />);
});
expect(childRef.current).toBe(child);
});

describe('useEffectEvent', () => {
// @gate enableUseEffectEventHook
it('can server render a component with useEffectEvent', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
14 changes: 12 additions & 2 deletions packages/react-reconciler/src/ReactFiberHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,10 @@ import {
markWorkInProgressReceivedUpdate,
checkIfWorkInProgressReceivedUpdate,
} from './ReactFiberBeginWork';
import {getIsHydrating} from './ReactFiberHydrationContext';
import {
getIsHydrating,
tryToClaimNextHydratableFormMarkerInstance,
} from './ReactFiberHydrationContext';
import {logStateUpdateScheduled} from './DebugTracing';
import {
markStateUpdateScheduled,
Expand Down Expand Up @@ -2010,6 +2013,12 @@ function mountFormState<S, P>(
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);
}
Comment on lines +2016 to +2020
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hydrating gate and the one in tryToClaim... are redundant. I assume there will be other logic in here so probably can drop the one in tryTo...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I did it that way to match the other similar functions in that file, which all have the guard at the top and also is redundant because the callers usually end up checking it regardless. Usually if I'm not the one who wrote a module I copy the style of the surrounding code for 1) consistency because I figure if I fix it here I should also fix it everywhere else 2) out of caution in case there was a good reason that I'm not thinking of :D


// State hook. The state is stored in a thenable which is then unwrapped by
// the `use` algorithm during render.
const stateHook = mountWorkInProgressHook();
Expand Down Expand Up @@ -2145,7 +2154,8 @@ function rerenderFormState<S, P>(
}

// This is a mount. No updates to process.
const state = stateHook.memoizedState;
const thenable: Thenable<S> = stateHook.memoizedState;
const state = useThenable(thenable);

const actionQueueHook = updateWorkInProgressHook();
const actionQueue = actionQueueHook.queue;
Expand Down
30 changes: 30 additions & 0 deletions packages/react-reconciler/src/ReactFiberHydrationContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ import {
canHydrateInstance,
canHydrateTextInstance,
canHydrateSuspenseInstance,
canHydrateFormStateMarker,
isFormStateMarkerMatching,
isHydratableText,
} from './ReactFiberConfig';
import {OffscreenLane} from './ReactFiberLane';
Expand Down Expand Up @@ -595,6 +597,34 @@ function tryToClaimNextHydratableSuspenseInstance(fiber: Fiber): void {
}
}

export function tryToClaimNextHydratableFormMarkerInstance(
fiber: Fiber,
): boolean {
if (!isHydrating) {
return false;
}
if (nextHydratableInstance) {
const markerInstance = canHydrateFormStateMarker(
nextHydratableInstance,
rootOrSingletonContext,
);
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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ describe('ReactFlightDOMForm', () => {
const ssrStream = await ReactDOMServer.renderToReadableStream(response);
await readIntoContainer(ssrStream);

const form = container.firstChild;
const form = container.getElementsByTagName('form')[0];
const span = container.getElementsByTagName('span')[0];
expect(span.textContent).toBe('Count: 1');

Expand Down Expand Up @@ -382,7 +382,7 @@ describe('ReactFlightDOMForm', () => {
const ssrStream = await ReactDOMServer.renderToReadableStream(response);
await readIntoContainer(ssrStream);

const form = container.firstChild;
const form = container.getElementsByTagName('form')[0];
const span = container.getElementsByTagName('span')[0];
expect(span.textContent).toBe('Count: 1');

Expand Down Expand Up @@ -423,7 +423,7 @@ describe('ReactFlightDOMForm', () => {
const ssrStream = await ReactDOMServer.renderToReadableStream(response);
await readIntoContainer(ssrStream);

const form = container.firstChild;
const form = container.getElementsByTagName('form')[0];
const span = container.getElementsByTagName('span')[0];
expect(span.textContent).toBe('Count: 1');

Expand Down
Loading