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

(bug): Remix hook form is not behaving as expected or documented #80

Closed
AdiRishi opened this issue Feb 24, 2024 · 6 comments
Closed

(bug): Remix hook form is not behaving as expected or documented #80

AdiRishi opened this issue Feb 24, 2024 · 6 comments

Comments

@AdiRishi
Copy link
Contributor

I've encountered a problem with remix-hook-form where form submissions are not behaving as expected or as I believe is documented.
My apologies in advance if this issue stems from a misunderstanding on my part. However, after several attempts and reviews of my code, I'm inclined to believe there might be an underlying issue with how remix-hook-form handles form submissions, specifically when trying to use the method="post" and action attributes without explicitly using a useFetcher.

Overview

  • Expected Behavior: When setting up a form using remix-hook-form with method="post" and an action pointing to a route, I expected the form submission to be directed to the specified route without causing a full page reload.
  • Actual Behavior: Instead of submitting to the specified route as expected, the form submission results in a full page reload along with it completely ignoring the url location given in the action prop. This behavior suggests that the form props are not being read and submission is not using the underlying fetcher as the docs state.
  • Workaround: The only way I've managed to get the form to submit to the correct route without causing a page reload is by explicitly using useFetcher and specifying a submitConfig in the form setup. This approach seems counterintuitive if the library is supposed to handle submissions more seamlessly by default.

Issue reproduction

I've made a repository that demonstrates the problem. You can find it here - https://stackblitz.com/edit/remix-run-remix-xr6vlj?file=app%2Froutes%2F_index.tsx

Key Points:

  • Two Forms: The demo includes two forms. One (SubscribeFormThatDoesNotWork) doesn't use useFetcher and fails to submit properly, causing a page reload. The other (SubscribeFormThatDoesWork) uses useFetcher and works fine, submitting without reloading the page.
  • Submission State: Another thing I noticed is that the form.formState.isSubmitSuccessful value that comes back from remix-hook-form is completely wrong. I've setup the API endpoint to respond with a 400, and yet it doesn't reflect that at all. Also the value is set as soon as the request initiates, it doesn't wait for success

Conclusion

Again, I'm very sorry if I'm being dumb and missing something obvious, but I can't for the life of me figure out what is going wrong.
Looking forward to finding out more 🙏

@AlemTuzlak
Copy link
Contributor

@AdiRishi So the reason your form does not work as you'd expect it is because the handleSubmit that is passed to the Form component actually uses useSubmit under the hood and submits to the current location, the extra submitConfig is basically used to override this behavior for the useSubmit. The method and action should be there as well in case JS isn't loaded but otherwise it would work as expected if you add the config.

The isSubmitSuccessful mimics the behavior from react-hook-form where it is set as soon as the data is SENT to the server. isSubmitting is used to determine if the submission is still going on.

So these are not bugs per say, but could be potentially improved upon. It's very hard to determine on the hooks end if the submission was successful. What if you return 200 with errors, or redirect to somewhere. Although I think it would maybe be possible to access the form event and get the method and action instead of having to define it via config though.

@AdiRishi
Copy link
Contributor Author

@AlemTuzlak thanks for looking into this. Do you think it's reasonable for the library to try and extract the method + action from the form directly? I'm fairly certain you would be able to reach it via event.target?

Just a little more clarification. As you said, when we add the submitConfig option to SubscribeFormThatDoesNotWork it does indeed post to the correct location. But it also does a page reload and moves to that action route.
Is that expected default behavior? It seems like we need to specify a fetcher and use fetcher.Form in order to get it to submit the form via ajax requests

@AdiRishi
Copy link
Contributor Author

Re your point on determining success vs failure. Yeah it makes sense, there are many scenarios that could mean success or failure. Generally speaking however I think it seems reasonable to assume a non-2xx response is a failure.

I can always get around this by try/catch on the action and make sure there is no uncaught error. What is the canonical way to return a "route failure" to the form with this library?

@AlemTuzlak
Copy link
Contributor

  1. It's more than reasonable and I'd prefer that than the current API tbh. I'll look into it when I get time, if you're in a rush feel free to create a PR. (Action and method I mean)
  2. The usual way is to return the errors object or create it yourself and return it and the hook will pick it up and use it to show errors

@AdiRishi
Copy link
Contributor Author

Thanks! I've learned a bit more since I made this issue, seems fetcher doesn't really return error states since it propagates that up to the error boundary. Maybe it makes sense for us to provide a helper error boundary that understands form errors or "top level" errors that can be thrown from the API.

Leaving the issue open since I plan to revisit this and maybe make a PR if I find a reasonable solution to these issues.

@AlemTuzlak
Copy link
Contributor

@AdiRishi I implemented this behavior in v5.0.0

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

No branches or pull requests

2 participants