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

Mantine Form Restricts Server Action Support With Client & Server Validation #7142

Closed
1 of 2 tasks
anthonyalayo opened this issue Nov 20, 2024 · 8 comments
Closed
1 of 2 tasks
Labels
Fixed patch Completed issues that will be published with next patch (1.0.X)

Comments

@anthonyalayo
Copy link
Contributor

anthonyalayo commented Nov 20, 2024

Dependencies check up

  • I have verified that I use latest version of all @mantine/* packages

What version of @mantine/* packages do you have in package.json?

7.13.5

What package has an issue?

@mantine/form

What framework do you use?

Next.js

In which browsers you can reproduce the issue?

All

Describe the bug

When attempting to perform both client side and server side validation with @mantine/form and server actions, the current API disallows this from working. Without modifications, you cannot call event?.currentTarget.requestSubmit() from the callback of form.onSubmit() after placing an action returned from useActionState in the action prop as required.

For comparison, this is how conform allows form submission with both client side and server side validation:
https://conform.guide/integration/nextjs

With the relevant snippet of code here:

export function LoginForm() {
  const [lastResult, action] = useFormState(login, undefined);
  const [form, fields] = useForm({
    // Sync the result of last submission
    lastResult,

    // Reuse the validation logic on the client
    onValidate({ formData }) {
      return parseWithZod(formData, { schema: loginSchema });
    },

    // Validate the form on blur event triggered
    shouldValidate: 'onBlur',
    shouldRevalidate: 'onInput',
  });

  return (
    <form id={form.id} onSubmit={form.onSubmit} action={action} noValidate>

With this configuration, server actions works seamlessly.

If possible, include a link to a codesandbox with a minimal reproduction

No response

Possible fix

The API can be updated to work correctly with server actions via useActionState. For context, here is a quick Q&A with the author of conform: edmundhung/conform#818

If your question is just about the action, all conform does is just calling event.preventDefault() conditionally based on the result of the client validation.
https://github.com/edmundhung/conform/blob/cb984945e9cf3a5448bafeef490d8a2dcc845226/packages/conform-react/context.tsx#L486-L501

In order for it to work well with @mantine/form, the following changes would be required:

  1. Right now we always prevent the default:
    https://github.com/mantinedev/mantine/blob/master/packages/%40mantine/form/src/use-form.ts#L205

If we move that line into the if statement for no errors below, it won't run on success by default. Searching up default javascript behavior, I believe if we return true/false from the form.onSubmit(), that should control whether the action is run.

  1. At the moment, the current API doesn't include a name property. Forms through server action require this (since they are being handled by html):
  <TextInput
    label="Email"
    placeholder="[email protected]"
    required
    name="email" // MISSING IN THE CURRENT API
    key={form.key('email')}
    {...form.getInputProps('email')}

We should update the {...form.getInputProps('email')} call to also include the name attribute.

  1. With the above two changes, this is how the resulting code would look:
    <form
      onSubmit={form.onSubmit((_values, event) => {
        event?.currentTarget.requestSubmit();
      })}
      action={formAction}
    >

This works, but it would be nice to have form.onSubmit() do this by default (like conform), since that will be a standard use case. That would finally reduce the usage to looking like the following:

export default function SignIn() {
  const [, formAction, pending] = useActionState(signIn, { error: '' });

  const form = useForm({
    mode: 'uncontrolled',
    initialValues: {
      email: '',
      password: '',
    },
    validate: zodResolver(signInSchema),
  });

  return (
    <form
      onSubmit={form.onSubmit}
      action={formAction}
    >

Which is clean and intuitive.

Points 2 and 3 are nice to have for developer experience, but point 1 is a necessity. There's no way in javascript to reverse the effect of event.preventDefault().

Self-service

  • I would be willing to implement a fix for this issue
@chewhx
Copy link

chewhx commented Nov 20, 2024

Hi @anthonyalayo, it looks like your problem might be resolved if you bind the submission action to a button, rather than <form> element.

As your LoginForm is a client component, it would make more sense to submit via the button onClick. I believe in Nextjs, it's possible to attach a server action to the client button component. You can await the submission and get back a json object. Within the server action, you can perform server side validation as well.

@anthonyalayo
Copy link
Contributor Author

@chewhx have you been successful with that approach? Do you have an example?

@rtivital
Copy link
Member

Will this work for you @anthonyalayo? I've added onSubmitPreventDefault: 'always' | 'never' | 'validation-failed' option to useForm hook,
it changes the onSubmit function the following way:

const onSubmit: OnSubmit<Values, TransformValues> =
    (handleSubmit, handleValidationFailure) => (event) => {
      if (onSubmitPreventDefault === 'always') {
        event?.preventDefault();
      }

      const results = validate();

      if (results.hasErrors) {
        if (onSubmitPreventDefault === 'validation-failed') {
          event?.preventDefault();
        }

        handleValidationFailure?.(results.errors, $values.refValues.current, event);
      } else {
        handleSubmit?.(transformValues($values.refValues.current) as any, event);
      }
    };

@rtivital rtivital added the Fixed patch Completed issues that will be published with next patch (1.0.X) label Nov 20, 2024
@rtivital
Copy link
Member

For 2: I do not think that it is a good idea to set name attribute with getInputProps, it is potentially breaking change and for some inputs it might not work as expected as path values in useForm might include special characters.

@anthonyalayo
Copy link
Contributor Author

This works, thanks @rtivital !

@anthonyalayo
Copy link
Contributor Author

@rtivital , when is the next release planned?

@rtivital
Copy link
Member

Next week

@rtivital
Copy link
Member

Fixed in 7.14.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fixed patch Completed issues that will be published with next patch (1.0.X)
Projects
None yet
Development

No branches or pull requests

3 participants