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

🖥️ Support Server Actions #11061

Draft
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

kotarella1110
Copy link
Member

@kotarella1110 kotarella1110 commented Oct 17, 2023

Issue

#10391

Summary

This PR is an experimental implementation for supporting Server Actions in React Hook Form. Its purpose is to explore the best way to support Server Actions within React Hook Form, and it's important to note that the changes proposed in this PR may not be adopted as is.

Additionally, this PR makes minimal changes, and there are various other considerations that need to be addressed.

Example

Server Actions

src/app/schema.ts:

import { z } from 'zod'

export const schema = z.object({
  username: z.string().min(1, "Username is required"),
  password: z.string().min(8, "Password must be at least 8 characters"),
})

src/app/action.ts:

'use server';

import { setTimeout } from 'timers/promises';
import { createActionValidator } from 'react-hook-form/server';
import { zodResolver } from '@hookform/resolvers/zod';

import { schema } from '@/app/schema';

let count = 0;
const maxCount = 1;

export async function login(prevState: any, formData: FormData) {
  await setTimeout(1000);

  const validator = createActionValidator(formData, {
    resolver: zodResolver(schema),
  });

  await validator.validate();

  if (!validator.isValid()) {
    return validator.getResult();
  }

  if (Math.random() < 0.5) {
    validator.setError('root.serverError', {
      type: '409',
      message: 'Username already exists. Please choose a different username.',
    });
    return validator.getResult();
  }

  if (Math.random() < 0.5) {
    count++;
    if (count > maxCount) {
      validator.setError('root.serverError', {
        type: '401',
        message: 'Too many login attempts.',
      });
      return validator.getResult();
    }

    validator.setError('root.serverError', {
      type: '401',
      message: 'Invalid username or password.',
    });
    return validator.getResult();
  }

  if (Math.random() < 0.5) {
    validator.setError('root.serverError', {
      type: '500',
      message: 'Internal server error. Please try again later.',
    });
  }

  return { status: 200, message: 'Login successful!' };
}

src/app/form.tsx:

'use client';

import {
  experimental_useFormState as useFormState,
  experimental_useFormStatus as useFormStatus,
} from 'react-dom';
import { useForm } from 'react-hook-form';
import { zodResolver } from '@hookform/resolvers/zod';

import { login } from '@/app/actions';
import { schema } from '@/app/schema';

function SubmitButton() {
  const { pending } = useFormStatus();
  console.log('pending', pending);

  return <input type="submit" aria-disabled={pending} />;
}

export function LoginForm() {
  const [formState, action] = useFormState(login, {
    values: {
      username: '',
      password: '',
    }
  });
  console.log("formState", formState);

  const {
    register,
    handleSubmit,
    formState: { errors },
  } = useForm<{
    username: string;
    password: string;
  }>({
    resolver: zodResolver(schema),
    defaultValues: formState.values, // Allows field values ​​to persist after form submission when JS is disabled.
    errors: formState.errors,
    progressive: true,
  });

  console.log('errors', errors);

  return (
    <form action={action} onSubmit={handleSubmit()}>
      <label>Username</label>
      <input {...register('username')} placeholder="Username" />
      {errors.username && <p>{errors.username.message}</p>}
      <label>Password</label>
      <input {...register('password')} type="password" placeholder="Password" />
      {errors.password && <p>{errors.password.message}</p>}
      <SubmitButton />
      {errors.root?.serverError && <p>{errors.root.serverError.message}</p>}
    </form>
  );
}

Remix Form

import type { MetaFunction, ActionFunctionArgs } from '@remix-run/node';
import { json } from '@remix-run/node';
import { Form, useActionData } from '@remix-run/react';
import { setTimeout } from 'timers/promises';
import { createActionValidator } from 'react-hook-form/server';
import { useForm } from 'react-hook-form';
import { zodResolver } from '@hookform/resolvers/zod';
import { z } from 'zod';

export const meta: MetaFunction = () => {
  return [
    { title: 'New Remix App' },
    { name: 'description', content: 'Welcome to Remix!' },
  ];
};

const schema = z.object({
  username: z.string().min(1, 'Username is required'),
  password: z.string().min(8, 'Password must be at least 8 characters'),
});

let count = 0;
const maxCount = 1;

