airjp73 / rvf

Easy form validation and state management for React and Remix
https://rvf-js.io
MIT License
840 stars 66 forks source link

[Bug]: Can't submit a `ValidatedForm` from a modal #268

Closed haines closed 3 months ago

haines commented 1 year ago

Which packages are impacted?

What version of these packages are you using?

Please provide a link to a minimal reproduction of the issue.

https://github.com/haines/remix-validated-form-submit-from-modal-repro

Steps to Reproduce the Bug or Issue

When a ValidatedForm is submitted by a button in a modal that is unmounted from the DOM on submit, it fails with an uncaught error. The same problem does not exist in Remix's standard Form.

  1. git clone https://github.com/haines/remix-validated-form-submit-from-modal-repro.git
  2. cd remix-validated-form-submit-from-modal-repro
  3. npm install
  4. npm run dev
  5. Open http://localhost:3000
  6. Enter a value in the unvalidated form, click "Save" and "Yes".
  7. See the submitted value logged in the server console, and no error in the browser console.
  8. Enter a value in the validated form, click "Save" and "Yes".
  9. See no value logged in the server console, and an error in the browser console
Uncaught (in promise) Error: Cannot submit a <button> or <input type="submit"> without a <form>
    at getFormSubmissionInfo (node_modules/react-router-dom/dom.ts:196:13)
    at node_modules/react-router-dom/index.tsx:954:48
    at handleSubmit (node_modules/remix-validated-form/src/ValidatedForm.tsx:356:9)

Expected behavior

ValidatedForm behaves the same as Remix's standard Form.

Screenshots or Videos

No response

Platform

Additional context

I guess this may be due to async submit. When Remix's Form calls React Router's submit hook, the submitter (the button element) has form set (because the button hasn't been removed from the DOM yet?). When ValidatedForm calls submit, the submitter's form property is null.

airjp73 commented 1 year ago

This is interesting. It's a little bit related to #117. Like you said, it's because of the async submit.

When I implement this pattern, I usually leave the modal present in the dom and show my loading state on the modal button instead of closing the modal. But both patterns should be considered valid, I think.

I believe useSubmit and fetcher.submit can both accept FormData directly instead of an element, so maybe the solution is to submit the data that we validated rather than the submitter or form elements. I'm not sure if this would miss some edge-cases that can only be handled by submitting the elements though. It could theoretically open us up to behavior that doesn't match the native / no-js behavior.

haines commented 1 year ago

I think that sounds like a good approach. It's unfortunate that React Router doesn't export getFormSubmissionInfo since that seems to handle the edge cases.

airjp73 commented 1 year ago

Could you test this again with v5? I updated when we capture the form data to avoid weird race conditions caused by the async submit. So this might be fixed now.

airjp73 commented 3 months ago

RVF v6 has been released 🎉

This should now longer be an issue in v6. If it's still a problem in that version, please feel free to open a new issue.

You can find the documentation here and the migration guide here.