search

jaredpalmer / formik

Build forms in React, without the tears 😭
https://formik.org
Apache License 2.0
33.9k stars 2.79k forks source link

Fix validation on stale values #3947

Open Jestermaxrko opened 8 months ago

Jestermaxrko commented 8 months ago

Fixes issue - https://github.com/jaredpalmer/formik/issues/2083

The cause of the issue is in this line https://github.com/jaredpalmer/formik/blob/0f960aaeeb0bdbef8312b5107cd3374884a0e62b/packages/formik/src/Formik.tsx#L185

beause of assigning value from stateRef.current to the state variable this variable contains "old" value until next re-render https://github.com/jaredpalmer/formik/blob/0f960aaeeb0bdbef8312b5107cd3374884a0e62b/packages/formik/src/Formik.tsx#L190

And then when setFieldValue and setFieldTouched are called consecutively stateRef.current references to updated value imediatelly after setFieldValue call, but state variable is still references to the old value and setFieldTouched runs validation on that old value

Using stateRef.current directly gurantees that actual values are taken

vercel[bot] commented 8 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
formik-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 11, 2024 10:40pm
changeset-bot[bot] commented 8 months ago

🦋 Changeset detected

Latest commit: e004b643077cd54ff52ad4b18ddb6001ebe87d2b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages | Name | Type | | ------------- | ----- | | formik | Patch | | formik-native | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

codesandbox-ci[bot] commented 8 months ago

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

ypk4 commented 7 months ago

Helpful change. Are we pending one more review to get this merged?

pvcresin commented 6 months ago

Thank you very much. I hope it will be released.

quantizor commented 5 months ago

Would love a test please

quantizor commented 5 months ago

I think I see some of the source of confusion, in Formik.tsx stateRef.current is saved to state but the issue is that ref is wholly-replaced inside of dispatch. We should probably remove that variable altogether because it is a quiet source of bugs.

https://github.com/jaredpalmer/formik/blob/c6ceb654761c268fdc76b225f31453dd4dec1be6/packages/formik/src/Formik.tsx#L185

gone-skiing commented 3 months ago

Any chance this can get merged and released? Seems like a helpful change to keep on the shelf.