export async function action({ request }: ActionFunctionArgs) {
  const formData = await request.formData();

  await setTimeout(1000);

  const validator = createActionValidator(formData, {
    resolver: zodResolver(schema),
  });

  await validator.validate();

  if (!validator.isValid()) {
    return json(validator.getResult());
  }

  if (Math.random() < 0.5) {
    validator.setError('root.serverError', {
      type: '409',
      message: 'Username already exists. Please choose a different username.',
    });
    return json(validator.getResult());
  }

  if (Math.random() < 0.5) {
    count++;
    if (count > maxCount) {
      validator.setError('root.serverError', {
        type: '401',
        message: 'Too many login attempts.',
      });
      return json(validator.getResult());
    }

    validator.setError('root.serverError', {
      type: '401',
      message: 'Invalid username or password.',
    });
    return json(validator.getResult());
  }

  if (Math.random() < 0.5) {
    validator.setError('root.serverError', {
      type: '500',
      message: 'Internal server error. Please try again later.',
    });
  }

  return json({ status: 200, message: 'Login successful!' });
}

export default function Index() {
  const actionData = useActionData<typeof action>();
  console.log('actionData', actionData);

  const {
    register,
    handleSubmit,
    formState: { errors },
  } = useForm<{
    username: string;
    password: string;
  }>({
    resolver: zodResolver(schema),
    defaultValues: actionData?.values || {
      username: '',
      password: '',
    },
    errors: actionData?.errors,
    progressive: true,
  });

  console.log('errors', errors);

  return (
    <Form method="post" onSubmit={handleSubmit()}>
      <label>Username</label>
      <input {...register('username')} placeholder="Username" />
      {errors.username && <p>{errors.username.message}</p>}
      <label>Password</label>
      <input {...register('password')} type="password" placeholder="Password" />
      {errors.password && <p>{errors.password.message}</p>}
      <input type="submit" />
      {errors.root?.serverError && <p>{errors.root.serverError.message}</p>}
    </Form>
  );
}

Two Proposals for Supporting Progressive Enhancement (or Progressive)

Currently, within useForm, there exists an progressive option designed to support Progressive. When this option is enabled, I am contemplating enabling one of the following two proposals. If you prefer not to change the behavior of the existing progressive option, we can also consider adding options such as serverActions.

=> As per #11061 (comment), we have decided to go with Proposal 1.

1. Execute preventDefault only on invalid in handleSubmit

PR

CodeSandbox

URL Example
Server Actions https://codesandbox.io/s/github/react-hook-form/react-hook-form/tree/experiment/support-server-actions/examples/server-actions https://cypnvs-3000.csb.app
Remix Form https://codesandbox.io/s/github/react-hook-form/react-hook-form/tree/experiment/support-server-actions/examples/remix-form https://ccjh85-3000.csb.app

If Remix Form example doesn't work correctly on CodeSandbox, please try it locally.

Pros & Cons

