fix(form-core): reset stale validation counter when shifting array meta#2247
fix(form-core): reset stale validation counter when shifting array meta#2247xianjianlf2 wants to merge 3 commits into
Conversation
When an array item is removed (or inserted/moved) while an async validator is in flight, `shiftMeta` copied the source field's transient `_pendingValidationsCount`/`isValidating` down to the shifted index. The pending promise is owned by the original field instance and settles its counter against the original index, so the shifted index's counter is orphaned and never decremented, leaving `isFieldsValidating` stuck `true`. Reset the transient validation state when shifting meta, and guard `endValidation` so a validation that resolves after its field was removed no longer resurrects deleted meta. Closes TanStack#2234
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis change prevents stale async-validation state after array values are removed or reordered by guarding inactive field validation completion and resetting transient metadata during shifts. Tests cover removal, move, and swap scenarios, with a patch Changesets entry. ChangesArray async-validation cleanup
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
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/metaHelper.ts (1)
100-116: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReset async-validation state in
handleArrayMove/handleArraySwap. ReapplyingfromMetaand swapping meta verbatim carriesisValidatingand_pendingValidationsCountto the new index, so an in-flight validation can stay stuck on the wrong field. MirrorshiftMeta’s reset here and add a move/swap regression test.🤖 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/src/metaHelper.ts` around lines 100 - 116, Reset async-validation state when restoring metadata in handleArrayMove and handleArraySwap, matching shiftMeta’s reset behavior instead of copying isValidating and _pendingValidationsCount unchanged. Update the metadata transfer logic around fromMeta and swapped metadata, then add regression coverage verifying moved and swapped fields do not retain in-flight validation state.
🤖 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.
Outside diff comments:
In `@packages/form-core/src/metaHelper.ts`:
- Around line 100-116: Reset async-validation state when restoring metadata in
handleArrayMove and handleArraySwap, matching shiftMeta’s reset behavior instead
of copying isValidating and _pendingValidationsCount unchanged. Update the
metadata transfer logic around fromMeta and swapped metadata, then add
regression coverage verifying moved and swapped fields do not retain in-flight
validation state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 48e8bfdf-c589-40d8-979e-008a9b920059
📒 Files selected for processing (4)
.changeset/fix-array-removevalue-isvalidating-stuck.mdpackages/form-core/src/FieldApi.tspackages/form-core/src/metaHelper.tspackages/form-core/tests/FieldApi.spec.ts
|
Addressed the CodeRabbit follow-up in 8488fc3. Move and swap now use the same transient async-validation reset as shifted array meta, so Local checks:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@packages/form-core/src/metaHelper.ts`:
- Around line 113-116: Preserve validation metadata for no-op operations by
returning early when fromIndex === toIndex in moveValue at
packages/form-core/src/metaHelper.ts:113-116, and when index === secondIndex in
swapValues at packages/form-core/src/metaHelper.ts:159-162, before resetting
field metadata. Add regression coverage for same-index moveValue and swapValues
calls with dontValidate enabled to verify pending validation remains intact.
🪄 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: 406bd3eb-5e14-4388-a433-7a8336813edd
📒 Files selected for processing (2)
packages/form-core/src/metaHelper.tspackages/form-core/tests/FieldApi.spec.ts
|
Addressed the same-index move/swap follow-up in What changed:
Local check: pnpm --filter @tanstack/form-core exec vitest run tests/FieldApi.spec.tsResult: 1 file / 110 tests passed, no type errors. |
🎯 Changes
Fixes #2234. When an array item is removed (or inserted/moved) while an async validator is in flight,
shiftMetacopied the source field's transient_pendingValidationsCount/isValidatingdown to the shifted index. The pending promise is owned by the original field instance and settles its counter against the original index, so the shifted index's counter is orphaned and never decremented — leavingform.state.isFieldsValidatingstucktrueand blocking submission.shiftMetanow resets the transient validation state (isValidating,_pendingValidationsCount) when shifting meta between indices. Array mutations already re-trigger validation on the shifted fields, so this is safe.endValidationbails out if the field's instance is no longer registered, so a validation that resolves after its field was removed no longer resurrects deleted meta (previously threw aTypeError).✅ Checklist
🚀 Release Impact
Summary by CodeRabbit