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

Make the second parameter of the formAction$ optional #204

Open
devcaeg opened this issue May 13, 2024 · 6 comments
Open

Make the second parameter of the formAction$ optional #204

devcaeg opened this issue May 13, 2024 · 6 comments
Assignees
Labels
question Further information is requested

Comments

@devcaeg
Copy link

devcaeg commented May 13, 2024

The formAction$ function forces you to pass the second parameter, even if it is not used. In my case, because of this restriction, I must pass $(() => ({})) as the second parameter.

export const useFormAction = formAction$<LoginForm>(
  async (values) => {
    ...
  },
  $(() => ({})),
);

I think this parameter should be optional.

@fabian-hiller
Copy link
Owner

fabian-hiller commented May 13, 2024

I'm not sure we should do this. It could lead to someone accidentally forgetting the argument, which could lead to security problems since it is used to validate the data of the form. This would allow an attacker to send any data to your server.

@fabian-hiller fabian-hiller self-assigned this May 13, 2024
@fabian-hiller fabian-hiller added the question Further information is requested label May 13, 2024
@devcaeg
Copy link
Author

devcaeg commented May 13, 2024

The problem is that if a person does not want to validate it, he is forced to put a "$" function that returns an empty object.

I know it's not a problem per se, it's simply a matter of laziness 😅

@fabian-hiller
Copy link
Owner

Don't you use a schema to make your form type safe?

@devcaeg
Copy link
Author

devcaeg commented May 13, 2024

I use a backend framework (my own) for end-to-end type safety. With that I am covered from errors thanks to TypeScript, and the server itself has its schemas on each request for data purging. For this reason I don't do those validations in the frontend (I know that maybe I should do them to avoid server requests), but this project is not a monorepo and I don't want to repeat the schemas in frontend and backend.

@devcaeg
Copy link
Author

devcaeg commented May 13, 2024

I actually have another problem with this haha.

type LoginForm = {
  email: string;
  password: string;
};

export const useFormAction = formAction$<LoginForm>(
  async (values) => {
    const _client = client<typeof loginModule>({
      url: 'http://localhost:3000',
    });

    const request = await _client.fetch('/auth/login', {
      method: 'POST',
      body: values,
    });

    if (!request.success) {
      throw new FormError<LoginForm>(request.response);
    }

    return {
      status: 'success',
      message: 'You are now logged in',
      data: request.response,
    };
  },
  $(() => ({})),
);

As you can see, I am not declaring the type TResponseData, so when in the return I add data: request.response the formAction$ function gives error because it expects data to be of type undefiend.

If TResponseData is not declared, it should infer the types returned in data so avoid errors and not lose type safety.

@fabian-hiller
Copy link
Owner

Thanks for the feedback. In general, I am very focused on Valibot at the moment. Therefore, I do not plan to implement your feedback right away, but I do plan to rewrite Modular Forms later this year or next year.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants