fix(form-core): clear stale onMount error on linked-field revalidation#2136
Open
mixelburg wants to merge 1 commit intoTanStack:mainfrom
Open
fix(form-core): clear stale onMount error on linked-field revalidation#2136mixelburg wants to merge 1 commit intoTanStack:mainfrom
mixelburg wants to merge 1 commit intoTanStack:mainfrom
Conversation
📝 WalkthroughWalkthroughFieldApi's validation logic updated to conditionally clear stale Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/form-core/src/FieldApi.ts (1)
1947-1969:⚠️ Potential issue | 🟡 MinorClear
errorSourceMap.onMountto maintain consistency witherrorMap.onMount.The same issue exists here as in
validateSync: when clearingerrorMap.onMount, the correspondingerrorSourceMap.onMountentry should also be cleared. This mirrors the submit error clearing pattern (lines 1787-1797).Additionally, use optional chaining (
prev.errorMap?.onMount) at line 1957 for defensive consistency with line 1950.🔧 Proposed fix to clear errorSourceMap.onMount
field.setMeta((prev) => { const nextErrorMap = { // eslint-disable-next-line `@typescript-eslint/no-unnecessary-condition` ...prev?.errorMap, [errorMapKey]: newErrorValue, } + const nextErrorSourceMap = { + ...prev.errorSourceMap, + [errorMapKey]: newSource, + } // Clear stale onMount error when a later validator confirms the field is valid if ( !newErrorValue && errorMapKey !== 'onMount' && - prev.errorMap.onMount + prev.errorMap?.onMount ) { nextErrorMap.onMount = undefined + nextErrorSourceMap.onMount = undefined } return { ...prev, errorMap: nextErrorMap, - errorSourceMap: { - ...prev.errorSourceMap, - [errorMapKey]: newSource, - }, + errorSourceMap: nextErrorSourceMap, } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/form-core/src/FieldApi.ts` around lines 1947 - 1969, In field.setMeta callback (the block updating errorMap/errorSourceMap), when you clear errorMap.onMount you must also clear errorSourceMap.onMount to keep maps consistent: update the logic that checks "!newErrorValue && errorMapKey !== 'onMount' && prev.errorMap.onMount" to use optional chaining (prev.errorMap?.onMount) and, in that branch, set nextErrorSourceMap.onMount = undefined (or remove it) alongside setting nextErrorMap.onMount = undefined; reference the existing symbols errorMapKey, newErrorValue, newSource, prev, errorMap, and errorSourceMap to locate and update this behavior inside the field.setMeta callback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/form-core/src/FieldApi.ts`:
- Around line 1738-1759: In field.setMeta's updater (the block handling
nextErrorMap), when you detect clearing a stale onMount error (the if that
checks !newErrorValue && errorMapKey !== 'onMount' && prev.errorMap.onMount),
also clear the corresponding entry in errorSourceMap (set
nextErrorSourceMap.onMount = undefined or ensure the returned errorSourceMap has
onMount removed) so errorMap and errorSourceMap stay in sync; additionally
change the check to use optional chaining (prev.errorMap?.onMount) for defensive
access. Reference: the field.setMeta updater, variables errorMapKey,
newErrorValue, newSource, and errorSourceMap in this diff.
---
Outside diff comments:
In `@packages/form-core/src/FieldApi.ts`:
- Around line 1947-1969: In field.setMeta callback (the block updating
errorMap/errorSourceMap), when you clear errorMap.onMount you must also clear
errorSourceMap.onMount to keep maps consistent: update the logic that checks
"!newErrorValue && errorMapKey !== 'onMount' && prev.errorMap.onMount" to use
optional chaining (prev.errorMap?.onMount) and, in that branch, set
nextErrorSourceMap.onMount = undefined (or remove it) alongside setting
nextErrorMap.onMount = undefined; reference the existing symbols errorMapKey,
newErrorValue, newSource, prev, errorMap, and errorSourceMap to locate and
update this behavior inside the field.setMeta callback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5da07b51-1570-4519-a1ac-93bc4e531c01
📒 Files selected for processing (1)
packages/form-core/src/FieldApi.ts
Comment on lines
+1738
to
+1759
| field.setMeta((prev) => { | ||
| const nextErrorMap = { | ||
| ...prev.errorMap, | ||
| [errorMapKey]: newErrorValue, | ||
| }, | ||
| errorSourceMap: { | ||
| ...prev.errorSourceMap, | ||
| [errorMapKey]: newSource, | ||
| }, | ||
| })) | ||
| } | ||
| // Clear stale onMount error when a later validator confirms the field is valid | ||
| if ( | ||
| !newErrorValue && | ||
| errorMapKey !== 'onMount' && | ||
| prev.errorMap.onMount | ||
| ) { | ||
| nextErrorMap.onMount = undefined | ||
| } | ||
| return { | ||
| ...prev, | ||
| errorMap: nextErrorMap, | ||
| errorSourceMap: { | ||
| ...prev.errorSourceMap, | ||
| [errorMapKey]: newSource, | ||
| }, | ||
| } | ||
| }) |
There was a problem hiding this comment.
Clear errorSourceMap.onMount to maintain consistency with errorMap.onMount.
When clearing errorMap.onMount, the corresponding errorSourceMap.onMount entry should also be cleared to keep the two maps in sync. The existing submit error clearing logic (lines 1787-1797) clears both maps, establishing this pattern.
Additionally, use optional chaining (prev.errorMap?.onMount) at line 1747 for defensive consistency with line 1737.
🔧 Proposed fix to clear errorSourceMap.onMount
field.setMeta((prev) => {
const nextErrorMap = {
...prev.errorMap,
[errorMapKey]: newErrorValue,
}
+ const nextErrorSourceMap = {
+ ...prev.errorSourceMap,
+ [errorMapKey]: newSource,
+ }
// Clear stale onMount error when a later validator confirms the field is valid
if (
!newErrorValue &&
errorMapKey !== 'onMount' &&
- prev.errorMap.onMount
+ prev.errorMap?.onMount
) {
nextErrorMap.onMount = undefined
+ nextErrorSourceMap.onMount = undefined
}
return {
...prev,
errorMap: nextErrorMap,
- errorSourceMap: {
- ...prev.errorSourceMap,
- [errorMapKey]: newSource,
- },
+ errorSourceMap: nextErrorSourceMap,
}
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/form-core/src/FieldApi.ts` around lines 1738 - 1759, In
field.setMeta's updater (the block handling nextErrorMap), when you detect
clearing a stale onMount error (the if that checks !newErrorValue && errorMapKey
!== 'onMount' && prev.errorMap.onMount), also clear the corresponding entry in
errorSourceMap (set nextErrorSourceMap.onMount = undefined or ensure the
returned errorSourceMap has onMount removed) so errorMap and errorSourceMap stay
in sync; additionally change the check to use optional chaining
(prev.errorMap?.onMount) for defensive access. Reference: the field.setMeta
updater, variables errorMapKey, newErrorValue, newSource, and errorSourceMap in
this diff.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2124
When a field has an validator and is linked to another field via /, the onMount error was never cleared even when the linked-field validator confirmed the field was valid. This left and stuck.
The fix: after a sync or async validator runs on a field (or its linked field) and reports no error, if an entry is present, clear it — the later validation supersedes the mount-time check. Same pattern already used for clearing submit errors on change.
Summary by CodeRabbit