Skip to content

Commit 95c9554

Browse files
authored
useFormState: Compare action signatures when reusing form state (#27370)
During an MPA form submission, useFormState should only reuse the form state if same action is passed both times. (We also compare the key paths.) We compare the identity of the inner closure function, disregarding the value of the bound arguments. That way you can pass an inline Server Action closure: ```js function FormContainer({maxLength}) { function submitAction(prevState, formData) { 'use server' if (formData.get('field').length > maxLength) { return { errorMsg: 'Too many characters' }; } // ... } return <Form submitAction={submitAction} /> } ```
1 parent 612b2b6 commit 95c9554

14 files changed

+377
-89
lines changed

packages/react-client/src/ReactFlightReplyClient.js

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
import type {
1111
Thenable,
12+
PendingThenable,
1213
FulfilledThenable,
1314
RejectedThenable,
1415
ReactCustomFormAction,
@@ -489,6 +490,69 @@ export function encodeFormAction(
489490
};
490491
}
491492

493+
function isSignatureEqual(
494+
this: any => Promise<any>,
495+
referenceId: ServerReferenceId,
496+
numberOfBoundArgs: number,
497+
): boolean {
498+
const reference = knownServerReferences.get(this);
499+
if (!reference) {
500+
throw new Error(
501+
'Tried to encode a Server Action from a different instance than the encoder is from. ' +
502+
'This is a bug in React.',
503+
);
504+
}
505+
if (reference.id !== referenceId) {
506+
// These are different functions.
507+
return false;
508+
}
509+
// Now check if the number of bound arguments is the same.
510+
const boundPromise = reference.bound;
511+
if (boundPromise === null) {
512+
// No bound arguments.
513+
return numberOfBoundArgs === 0;
514+
}
515+
// Unwrap the bound arguments array by suspending, if necessary. As with
516+
// encodeFormData, this means isSignatureEqual can only be called while React
517+
// is rendering.
518+
switch (boundPromise.status) {
519+
case 'fulfilled': {
520+
const boundArgs = boundPromise.value;
521+
return boundArgs.length === numberOfBoundArgs;
522+
}
523+
case 'pending': {
524+
throw boundPromise;
525+
}
526+
case 'rejected': {
527+
throw boundPromise.reason;
528+
}
529+
default: {
530+
if (typeof boundPromise.status === 'string') {
531+
// Only instrument the thenable if the status if not defined.
532+
} else {
533+
const pendingThenable: PendingThenable<Array<any>> =
534+
(boundPromise: any);
535+
pendingThenable.status = 'pending';
536+
pendingThenable.then(
537+
(boundArgs: Array<any>) => {
538+
const fulfilledThenable: FulfilledThenable<Array<any>> =
539+
(boundPromise: any);
540+
fulfilledThenable.status = 'fulfilled';
541+
fulfilledThenable.value = boundArgs;
542+
},
543+
(error: mixed) => {
544+
const rejectedThenable: RejectedThenable<number> =
545+
(boundPromise: any);
546+
rejectedThenable.status = 'rejected';
547+
rejectedThenable.reason = error;
548+
},
549+
);
550+
}
551+
throw boundPromise;
552+
}
553+
}
554+
}
555+
492556
export function registerServerReference(
493557
proxy: any,
494558
reference: {id: ServerReferenceId, bound: null | Thenable<Array<any>>},
@@ -499,6 +563,7 @@ export function registerServerReference(
499563
// Only expose this in builds that would actually use it. Not needed on the client.
500564
Object.defineProperties((proxy: any), {
501565
$$FORM_ACTION: {value: encodeFormAction},
566+
$$IS_SIGNATURE_EQUAL: {value: isSignatureEqual},
502567
bind: {value: bind},
503568
});
504569
}

packages/react-dom/src/client/ReactDOMRoot.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ export type HydrateRootOptions = {
5757
unstable_transitionCallbacks?: TransitionTracingCallbacks,
5858
identifierPrefix?: string,
5959
onRecoverableError?: (error: mixed) => void,
60-
experimental_formState?: ReactFormState<any> | null,
60+
experimental_formState?: ReactFormState<any, any> | null,
6161
...
6262
};
6363

packages/react-dom/src/server/ReactDOMFizzServerBrowser.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ type Options = {
4141
onPostpone?: (reason: string) => void,
4242
unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor,
4343
importMap?: ImportMap,
44-
experimental_formState?: ReactFormState<any> | null,
44+
experimental_formState?: ReactFormState<any, any> | null,
4545
};
4646

4747
type ResumeOptions = {

packages/react-dom/src/server/ReactDOMFizzServerBun.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ type Options = {
3939
onPostpone?: (reason: string) => void,
4040
unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor,
4141
importMap?: ImportMap,
42-
experimental_formState?: ReactFormState<any> | null,
42+
experimental_formState?: ReactFormState<any, any> | null,
4343
};
4444

4545
// TODO: Move to sub-classing ReadableStream.

packages/react-dom/src/server/ReactDOMFizzServerEdge.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ type Options = {
4141
onPostpone?: (reason: string) => void,
4242
unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor,
4343
importMap?: ImportMap,
44-
experimental_formState?: ReactFormState<any> | null,
44+
experimental_formState?: ReactFormState<any, any> | null,
4545
};
4646

4747
type ResumeOptions = {

packages/react-dom/src/server/ReactDOMFizzServerNode.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ type Options = {
5454
onPostpone?: (reason: string) => void,
5555
unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor,
5656
importMap?: ImportMap,
57-
experimental_formState?: ReactFormState<any> | null,
57+
experimental_formState?: ReactFormState<any, any> | null,
5858
};
5959

6060
type ResumeOptions = {

packages/react-reconciler/src/ReactFiberReconciler.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ export function createHydrationContainer(
281281
identifierPrefix: string,
282282
onRecoverableError: (error: mixed) => void,
283283
transitionCallbacks: null | TransitionTracingCallbacks,
284-
formState: ReactFormState<any> | null,
284+
formState: ReactFormState<any, any> | null,
285285
): OpaqueRoot {
286286
const hydrate = true;
287287
const root = createFiberRoot(

packages/react-reconciler/src/ReactFiberRoot.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ function FiberRootNode(
5252
hydrate: any,
5353
identifierPrefix: any,
5454
onRecoverableError: any,
55-
formState: ReactFormState<any> | null,
55+
formState: ReactFormState<any, any> | null,
5656
) {
5757
this.tag = tag;
5858
this.containerInfo = containerInfo;
@@ -145,7 +145,7 @@ export function createFiberRoot(
145145
identifierPrefix: string,
146146
onRecoverableError: null | ((error: mixed) => void),
147147
transitionCallbacks: null | TransitionTracingCallbacks,
148-
formState: ReactFormState<any> | null,
148+
formState: ReactFormState<any, any> | null,
149149
): FiberRoot {
150150
// $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
151151
const root: FiberRoot = (new FiberRootNode(

packages/react-reconciler/src/ReactInternalTypes.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ type BaseFiberRootProperties = {
272272
errorInfo: {digest?: ?string, componentStack?: ?string},
273273
) => void,
274274

275-
formState: ReactFormState<any> | null,
275+
formState: ReactFormState<any, any> | null,
276276
};
277277

278278
// The following attributes are only used by DevTools and are only present in DEV builds.

packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMForm-test.js

Lines changed: 169 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ describe('ReactFlightDOMForm', () => {
6767
webpackServerMap,
6868
);
6969
const returnValue = boundAction();
70-
const formState = ReactServerDOMServer.decodeFormState(
70+
const formState = await ReactServerDOMServer.decodeFormState(
7171
await returnValue,
7272
formData,
7373
webpackServerMap,
@@ -435,6 +435,174 @@ describe('ReactFlightDOMForm', () => {
435435
}
436436
});
437437

438+
// @gate enableFormActions
439+
// @gate enableAsyncActions
440+
it(
441+
'useFormState preserves state if arity is the same, but different ' +
442+
'arguments are bound (i.e. inline closure)',
443+
async () => {
444+
const serverAction = serverExports(async function action(
445+
stepSize,
446+
prevState,
447+
formData,
448+
) {
449+
return prevState + stepSize;
450+
});
451+
452+
function Form({action}) {
453+
const [count, dispatch] = useFormState(action, 1);
454+
return <form action={dispatch}>{count}</form>;
455+
}
456+
457+
function Client({action}) {
458+
return (
459+
<div>
460+
<Form action={action} />
461+
<Form action={action} />
462+
<Form action={action} />
463+
</div>
464+
);
465+
}
466+
467+
const ClientRef = await clientExports(Client);
468+
469+
const rscStream = ReactServerDOMServer.renderToReadableStream(
470+
// Note: `.bind` is the same as an inline closure with 'use server'
471+
<ClientRef action={serverAction.bind(null, 1)} />,
472+
webpackMap,
473+
);
474+
const response = ReactServerDOMClient.createFromReadableStream(rscStream);
475+
const ssrStream = await ReactDOMServer.renderToReadableStream(response);
476+
await readIntoContainer(ssrStream);
477+
478+
expect(container.textContent).toBe('111');
479+
480+
// There are three identical forms. We're going to submit the second one.
481+
const form = container.getElementsByTagName('form')[1];
482+
const {formState} = await submit(form);
483+
484+
// Simulate an MPA form submission by resetting the container and
485+
// rendering again.
486+
container.innerHTML = '';
487+
488+
// On the next page, the same server action is rendered again, but with
489+
// a different bound stepSize argument. We should treat this as the same
490+
// action signature.
491+
const postbackRscStream = ReactServerDOMServer.renderToReadableStream(
492+
// Note: `.bind` is the same as an inline closure with 'use server'
493+
<ClientRef action={serverAction.bind(null, 5)} />,
494+
webpackMap,
495+
);
496+
const postbackResponse =
497+
ReactServerDOMClient.createFromReadableStream(postbackRscStream);
498+
const postbackSsrStream = await ReactDOMServer.renderToReadableStream(
499+
postbackResponse,
500+
{experimental_formState: formState},
501+
);
502+
await readIntoContainer(postbackSsrStream);
503+
504+
// The state should have been preserved because the action signatures are
505+
// the same. (Note that the amount increased by 1, because that was the
506+
// value of stepSize at the time the form was submitted)
507+
expect(container.textContent).toBe('121');
508+
509+
// Now submit the form again. This time, the state should increase by 5
510+
// because the stepSize argument has changed.
511+
const form2 = container.getElementsByTagName('form')[1];
512+
const {formState: formState2} = await submit(form2);
513+
514+
container.innerHTML = '';
515+
516+
const postbackRscStream2 = ReactServerDOMServer.renderToReadableStream(
517+
// Note: `.bind` is the same as an inline closure with 'use server'
518+
<ClientRef action={serverAction.bind(null, 5)} />,
519+
webpackMap,
520+
);
521+
const postbackResponse2 =
522+
ReactServerDOMClient.createFromReadableStream(postbackRscStream2);
523+
const postbackSsrStream2 = await ReactDOMServer.renderToReadableStream(
524+
postbackResponse2,
525+
{experimental_formState: formState2},
526+
);
527+
await readIntoContainer(postbackSsrStream2);
528+
529+
expect(container.textContent).toBe('171');
530+
},
531+
);
532+
533+
// @gate enableFormActions
534+
// @gate enableAsyncActions
535+
it('useFormState does not reuse state if action signatures are different', async () => {
536+
// This is the same as the previous test, except instead of using bind to
537+
// configure the server action (i.e. a closure), it swaps the action.
538+
const increaseBy1 = serverExports(async function action(
539+
prevState,
540+
formData,
541+
) {
542+
return prevState + 1;
543+
});
544+
545+
const increaseBy5 = serverExports(async function action(
546+
prevState,
547+
formData,
548+
) {
549+
return prevState + 5;
550+
});
551+
552+
function Form({action}) {
553+
const [count, dispatch] = useFormState(action, 1);
554+
return <form action={dispatch}>{count}</form>;
555+
}
556+
557+
function Client({action}) {
558+
return (
559+
<div>
560+
<Form action={action} />
561+
<Form action={action} />
562+
<Form action={action} />
563+
</div>
564+
);
565+
}
566+
567+
const ClientRef = await clientExports(Client);
568+
569+
const rscStream = ReactServerDOMServer.renderToReadableStream(
570+
<ClientRef action={increaseBy1} />,
571+
webpackMap,
572+
);
573+
const response = ReactServerDOMClient.createFromReadableStream(rscStream);
574+
const ssrStream = await ReactDOMServer.renderToReadableStream(response);
575+
await readIntoContainer(ssrStream);
576+
577+
expect(container.textContent).toBe('111');
578+
579+
// There are three identical forms. We're going to submit the second one.
580+
const form = container.getElementsByTagName('form')[1];
581+
const {formState} = await submit(form);
582+
583+
// Simulate an MPA form submission by resetting the container and
584+
// rendering again.
585+
container.innerHTML = '';
586+
587+
// On the next page, a different server action is rendered. It should not
588+
// reuse the state from the previous page.
589+
const postbackRscStream = ReactServerDOMServer.renderToReadableStream(
590+
<ClientRef action={increaseBy5} />,
591+
webpackMap,
592+
);
593+
const postbackResponse =
594+
ReactServerDOMClient.createFromReadableStream(postbackRscStream);
595+
const postbackSsrStream = await ReactDOMServer.renderToReadableStream(
596+
postbackResponse,
597+
{experimental_formState: formState},
598+
);
599+
await readIntoContainer(postbackSsrStream);
600+
601+
// The state should not have been preserved because the action signatures
602+
// are not the same.
603+
expect(container.textContent).toBe('111');
604+
});
605+
438606
// @gate enableFormActions
439607
// @gate enableAsyncActions
440608
it('useFormState can change the action URL with the `permalink` argument', async () => {

0 commit comments

Comments
 (0)