remix-validated-form icon indicating copy to clipboard operation
remix-validated-form copied to clipboard

[Bug]: FormStore validateField appears to have a race condition on simultaneous onBlur validation and submission with a form that removes itself on submission

Open fnimick opened this issue 2 years ago • 2 comments

Which packages are impacted?

  • [X] remix-validated-form
  • [ ] @remix-validated-form/with-zod
  • [ ] @remix-validated-form/with-yup
  • [ ] zod-form-data

What version of these packages are you using?

  • remix-validated-form: 4.5.5

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

(could not reproduce outside of Cypress test environment)

Steps to Reproduce the Bug or Issue

  1. Create a form with an input that performs default validation behavior (on blur, then on change). Have the form submission remove the form itself from the DOM when the submission completes, using onSubmit.
  2. Type into the form a valid value.
  3. Click the submit button of the form, which blurs the field and triggers field-level validation at the same time as the form itself is validating and submitting.
  4. Sometimes (!!) the form submission completes and the form is removed before validateField completes, and the get().clearFieldErrors(...) call throws an error as get() returns undefined.

Expected behavior

The order of field validation and form submission should be predictable.

Screenshots or Videos

Error when test moves straight from typing to clicking the form submission button: image

Identical test works fine when the field is manually blurred prior to submission: image

Platform

  • OS: macOS
  • Browser: Chrome
  • Version: 105

Additional context

I'm not able to duplicate it manually, only when the browser is being run in a Cypress test environment.

fnimick avatar Sep 21 '22 22:09 fnimick

Interesting interaction -- Thanks for bringing this up. I bet we could even reproduce this manually with an async validator that takes a couple seconds. Seems like we could fix this and #117 all in one go.

If you have time, could you write a failing cypress test for this in the test-app? That would involve writing a new route (might even be able to reuse a route) and adding a test to one of the existing test files (maybe async-validation)

airjp73 avatar Sep 25 '22 16:09 airjp73

I just released 4.6.0 which includes some pretty extensive refactoring of the form store. It's been months since I worked on it though. Would you mind verifying if this is still an issue in the latest version?

airjp73 avatar Oct 27 '22 03:10 airjp73