Option for automatically resetting validators #907
Conversation
☁️ Nx Cloud ReportCI is running/has finished running commands for commit eb01133. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 2 targetsSent with 💌 from NxCloud. |
oscartbeaumont
force-pushed
the
automaticallyResetValidators
branch
from
6a28172
to
f3f8bf4
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #907 +/- ##
==========================================
- Coverage 91.55% 88.48% -3.07%
==========================================
Files 21 26 +5
Lines 900 938 +38
Branches 206 210 +4
==========================================
+ Hits 824 830 +6
- Misses 71 102 +31
- Partials 5 6 +1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Thanks for the PR, the overall idea looks good!
To answer the question of what the default behaviour should be, I think it makes more sense to keep this false for primarily two reasons:
- As you also mentioned, that would be a breaking change
- This adds extra complexity on setting validators, most notably every time you trigger blur on an input it will remove the onChange errors (if a blur validator is set) which might not be expected.
This last point is also the main reason why my initial idea was to only focus on the mount errors.
Would you mind adding some unit tests? I'll be happy to merge this!
| * | ||
| * While `true`: | ||
| * - `onMount` and `onBlur` errors will be cleared when `onChange` triggers validation. | ||
| * | ||
| * While `false`: | ||
| * - An `onMount` error will never be cleared. | ||
| * - An `onBlur` error will only be cleared on the next `onBlur` event. | ||
| * - An `onChange` error will only be cleared on the next `onChange` event. |
There was a problem hiding this comment.
I'd probably add an extra line on top like "Some examples:" since those aren't hardcoded rules.
| * | ||
| * @default false | ||
| */ | ||
| automaticallyResetValidators?: boolean |
There was a problem hiding this comment.
The action this flag enables is that errors are removed when a new validator is run, I think we should better explain this.
I like resetOnRevalidate to indicate that an action (reset) is performed when something happens (onRevalidate) or resetErrorsOnRevalidate if we want to be extra explicit.
What do you think?
A potential solution for #903.
I think this is a more general solution than #726 but they both cover the same issue.
My assumption coming into Tanstack Form was that all previous validation errors would be cleared out when running the next validation but that doesn't seem to be the case.
So
onMountreturning an error means the error will always exist andonBlurreturning an error is only cleared on the next blur event.New behavior
When
form.options.automaticallyResetValidators === trueprevious validation errors are cleared when running the validator,else retain the previous behaviour.
Known problems
Calling
.validateSyncor.validateAsyncwill not cause the fields to be reset. These fields are however marked as@privateso I think this is fine.I think fixing this would also add a lot more complexity because the
.validatemethod calls.validateSyncand then.validateAsync, so we would need to know not to clear the errors that were just added by.validateSyncin.validateAsync.Questions
truebut this would be a breaking change.