final-form / react-final-form

🏁 High performance subscription-based form state management for React
https://final-form.org/react
MIT License
7.39k stars 481 forks source link

Race condition for async field-level validation #437

Open kim-knudsen opened 5 years ago

kim-knudsen commented 5 years ago

Are you submitting a bug report or a feature request?

Bug

What is the current behavior?

Async field-level validation may silently fail if the async response times differ.

What is the expected behavior?

That older ongoing promises are ignored, preventing potential "race conditions".

Sandbox Link

A fork of the official async field-level validation example can be found here:

https://codesandbox.io/s/1zpv27y0qq

Notice how usernameAvailable now has a random (more realistic) response time:

const randomBetween = (min, max) => {
  return Math.floor(Math.random() * (max - min + 1) + min);
}

const usernameAvailable = async value => {
  if (!value) {
    return "Required";
  }

  await sleep(randomBetween(400, 2000));

  if (
    ~["john", "paul", "george", "ringo"].indexOf(value && value.toLowerCase())
  ) {
    return "Username taken!";
  }
};

You don't need many attempt to see that validation will fail for the username.

What's your environment?

Using Sandbox with the following dependencies:

"dependencies": {
  "final-form": "4.6.1",
  "react": "16.3.2",
  "react-dom": "16.3.2",
  "react-final-form": "3.4.0",
  "styled-components": "3.2.6"
}
Andarist commented 5 years ago

Seems like a bug in final-form. It could be better to keep field level validation promises under field names in asyncValidationPromises and replace the pending promises with the latest ones. The change would have to be done around those lines https://github.com/final-form/final-form/blob/f02fd2699e4cb056b9d55882c1ac0d80e77ef9e7/src/FinalForm.js#L301

Would you like to work on fixing this?

thomaschaaf commented 5 years ago

It was fixed πŸŽ‰ https://github.com/final-form/final-form/pull/265