fix(form-core): clear stale onMount error on linked-field revalidation - #2244
fix(form-core): clear stale onMount error on linked-field revalidation#2244xianjianlf2 wants to merge 1 commit into
Conversation
A field-level `onMount` error was cleared when the field's own value changed (via `setFieldValue`), but never when the field was revalidated indirectly through `onChangeListenTo`/`onBlurListenTo`. Because a linked revalidation does not change the field's value, the mount-time error lingered even after the field's validators passed, leaving the field (and the form) permanently invalid. Clear the stale `onMount` slot when a passing linked-field revalidation supersedes it, mirroring the existing value-change behavior. Closes TanStack#2124
📝 WalkthroughWalkthroughLinked-field revalidation now clears stale field-level ChangesLinked-field validation
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/form-core/tests/FieldApi.spec.ts (1)
2304-2353: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood regression test; consider adding
onBlurListenTocoverage.The test correctly validates the core sync scenario. Since the PR objectives mention both
onChangeListenToandonBlurListenTo, adding a parallel test for blur-triggered revalidation would protect against regressions in the blur path.🧪 Suggested additional test for onBlurListenTo
+ it('should clear a stale onMount error when a linked field blur revalidation passes', () => { + const form = new FormApi({ + defaultValues: { + password: '', + confirm_password: 'password', + }, + }) + + form.mount() + + const passField = new FieldApi({ form, name: 'password' }) + const passconfirmField = new FieldApi({ + form, + name: 'confirm_password', + validators: { + onMount: ({ value, fieldApi }) => + value !== fieldApi.form.getFieldValue('password') + ? 'Passwords do not match' + : undefined, + onBlurListenTo: ['password'], + onBlur: ({ value, fieldApi }) => + value !== fieldApi.form.getFieldValue('password') + ? 'Passwords do not match' + : undefined, + }, + }) + + passField.mount() + passconfirmField.mount() + + expect(passconfirmField.state.meta.errorMap.onMount).toBe('Passwords do not match') + expect(passconfirmField.getMeta().isValid).toBe(false) + + passField.setValue('password') + passField.handleBlur() + + expect(passconfirmField.state.meta.errorMap.onMount).toBeUndefined() + expect(passconfirmField.state.meta.errors).toStrictEqual([]) + expect(passconfirmField.getMeta().isValid).toBe(true) + expect(form.state.isValid).toBe(true) + })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/form-core/tests/FieldApi.spec.ts` around lines 2304 - 2353, Add a parallel regression test alongside the existing linked-field test covering onBlurListenTo: configure the dependent field with an onMount error and blur-triggered validation, trigger revalidation by blurring the linked field after correcting its value, and assert the stale onMount error is cleared and both field and form validity become true.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/form-core/tests/FieldApi.spec.ts`:
- Around line 2304-2353: Add a parallel regression test alongside the existing
linked-field test covering onBlurListenTo: configure the dependent field with an
onMount error and blur-triggered validation, trigger revalidation by blurring
the linked field after correcting its value, and assert the stale onMount error
is cleared and both field and form validity become true.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3299dd8a-a492-4a08-8a92-22e99a987e12
📒 Files selected for processing (3)
.changeset/clear-stale-onmount-linked-revalidation.mdpackages/form-core/src/FieldApi.tspackages/form-core/tests/FieldApi.spec.ts
MILLERMARRU
left a comment
MILLERMARRU
left a comment
There was a problem hiding this comment.
Went to check setFieldValue in FormApi.ts to confirm the asymmetry the changeset describes is real. It does clear errorMap.onMount unconditionally whenever a field's own value changes, regardless of whether the field's new value actually passes validation. A linked field never goes through that path since its own value doesn't change, only the field it's listening to does, so before this fix a stale onMount error on the listening field had no way to ever clear short of the user editing that field directly, which defeats the point of onChangeListenTo existing in the first place for things like password-confirmation pairs.
Worth noting the new code is actually more conservative than the thing it's filling a gap for: setFieldValue clears onMount on any edit whether or not the new value is valid, this only clears it when the linked revalidation for that specific cause has just passed (!newErrorValue). That's arguably the more correct behavior of the two, since it won't clear a mount error out from under a field that's still genuinely invalid.
Scoping the clear to just the onMount key inside errorMap/errorSourceMap rather than resetting the whole map means any errors from other validation causes on that field survive, which matters since a field can accumulate errors from multiple sources independently.
The test builds the actual password/confirm-password scenario this is meant to fix rather than a synthetic minimal repro, mounts with a real mismatch so onMount genuinely fires first, then changes the linked field and checks the field is fully valid afterward (errors empty, isValid true, and the form's own isValid too), which is the right level to assert at since the bug's real symptom is a permanently-invalid form the user has no way out of.
2 participants
Problem
A field-level
onMounterror is cleared when the field's own value changes, but not when the field is revalidated indirectly throughonChangeListenTo/onBlurListenTo. After a passing cross-field revalidation the stale mount-time error lingers, so the field (and the whole form) stays permanently invalid (#2124).Root cause
FormApi.setFieldValueclearserrorMap.onMounton a direct value change, treating onMount as a mount-time snapshot. Linked-field revalidation re-runs the field's validators without changing its value, so that clearing never runs.Fix
Thread an
isLinkedFieldflag intovalidateFieldFn; on a passing linked-field revalidation, clear the staleonMounterror/source slots. This mirrors the existing value-change clear and the adjacent "clear stale submit error on successful revalidation" logic in the same function.Testing
Added a regression test in
packages/form-core/tests/FieldApi.spec.ts(red before, green after). Full form-core suite (504 tests) passes with no type errors. Changeset included.Closes #2124
Summary by CodeRabbit
Bug Fixes
Tests