fix: sync validation errors for delayed-mounted fields #1691
Conversation
|
👀 |
|
@LeCarbonator I thought this issue needed to be resolved promptly, and I'm curious to hear your thoughts on the matter. |
|
I'm also quite curious about this one. I've had to use |
|
View your CI Pipeline Execution ↗ for commit 2289eae
☁️ Nx Cloud last updated this comment at |
|
I'm just not sure if this is the way to go about it. I've been tinkering behind the scenes to see what truly causes this, but I suppose I should focus more on it so it can get fixed |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1691 +/- ##
==========================================
+ Coverage 90.35% 90.55% +0.19%
==========================================
Files 38 38
Lines 1752 1810 +58
Branches 444 470 +26
==========================================
+ Hits 1583 1639 +56
- Misses 149 151 +2
Partials 20 20 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I'm not entirely sure why the delayed-mounted field isn't receiving form validation either. While my proposed method could resolve it, errors exist in both If you have any ideas, please share them so we can explore them together. I have three potential approaches in mind
|
|
The issue is that while mapping form validators to respective fields, it only iterates through the existing record of (previously registered) fields.
|
|
@LeCarbonator FormApi has been improved with a comprehensive error-handling approach that includes all field errors, and features have been added to automatically generate fieldInfo and prevent memory leaks in the deleteField method. |
|
@jiji-hoon96 does this mean the issue should be resolved? |
|
@brandonleichty Yes! |
|
packages/form-core/src/FormApi.ts
Outdated
| if (fieldErrors) { | ||
| const allFieldErrors: Partial< | ||
| Record<DeepKeys<TFormData>, ValidationErrorMap> | ||
| > = this.state._allFieldErrors || {} | ||
| for (const [fieldName, fieldError] of Object.entries(fieldErrors)) { | ||
| if (fieldError) { | ||
| const typedFieldName = fieldName as DeepKeys<TFormData> | ||
| allFieldErrors[typedFieldName] = { | ||
| ...allFieldErrors[typedFieldName], | ||
| [errorMapKey]: fieldError, | ||
| } | ||
| } | ||
| } | ||
| this.baseStore.setState((prev) => ({ | ||
| ...prev, | ||
| _allFieldErrors: allFieldErrors, | ||
| })) | ||
| } | ||
|
|
||
| const allFieldsToProcess = new Set([ | ||
| ...Object.keys(this.state.fieldMeta), | ||
| ...Object.keys(fieldErrors || {}), | ||
| ] as DeepKeys<TFormData>[]) | ||
|
|
||
| for (const field of allFieldsToProcess) { | ||
| if (fieldErrors?.[field] && !this.fieldInfo[field]) { | ||
| this.getFieldInfo(field) | ||
| } | ||
|
|
There was a problem hiding this comment.
This is going in the right direction, but _allFieldErrors is not the way. Iterating over previously mounted fields in fieldMeta is not enough for validation errors (like you correctly asserted), so ones that are missing should be added to it.
Fields derive their data from fieldMeta, so if they were added here, they would have the data not just on mount, but on construction.
One issue that we might run into with this approach is that wrong field names can result in a locked state. There is no field to read the stored error, so it is never visually shown. I don't think that's an issue because:
- Our standard schemas are type safe when it comes to keys.
- Standard schemas send a copy of the error list to the form error state, so it can be figured out if there's an issue.
Overall, I think the right way to go is to add missing fieldMeta from field errors if they're not present. Use the default field meta state for the other props being added.
There was a problem hiding this comment.
Thank you for the detailed feedback!
You're absolutely right. I've refactored the implementation to use fieldMeta instead of _allFieldErrors.
| }) | ||
| existingField.mount() | ||
|
|
||
| await new Promise((resolve) => setTimeout(resolve, 100)) |
There was a problem hiding this comment.
There is a sleep helper from utils.ts you can use, along with vi.useFakeTimers(). See other async unit tests for details.
There was a problem hiding this comment.
Great catch! I've replaced all setTimeout promises with the sleep helper from utils.ts and added vi.useFakeTimers() / vi.useRealTimers() to the async tests, following the pattern used in other test files.
3 participants
Fixes validation errors not appearing on fields that mount after form initialization
Problem
When fields are mounted after the form's initial validation runs, validation errors from form-level validators (like [onMount]) are not displayed on these delayed fields, leading to inconsistent validation UX.
Solution
_allFieldErrorsstateChanges
_allFieldErrorsproperty to BaseFormState to persist validation errors for all fieldsFieldApi.mount()to sync existing errors from form state on field mountCloses #1630