Pros:

  • No need for Custom Invocation with startTransition
  • useFormStatus is working
  • True support for Progressive Enhancement
    • When JavaScript is enabled, the server request becomes asynchronous (form submission doesn't trigger a full page reload)

Cons:

  • Calling preventDefault synchronously within handleSubmit is required only on invalid submissions. If there is asynchronous processing before preventDefault, the browser will execute the submission during that time.
  • Implementing preventDefault synchronously entails more changes, including synchronous execution of native and schema validation processes.
  • Limitations in performing client-side asynchronous validation at the form level in the onSubmit mode (field-level asynchronous validation, such as onChange and onBlur modes, are possible).
    • However, since most asynchronous validation cases involve server calls, it is often more efficient to submit and perform server-side validation, rather than waiting for it.

2. Trigger submit on valid in handleSubmit

PR

CodeSandbox

URL Example
Server Actions https://codesandbox.io/p/sandbox/github/react-hook-form/react-hook-form/tree/experiment/trigger-submit-on-pass/examples/server-actions https://jk5zd8-3000.csb.app/
Remix Form https://codesandbox.io/s/github/react-hook-form/react-hook-form/tree/experiment/support-server-actions/examples/remix-form https://zdqj25-3000.csb.app

If Remix Form example doesn't work correctly on CodeSandbox, please try it locally.

Pros & Cons

Pros:

  • No need for Custom Invocation with startTransition
  • Minimal changes compared to proposal 1
    • Therefore, there is no need to implement preventDefault synchronously

Cons:

  • useFormStatus is not working
  • Provides the same behavior for JavaScript-enabled and JavaScript-disabled scenarios. This means that even when JavaScript is enabled, the form will be submitted with a full page reload, which may not provide an ideal user experience. This may not fully align with the "Enhancement" aspect of Progressive Enhancement.

API

To support Server Actions, the following APIs have been added or changed. If you have better naming or design suggestions for these APIs, please provide your feedback.

New: createActionValidator

This is a validation API for Server Actions or Remix Actions, facilitating consistent validation on both the frontend and backend. The getResult function returns the validation results, including the submitted form values and validation errors. The structure of the values and errors objects aligns with the structure of values and errors handled by React Hook Form, further simplifying integration with useForm.

import { createActionValidator } from 'react-hook-form/server';

// schema validation
const validator = createActionValidator <FormValues>(formData, {
  resolver: zodResolver(schema),
});
await validator.validate();
validator.getResult(); // { values: { ... }, errors: { ... } }

//  build-in validation
const validator = createActionValidator <FormValues>(formData);
await validator
  .register('name', { require: true })
  .register('age', { min: 18, max: 99 })
  .validate();
validator.getResult(); // { values: { ... }, errors: { ... } }

// add custom error
validator.setError('root.serverError', {
  type: '500',
  message: 'Internal server error. Please try again later.',
});

Change: progressive prop

Enabling this prop will activate the features mentioned in the proposals. This is a breaking change. If you want to avoid breaking changes, you can consider adding a new prop like serverActions or a new hook like useActionForm.

import { useForm } from 'react-hook-form';

const {
  register,
  handleSubmit,
  formState: { errors },
} = useForm({ progressive: true });

New: errors prop => DONE: #11188

This prop will react to changes and update the errors, which is useful when your form needs to be updated by external state or server data. You can pass the errors returned from a Server Action to this prop to render error messages regardless of whether JavaScript is enabled or disabled.

The error prop could be beneficial even for users who do not use Server Actions. Therefore, it may be worthwhile to address it separately from Server Actions support and release it in advance.

  import {
    experimental_useFormState as useFormState,
    experimental_useFormStatus as useFormStatus,
  } from 'react-dom';
  import { useForm } from 'react-hook-form';
  
  const [formState, action] = useFormState(login, null);
  const {
    register,
    handleSubmit,
    formState: { errors },
  } = useForm({
+   errors: formState.errors,
    progressive: true,
  });

New: defaultErrors props

=> This prop is not required for Server Actions support. if this prop is needed, it can be included later as well.

This prop allows you to specify default errors. With the addition of the errors prop, the defaultErrors prop has also been added. Note that adding this prop is not mandatory for Server Actions support.

The defaultErrors prop, just like errors prop, it may be worthwhile to address it separately from Server Actions support and release it in advance.

  import {
    experimental_useFormState as useFormState,
    experimental_useFormStatus as useFormStatus,
  } from 'react-dom';
  import { useForm } from 'react-hook-form';
  
  const [formState, action] = useFormState(login, null);
  const {
    register,
    handleSubmit,
    formState: { errors },
  } = useForm({
+   defaultErrors: formState.errors,
    progressive: true,
  });

Change: handleSubmit

In anticipation of an increased use case for not specifying arguments for handleSubmit due to the support of Server Actions, we have made the onValid argument optional.

- <form onSubmit={handleSubmit(onValid)}>
+ <form onSubmit={handleSubmit()}>

Credit

This PR has been made possible with the invaluable advice and support of @koichik. With heartfelt gratitude 💚

Additional Note

This PR makes minimal changes. Therefore, the following tasks will be addressed in the future:

  • Support Server Actions within Form component
    • example: <Form serverAction="" />
  • Support nested form values
  • Optimization of build configurations using esbuild or tsup
  • Adding tests
  • Consideration of how to handle submit-related formState properties such as isSubmitting in React Hook Form
    • Currently, I am not considering integrating useFormStatus and useNavigation into the formState. This is because I haven't come up with a suitable method for integration. If you have any suggestions for a good way to integrate them, please let me know.
  • If Proposal 1 is adopted:
    • Providing synchronous resolvers for all schemas and implementing synchronous native validation
    • Improving types by changing the types of handleSubmit, resolver, and register according to the progressive prop.

@codesandbox
Copy link

codesandbox bot commented Oct 17, 2023

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

@github-actions
Copy link
Contributor

github-actions bot commented Oct 17, 2023

Size Change: +107 B (+1%)

Total Size: 20.4 kB

Filename Size Change
dist/index.cjs.js 10.2 kB +53 B (+1%)
dist/index.umd.js 10.3 kB +54 B (+1%)

compressed-size-action

@kotarella1110 kotarella1110 force-pushed the experiment/support-server-actions branch from 4623505 to 24d2369 Compare October 18, 2023 01:10
@kotarella1110 kotarella1110 force-pushed the experiment/support-server-actions branch from a3375be to 0d12208 Compare October 21, 2023 02:09
@kotarella1110
Copy link
Member Author

kotarella1110 commented Oct 21, 2023

@bluebill1049 @jorisre @Moshyfawn
I have two proposals for supporting Server Actions and Progressive Enhancement. While I tend to prefer Proposal 1 for the best possible user experience, but I would like to hear your opinions.
Additionally, if you have any other thoughts or topics for discussion, please feel free to share.

@Myks92
Copy link

Myks92 commented Oct 21, 2023

Great!) The only thing is there is no example when validation occurs on the API server with an example response:

