fix: bump array version on reset so array fields re-render - #2243
fix: bump array version on reset so array fields re-render#2243hemraj-007 wants to merge 1 commit into
Conversation
Bump _arrayVersion after form.reset and form.resetField so React array fields re-render when reset shortens the array. Fixes TanStack#2228 Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughForm reset operations now bump ChangesArray reset rerender
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/form-core/src/FormApi.ts (1)
1850-1858: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract duplicated array-version-bump loop into a shared helper.
The loop at lines 1851–1858 is identical to the one already in
update()at lines 1786–1793. Extracting it into a private method (e.g.,bumpArrayVersionsForAllFields()) would eliminate the duplication and ensure both call sites stay in sync.♻️ Proposed refactor
+ /** + * `@private` Bump _arrayVersion for every mounted field whose value is an array. + */ + private bumpArrayVersionsForAllFields = () => { + const helper = metaHelper(this) + for (const fieldKey of Object.keys( + this.fieldInfo, + ) as DeepKeys<TFormData>[]) { + if (Array.isArray(this.getFieldValue(fieldKey))) { + helper.bumpArrayVersion(fieldKey) + } + } + } + reset = (values?: TFormData, opts?: { keepDefaultValues?: boolean }) => { // ... existing reset logic ... - const helper = metaHelper(this) - for (const fieldKey of Object.keys( - this.fieldInfo, - ) as DeepKeys<TFormData>[]) { - if (Array.isArray(this.getFieldValue(fieldKey))) { - helper.bumpArrayVersion(fieldKey) - } - } + this.bumpArrayVersionsForAllFields() }Then apply the same replacement in
update()at lines 1786–1793.🤖 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/FormApi.ts` around lines 1850 - 1858, Extract the duplicated array-version-bump loop from update() and the shown flow into a shared private method, such as bumpArrayVersionsForAllFields(). Have the helper create or use metaHelper(this), iterate over fieldInfo keys, and call bumpArrayVersion for array-valued fields; replace both existing loops with calls to this method.
🤖 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/FormApi.ts`:
- Around line 2966-2969: Add a FormApi.spec.ts test covering resetField on an
array field, using the existing FormApi, FieldApi, and resetField test patterns.
Set an array value, call resetField, then assert the default array is restored
and the field meta _arrayVersion is bumped as expected.
---
Nitpick comments:
In `@packages/form-core/src/FormApi.ts`:
- Around line 1850-1858: Extract the duplicated array-version-bump loop from
update() and the shown flow into a shared private method, such as
bumpArrayVersionsForAllFields(). Have the helper create or use metaHelper(this),
iterate over fieldInfo keys, and call bumpArrayVersion for array-valued fields;
replace both existing loops with calls to this method.
🪄 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: b94af9cf-baaa-470c-9593-93e75079298b
📒 Files selected for processing (4)
.changeset/fix-array-reset-rerender.mdpackages/form-core/src/FormApi.tspackages/form-core/tests/FormApi.spec.tspackages/react-form/tests/useField.test.tsx
|
|
||
| if (Array.isArray(this.getFieldValue(field))) { | ||
| metaHelper(this).bumpArrayVersion(field) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add test coverage for resetField array version bump.
The reset() path has a dedicated test in FormApi.spec.ts, but the resetField() array-version bump (lines 2967–2968) has no corresponding test. A regression could silently break this path.
🧪 Suggested test
it('should bump array version when resetField is called on an array field', () => {
const form = new FormApi({
defaultValues: {
items: [1, 2, 3],
},
})
form.mount()
const field = new FieldApi({ form, name: 'items' })
field.mount()
form.setFieldValue('items', [1, 2, 3, 4, 5])
// _arrayVersion is bumped by pushFieldValue or setFieldValue paths
form.resetField('items')
expect(form.getFieldValue('items')).toEqual([1, 2, 3])
expect(form.getFieldMeta('items')?._arrayVersion).toBe(1)
})🤖 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/FormApi.ts` around lines 2966 - 2969, Add a
FormApi.spec.ts test covering resetField on an array field, using the existing
FormApi, FieldApi, and resetField test patterns. Set an array value, call
resetField, then assert the default array is restored and the field meta
_arrayVersion is bumped as expected.
MILLERMARRU
left a comment
MILLERMARRU
left a comment
There was a problem hiding this comment.
Went to check why bumping _arrayVersion specifically is what's needed here, and it's because react-form's useField deliberately subscribes to state.meta._arrayVersion instead of state.value for mode="array" fields, there's already a comment there referencing #1925 explaining it's an intentional optimization to avoid re-rendering on every child property change. That means for an array field, React genuinely does not know the array changed unless _arrayVersion itself changes, no matter what happened to the actual array reference or length. form.reset()/resetField() changing the array without touching that counter is exactly the kind of thing that selector is blind to, so the fix is targeting the right mechanism, not just papering over a symptom.
Checked the ordering in both call sites too. In reset(), the new loop runs after the baseStore.setState that actually commits the new values, so this.getFieldValue(fieldKey) inside the loop is reading the post-reset array, not stale state. Same story in resetField.
One minor thing, not a correctness issue: resetField isn't wrapped in batch(), so setting fieldMetaBase[field] to defaultFieldMeta (which resets _arrayVersion to 0) and then calling bumpArrayVersion right after are two separate store commits rather than one. Since the array value itself is already correct as of the first commit, this shouldn't produce wrong output, just possibly one extra render pass in cases where _arrayVersion wasn't already 0 going in. Worth a look if this file already batches similar multi-step meta+value updates elsewhere, but not something I'd block on.
Good that the React test renders through an actual mode="array" field and checks the DOM list length after reset, rather than asserting on internal state, since the whole point of the fix is what the UI shows, not just what the store holds.
2 participants
Summary
_arrayVersionafterform.reset()andform.resetField()when the target field is an array.[1,2,3]→[4]previously still rendered 3 items).Test Plan
pnpm run test:libinpackages/form-corepnpm run test:libinpackages/react-formSummary by CodeRabbit