facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
229.76k stars 47.05k forks source link

[React 19] Parameter order for server action when using `useActionState` #31356

Open jonathanhefner opened 1 month ago

jonathanhefner commented 1 month ago

Summary

useActionState currently requires accepting an additional state argument as the first parameter of a server action, like so (adapted from Next.js documentation):

// in app/actions.ts

-export async function updateUser(userId: string, formData: FormData) {
+export async function updateUser(prevState: any, userId: string, formData: FormData) {

This interferes with the use case of binding leading arguments to the action (adapted from Next.js documentation):

// in app/client-component.tsx

import { updateUser } from './actions'

const updateUserWithId = updateUser.bind(null, 'some id')
const [state, formAction] = useActionState(updateUserWithId, 'initial state')

For the above example to work, updateUser would need to accept the previous state as the middle parameter:

// in app/actions.ts

-export async function updateUser(prevState: any, userId: string, formData: FormData) {
+export async function updateUser(userId: string, prevState: any, formData: FormData) {

Additionally, the server action might not actually care about the previous state. For example, if the action validates form data and the state is just a validation error message, then the action likely doesn't care about previous error messages. In such cases, it would be nicer DX to be able to omit the state parameter.

Proposal

Make useActionState pass the previous state to the server action as the last argument, always:

// in app/actions.ts

-export async function updateUser(prevState: any, userId: string, formData: FormData) {
+export async function updateUser(userId: string, formData: FormData, prevState: any) {

This might also enable skipping the serialization of the previous state when the server action does not use it (i.e. when action.length < 2).

Additional Information

If the React team agrees to the change, I would like to submit a PR as my first contribution to React. I searched through the codebase for definitions of useActionState, but could not determine how dispatcher.useActionState is implemented. Is that implemented externally?

eps1lon commented 1 month ago

Is bind not working for actions passed to useActionState or is this only about the order of parameters?

This would be a breaking change so the bar for this change is very high.

jonathanhefner commented 1 month ago

Is bind not working for actions passed to useActionState or is this only about the order of parameters?

This would be a breaking change so the bar for this change is very high.

Let me preface by saying: I think form submission and validation will be a major use case of useActionState.

If I have an action f(x1, x2, ..., formData) that processes a form submission and returns validation errors, then, to use useActionState, I must create a new function fState(prevState, x1, x2, ..., formData).

If I want to bind one argument, then I must create another new function fStateBind1(x1, prevState, x2, ..., formData).

If I want to bind two arguments, then I must create another new function fStateBind2(x1, x2, prevState, ..., formData).

The above works, but it is poor DX. It also requires serializing and transmitting state that the action doesn't use.

All of that could be avoided by passing prevState as the final argument.

But, if it is too disruptive to change useActionState, what about adding another hook to address this use case? Something like useFormAction, useActionResult, or useActionOutput?

lokeshdaiya commented 1 week ago

This is the how I have solved in my application. I removed bind method and used like this.

my client component code

    const param =1;
    const [formState, formAction] = useActionState((state:FormState, formData: FormData) => handleSubmit(state, formData, param), INITAIL_STATE, undefined)
    console.log({formState});
    return (
    <form action={formAction}>
        <input name="username" id="username" />
        <button type="submit">Submit</button>
    </form>
    )

server action code

'use server'

export async function handleSubmit(prevState: object, formData: FormData, param?: number) {

    const data = {name: formData.get('username')};
    console.log({prevState})
    console.log({formData})
    console.log({param})
    return {
        ...prevState,
        data: data
    }
}
enzzzooo commented 5 days ago

@lokeshdaiya The nextjs docs list two downsides to this. "the value will be part of the rendered HTML and will not be encoded." and ".bind works in both Server and Client Components. It also supports progressive enhancement." from the docs What do you think of this, like is it important to have the values 'encoded'?

lokeshdaiya commented 4 days ago

@lokeshdaiya The nextjs docs list two downsides to this. "the value will be part of the rendered HTML and will not be encoded." and ".bind works in both Server and Client Components. It also supports progressive enhancement." from the docs What do you think of this, like is it important to have the values 'encoded'?

In the official documentation it talks about .bind method and hidden fields. I have suggested another way which solves the passing additional argument to action methods.

With hidden fields the drawback is it will be part of dom and anybody can see this by inspecting through developer tools.

anthonyalayo commented 3 days ago

Are developers requiring this in order to handle complex forms? I have a multi-step form that is rendered across multiple components, such that it requires storing the state in a controlled manner. Calling action={formAction} naively doesn't pass the form fields that are no longer being rendered, so I need a way of passing it up to the server action. I figured bind() was the right tool for the job here, and ended up on this discussion.