{    
   "errors": {        
      "chatId": "The value should not be empty."
   }
}

And also combining zodResolver and response api server

@kotarella1110
Copy link
Member Author

kotarella1110 commented Oct 22, 2023

@Myks92 I think it can be implemented as follows. What do you think?

export async function action(prevState: any, formData: FormData) {
  const validator = createActionValidator(formData, {
    resolver: zodResolver(schema),
  });

  await validator.validate();

  const res = await apiRequest();
  // {    
  //.  "errors": {        
  //    "chatId": "The value should not be empty."
  //   }
  // }

  if (res?.errors) {
    res.errors.forEach((message, key) => {
      validator.setError(`root.${key}`, {
        message: 'message',
      });
    });
  }

  if (!validator.isValid()) {
    return validator.getResult();
  }

  return { message: 'Success!' };
}

@Myks92
Copy link

Myks92 commented Oct 22, 2023

@kotarella1110
It seems to me that it is worth looking towards the second resolver, apiResolver and trying on the Chain pattern:

const resolvers = resolverChain([
    zodResolver(schema),
    apiResolver(request)
  ])

  const validator = createValidator(formData, {
    resolvers: resolvers
  })

Or apply this pattern to the Validator itself:

const validator = validators([
   zodValidator(schema),
   apiValidator(request)
])

  await validator.validate(formData)

I would also transfer data validation from createAction Validator to the validate() function:

  await validator.validate(formData)

Total:

Validator Contract

interface Validator {
 validate: (data: any) => Errors
}

Error Contract

interface ValidationError {
  /**
   * Returns the property path from the root element to the violation.
   **/
  path: string
  /**
   * Returns the violation message.
   **/
  message: string
  /**
   * Returns the value that caused the violation.
   **/
  invalidValue: string
}

interface Errors {
  get: (path) => ValidationError
  add: (ValidationError) => void
  remove: (path) => void
  errors: () => ValidationError[]
}

Server Validator

class ServerValidator implements Validator {
  validate(data: any): Errors {
    const errors new Errors()
    const result = new api.post('/users/create', data)
    if (result instanceof ValidationError) {
      result.errors.forEach((error) => {
        errors.add(new ValidationError(error.path,  error.message, error?.invalidValue))
      }
    }
    return errors;
  }
}

Zod Validator

class ZodValidator implements Validator {
  validate(data: any): Errors {
    const errors new Errors()
    //...
    schema.parse(data)
    //...
    return errors;
  }
}

Chain Validator

class ChainValidator implements Validator {
  private validators
  constructor(validators: Validator[]) {
    this.validators = validators
  }
  validate(data: any): Errors {
    const errors new Errors()
    this.validators.forEach(validator => {
      validator.validate().errors().forEach(error => {
        errors.add(error)
      })

    })
    return errors;
  }
}

Use:

const validator = new ChainValidator([new ZodValidator(), new ServerValidator()])
const result = validator.validate({id: 1, name: 2})
console.log(result.errors())

Also, for the validation() function, you can add a second optional parameter for custom rules:

interface Validator {
 validate: (data: any, rules?: Rule[]) => Errors
}

Also, I don't think the validator needs to add global error handling functions from a server like this:

validator.setError('root.serverError', {
   type: '500',
   message: 'Internal server error. Please try again later.',
});

The error does not apply to validation a data. It is important. It seems to me that it is necessary to separate validation from working with other errors. It may be worth separating action processing and validation since these are different responsibilities.

Also, please do not judge strictly for my upper code. I was just trying to convey the idea of the flexibility of Validators, but not to make a full implementation.

There is still something to think about. I don't like everything here. Maybe this will help in some way. In any case, your decision is long-awaited for me. I will be very glad to see this feature.

@kotarella1110
Copy link
Member Author

@Myks92

Thank you for your feedback!

It seems to me that it is worth looking towards the second resolver, apiResolver and trying on the Chain pattern:

I thought it was a great idea to create a utility for composing multiple resolvers (it seems like there's a need for this on the client side as well).

It seems to me that it is necessary to separate validation from working with other errors. It may be worth separating action processing and validation since these are different responsibilities.

It might be valuable to consider separating the responsibilities of action processing and validation as they indeed serve different purposes.
Do you have any specific proposals in mind regarding this matter?

@Myks92
Copy link

Myks92 commented Oct 23, 2023

After thinking about the validation, I thought that it might be related not only to the actions of the server. It can be used to make a request to the server. for example, we make a request to receive something. In total, the uuid was transmitted with an increment of the identifier. Thus, the api can only answer the question of flexible validation. It turns out you can do a more universal thing:

  1. Contract Validator
  2. Validator for Zod
  3. Validator for the Server
  4. Validator Adapter for React hook Form

Then we can use different validators throughout the application.

@kotarella1110
Copy link
Member Author

kotarella1110 commented Oct 24, 2023

@bluebill1049

Option 2 looks really good, but this is a deal breaker. If a page refreshes.

Yes, Option 2 has few changes, but it negatively impacts the UX.

What's the API usage like for build-in validation?

I envision the usage as follows:

action:

export async function login(prevState: any, formData: FormData) {
  const validator = createActionValidator(formData);

  await validator
    .register('username', { required: 'Username is required' })
    .register('password', { {
        minLength: {
          value: 1,
          message: 'Password must be at least 8 characters',
        }
    })
    .validate();

  if (!validator.isValid()) {
    return validator.getResult();
  }
  // ...
}

form:

export function LoginForm() {
  const [formState, action] = useFormState(login, {
    values: {
      username: '',
      password: '',
    }
  });
  const {
    register,
    handleSubmit,
    formState: { errors },
  } = useForm<{
    username: string;
    password: string;
  }>({
    defaultValues: formState.values,
    errors: formState.errors,
    progressive: true,
  });

  return (
    <form action={action} onSubmit={handleSubmit()}>
      <label>Username</label>
      <input {...register('username', { required: 'Username is required' })} placeholder="Username" />
      {errors.username && <p>{errors.username.message}</p>}
      <label>Password</label>
      <input {...register('password', {
          minLength: {
            value: 1,
            message: 'Password must be at least 8 characters',
          }
        })} type="password" placeholder="Password" />
      {errors.password && <p>{errors.password.message}</p>}
      <SubmitButton />
      {errors.root?.serverError && <p>{errors.root.serverError.message}</p>}
    </form>
  );
}

In the context of built-in validation, it would be better if validation logic could be shared between the client and server, just like the schema. However, achieving this would likely require breaking changes.

Do we need both errors and defaultErrors at useForm? or do we need them at all while setError API can be improved?

If we choose Option 1, then 'errors' is a mandatory requirement. If we go with Option 2, either 'errors' or 'defaultErrors' becomes necessary.

When the form submission triggers a page refresh, such as when JavaScript is disabled, you can render error messages by set react-dom's 'formState' as the default value for rhf's errors

errors:
isObject(props.errors) || isObject(props.defaultErrors)
? props.errors || props.defaultErrors || {}
: {},

In cases where the form submission doesn't result in a page refresh, you can render error messages by synchronizing react-dom's formState with rhf's errors.

React.useEffect(() => {
if (props.errors) {
control._setErrors(props.errors);
}
}, [props.errors, control]);

Moreover, to seamlessly integrate react-dom's formState into rhf's errors it's essential to employ the setError API in conjunction with 'useEffect,' like this:

useEffect(() => {
  setError("username", { message: formState.errors.username.message })
}, [setError, formState]);

However, this approach doesn't work when JavaScript is disabled, as 'useEffect' doesn't run on SSR. Therefore, there is a need for errors prop at useForm.

@kotarella1110
Copy link
Member Author

kotarella1110 commented Oct 30, 2023

We discussed which of the two proposals to adopt with @bluebill1049, and after careful consideration, we have decided to go with Proposal 1, "Execute preventDefault only on invalid in handleSubmit". This decision is based on the fact that Proposal 2 would result in a decrease in DX due to page refresh upon submission and the inability to support useFormStatus.

Also, we are considering improvements to the API in order to facilitate the sharing of built-in validations between the server and client. Here's how we plan to enhance the API:

built-in:

import { createValidator } from "@hookform/validators"

const validator = createValidator({ // You may also consider naming it `validator`
  rules: {
    name: { require: true },
    age: { min: 18, max: 99 }
  }
})

// Server
validator.validate(formData)

// Client
useForm({
  + validator, // new
})

schema:

import { createZodValidator } from "@hookform/validators/zod"
const zodValidator = createZodValidator(option) // You may also consider naming it `zodValidator`

// Server
zodValidator.validate(formData)

// Client
useForm({
  - resolver, // deprecate
  + validator: zodValidator, // new
})

combine:

import { combineValidators } from "@hookform/validators"
import { createZodValidator } from "@hookform/validators/zod"

const validator = combineValidators(
  createZodValidator(options),
  createApiValidator(request)
);

validator.validate(formData)

@Myks92
Copy link

Myks92 commented Oct 30, 2023

@kotarella1110, I already like this option. Looks good!

@Myks92
Copy link

Myks92 commented Oct 30, 2023

Perhaps it is also worth giving developers the opportunity to transfer not only FormData, but also any other data to the validator.

const data = {id:1, name: 'User'}
validator.validate(data)
//or
validator.validate(formData)

This can be useful when we want to process data not only FormData.

@kotarella1110
Copy link
Member Author

@Myks92 Yes, I intend to do so 👍

@bluebill1049 bluebill1049 changed the title Support Server Actions 🖥️ Support Server Actions Oct 31, 2023
…e enhancement (#11078)

* feat: execute preventDefault only on invalid

* chore: update example

* chore: update remix-form example

* chore: update remix-form example

* chore: update remix-form example

* chore: update remix-form example

* chore: update remix-form example

* chore: update examples

* chore: update hookform packages
@kotarella1110 kotarella1110 force-pushed the experiment/support-server-actions branch 2 times, most recently from 366b662 to 87f4cb2 Compare November 14, 2023 05:38
@kotarella1110 kotarella1110 force-pushed the experiment/support-server-actions branch from 87f4cb2 to 47d2a0f Compare November 14, 2023 05:56
@AlemTuzlak
Copy link

A bit late to the party but I'm the maintainer of remix-hook-form which handles PE & form actions in Remix, I've been doing this for a while and would be glad to help you guys on this!

@kotarella1110
Copy link
Member Author

@AlemTuzlak

Thank you for your comment. I've thoroughly reviewed the code of remix-hook-form.
Great job on the RHF wrapper! It's well done!
What we aim to achieve with this PR is similar to remix-hook-form: Server Actions, Remix Form, and Progressive Enhancement.
However, it's still in the consideration stage. After reviewing this PR, do you have any opinions?
Also, I'm still pondering how to design the server-side API.
I would like to provide a validation API similar to validateFormData in remix-hook-form within RHF. Any advice on this matter would be appreciated.

@arjunyel
Copy link

@kotarella1110 if you put out a preview release with this change I can try it out in my Remix project

@AlemTuzlak
Copy link

AlemTuzlak commented Dec 21, 2023

Sorry in advance for the long comment @kotarella1110

Alright so I'll write a very detailed overview of everything I've figured out from the point I made it to the current release. I'd like to preface this that this is only valid for web standard API's and I have no idea what Next does, I know they like to add custom stuff on top of the web platform so I can't guarantee this will make much sense in that context.

Progressive Enhancement

So for progressive enhancement use-cases here is a rough overview on what happens with form submissions without JS enabled:

  • User enters form values
  • User submits form values
  • Request fires off with form data to some server endpoint by creating a browser form submit event with either a GET or POST
  • The browser redirects you to the action with the specified verb and stores the data in search parameters
  • The action gets the request and either has to get them from the formData object from the request OR has to manually parse the values from the search params in case of a GET request.
  • The action does something
  • User gets a response

Error handling

So let's assume the the action does something returns errors. Well in this case we need to show these errors to the user. From what I've seen you guys added the errors prop to the useForm hook so I'd assume if the same format is provided this will work out of the box.

Preserving old values

So here is the interesting part, if you submit something to the action and that gets validated and the errors are returned, this will work but there is a big problem when it comes to PE, because we did a native browser submission the values that were submitted are blown away and we have to re-enter everything we already entered. This is where we can do the following:

  • Collect all the data that was submitted on the server
  • Validate the data
  • Return the original data + the validation errors

Once we return the initial data we can either provide it to the hook as defaultValues or what I decided to do was extend the register function to add it as the defaultValue and allow the user to turn it on/off per field because somewhere if you want a controlled input you would get an error message from react that a controlled input can't have a default value error message. But in any case this ensures when there are validation errors we have preserved whatever the user has selected until that point.
You can find how I did that here, the example on how it works would basically be:

 <input {...register("input", { disableProgressiveEnhancement: true }) />

I think you need to provide a per value lever to be pulled here OR be smarter about it and figure out if something is controlled or not and handle it internally, in any case the point stands.

Parsing the data

So from the above scenario we notice the following two subscenarios when it comes to parsing the data:

  • We either have to parse it from the GET request from the search params
  • We can directly access it through the request.formData in case of a POST request

I've made the following observation:

 // This generates an identical object as doing // request.formData
 const searchParams = new URL(request.url).searchParams;

So your options are the following:

  • Ask the user to provide one or the other and parse it yourself depending on the request verb (This is the approach I chose and you can see the implementation here )
  • Create two utilities, one to be used for a GET and one for a POST.

Serialization

As you might have already known the FormData object only takes two types of values, strings or files. This means when you're sending data to the server you have two options:

  • Stringify EVERYTHING
  • Stringify EVERYTHING BESIDES STRINGS

So both of these have their pros/cons and I will go into further detail on everything you need to be aware of for these approaches.

Stringify everything

So this approach is pretty simple, you stringify everything, even the strings. So this might look weird at first but here are the

Pros:

  • Type safety => because everything is serialized when you try to parse everything back on the server back to original values you will get correct types for everything, strings will be strings, numbers will be numbers, booleans will be booleans and so on.
  • Ease of use => this approach is far easier to use for the end-user because he has nothing to do here, his values are the same as they were on the client, you just re-validate them and you're good to go on the server

Cons:

  • Potentially hurts progressive enhancement => The issue with this is that everything will be stringifed, this means that the url search params will have strings with quotes in them which might cause issues in some cases, eg the value you'd expect is: string but you will get a "string"
  • Hurts the user if he doesn't use provided methods to parse the form data => if the user manually parses form data and tries to use it he will have issues because of the above mentioned issue, the strings are double quoted

So when do we want to use this approach:

  • User relies on the library to do everything, doesn't care much about double quoted strings as he doesn't do anything custom with it
  • Double quoted strings are not ap roblem in the url

Benefits

Really simple and easy to use.
No need for the user to coerce the types

Stringify everything besides strings

So this approach differs from the above mentioned one in one key difference, we don't stringify strings.

Pros:

  • The strings are not double quoted inside
  • More aligned to web standards

Cons

  • Lost type-safety => you don't know if the value used to be a string or a number or a boolean
  • False positives => if you try to convert strings back to numbers and booleans and other types there are false positives, eg: password: "1234" => password: 1234 (we want it to be a string because that's what we expect)
  • Type coercion in validation => user will have to coerce the types himself in the validation schema, otherwise the schema will throw errors that string is not a valid number and so on.

Benefits

Aligned with web standards

Other concerns

The biggest concern in all of this is what happens with the following formState values:

  • isSubmitSuccessful => when is this actually successful?
  • isSubmitted, => when is the form considered submitted?
  • isSubmitting => when is this true/false

I've made all of the above work in remix-hook-form but they are tied to Remix obviously, but all of the things I wrote are web standard related and things to figure out to bring true PE + server actions and validations.

IMO what needs to happen on the server is the following:

  • Have identical way to build validation be/fe with the resolver
  • Have a way to parse the form data and do what's needed (parse it => validate it =>return it)
  • Give the user the option to opt into as much as he wants (he either gives you the request, the form data, some custom object he created after using the formdata and parsing it himself) and they all work
  • Have a way to add custom errors
  • Make it as simple as possible for the end-user

PR review and my thoughts

I would maybe consider having the validator.validate() return the result instead of having to do isValid and getResult, make it a discriminated union so what users can do is:

const { data, errors, defaultValues } = validator.validate();
if(errors){
  // default values returned to frontend for PE
  return json({ errors, defaultValues });
}
// Fully type-safe data here 
 data.value => string

This would allow the most common use case to be the most simple, because most cases require you to only validate the data and not set custom fields (my experience from writing 100+ forms in remix). If you want something more advanced then you set the errors on the validator and you're good, although if you used the pattern of merging validators and had the validator actually do async checks, eg password validator that checks your db and validates then you don't even need the setError stuff.

From the above writing you would also need to probably extend the useForm hook to handle the things I talked about.

If you wish to discuss this @kotarella1110 feel free to message me on twitter or anywhere.

@arjunyel
Copy link

One issue with falling back to GET requests is this would bypass CSRF protection. So personally in my apps I would want to disable this feature even at the cost of progressive enhancement

@kotarella1110
Copy link
Member Author

@arjunyel
Thank you for your support!
However, please PR that this change is still in the experimental stage, and we are not currently considering a preview release.
If you wish to try it out, you can install the package from CodeSandbox CI and conduct the testing.
https://ci.codesandbox.io/status/react-hook-form/react-hook-form/pr/11061/builds/447043

@kotarella1110
Copy link
Member Author

kotarella1110 commented Dec 23, 2023

@AlemTuzlak

Thank you very much for providing valuable information.

Once we return the initial data we can either provide it to the hook as defaultValues or what I decided to do was extend the register function to add it as the defaultValue and allow the user to turn it on/off per field because somewhere if you want a controlled input you would get an error message from react that a controlled input can't have a default value error message.

I have taken this into consideration in this PR as well, assuming that users would use the defaultValues of useForm. Also, rather than providing an option like disableProgressiveEnhancement in the register API for applying original data per field, I thought it would be better to simply have users pass the defaultValue prop to the field. What do you think?

const [formState, action] = useFormState(myAction, {
    values: {}
  });
 <input {...register("input") defaultValue={formState.values?.["input"]} />

We either have to parse it from the GET request from the search params

Until I read the code of remix-hook-form, I completely overlooked the consideration for GET requests. Thank you for bringing this to my attention. I would like to support it similarly to remix-hook-form.

FormData object only takes two types of values, strings or files.

While I have knowledge about this, it was not considered at all in this PR. In the case of Remix, it provides hooks such as useSubmit to control the timing of submission. However, currently, React (Server Actions) does not provide such hooks. Therefore, if my understanding is correct, it is impossible to serialize FormData before requesting it to the server to support Progressive Enhancement. Hence, I believe we have no choice but to choose the "Stringify everything besides strings" approach. What we can do is provide an option for users to parse FormData at server-side validation.

The biggest concern in all of this is what happens with the following formState values

As mentioned in the "Additional Note" of this PR overview, we need to discuss how to implement this in the future.

If you wish to discuss this @kotarella1110 feel free to message me on twitter or anywhere.

Thank you. Let's discuss it here for now to keep the conversation as open as possible! I will contact you individually if needed 😚

@AlemTuzlak
Copy link

I have taken this into consideration in this PR as well, assuming that users would use the defaultValues of useForm. Also, rather than providing an option like disableProgressiveEnhancement in the register API for applying original data per field, I thought it would be better to simply have users pass the defaultValue prop to the field. What do you think?

This could also work, but this means the user has to set the progressive enhancement up rather than the library doing it for him behind the scenes, it's very hard to say what the right approach here is because you either abstract it and make it simpler but then risking running into problems with controlled values or ask the user to set the value on every field, which could be a hassle. I can't really say if I prefer one or the other, from the users point of view I'd like to have everything set up for me but from the maintainer point of view I can see issues coming in that people get react errors when they register a component.

While I have knowledge about this, it was not considered at all in this PR. In the case of Remix, it provides hooks such as useSubmit to control the timing of submission. However, currently, React (Server Actions) does not provide such hooks. Therefore, if my understanding is correct, it is impossible to serialize FormData before requesting it to the server to support Progressive Enhancement. Hence, I believe we have no choice but to choose the "Stringify everything besides strings" approach. What we can do is provide an option for users to parse FormData at server-side validation.

That is correct. Even though Remix provides this you are still not guaranteed that the user will serialize it himself. Nothing stopping me from just using Remix's Form component without any sort of custom submission logic. And even if the data is serialized you could get it in differently if JS is not enabled or hasn't loaded and the form is submitted, in that case you would have to fall back to plain formData object and parsing that so I think the second stringify everything but strings approach is what you have to go with.

My biggest gripe with that is the "false positives", you would have to force the user to coerce types into correct ones with their validation libraries. Not a big deal with new projects but a big deal with projects moving onto react-hook-form with server actions because suddenly their z.number() isn't working on the server anymore because the serialized value is a string. And if you try to parse it in the library you could end up parsing a string number (eg "123456" => supposed to be a string type) into numbers and then z.string() fails. I think getting this right is the biggest pain point.
I was thinking of trying to maybe be smart about it, try to parse every value and then test the validation with it being a string/number but this would cause too much validation overhead.

Copy link

stale bot commented Mar 13, 2024

Thank you for your contributions! This Pull Request has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Best, RHF Team ❤️

@stale stale bot added the stale label Mar 13, 2024
@stale stale bot removed the stale label Apr 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants