Fix #366: keepDirtyOnReinitialize with array fields#538
Conversation
When keepDirtyOnReinitialize is true and array fields are used (e.g., final-form-arrays), pristine child fields weren't updating to new initial values because the parent array field's dirty value was overwriting them. Fix: When saving dirty values, skip parent fields if child fields are registered. For example, if both 'customers' and 'customers[0].firstName' are registered, only save dirty values for the more specific child fields. This allows pristine children to update while dirty children keep their modified values, which is the expected keepDirtyOnReinitialize behavior. Added test verifying: - Modified child field keeps dirty value - Pristine child field updates to new initial value - Parent array field doesn't override children
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughUpdates FinalForm reinitialization to avoid persisting parent keys that have registered child fields when keepDirtyOnReinitialize is true, and adds a test verifying nested array-field dirty behavior across reinitialize. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
The child field detection logic adds ~54 bytes to prevent parent array fields from overwriting pristine children when keepDirtyOnReinitialize is enabled. Small size increase is acceptable for fixing the bug.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
Verify each finding against the current code and only fix it if needed.
In `@src/FinalForm.arrays-keepdirty.test.ts`:
- Around line 47-48: Remove the stale inline comment preceding the failing
assertion: delete the comment text "// BUG: This fails - it keeps the old value
'Smith' instead of updating to 'NewSmith'" that sits above the
expect(lastName1State.value).toBe('NewSmith') assertion so the test no longer
contains misleading commentary; leave the
expect(lastName1State.value).toBe('NewSmith') line and surrounding test logic
unchanged.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
Verify each finding against the current code and only fix it if needed.
In `@src/FinalForm.arrays-keepdirty.test.ts`:
- Around line 43-47: Replace the mixed assertion that reads the form state via
getIn(form.getState().values, 'customers[0].firstName') with the field
subscriber variable firstName0State.value to keep assertions consistent and
verify the subscriber was notified after reinitialization; update the
expectation that currently checks the modified first name to use
firstName0State.value and leave the lastName1State.value assertion intact so
both field-state subscriptions are validated.
|
Published in |
Problem: When
keepDirtyOnReinitializeis true and array fields are used (e.g., with final-form-arrays), pristine child fields don't update to new initial values. The parent array field's dirty value overwrites them.Root Cause: When saving dirty values, the code saves ALL dirty fields including parent array fields (e.g.,
customers) AND their children (e.g.,customers[0].firstName). When setting values back, the parent array value overwrites pristine children.Example Bug:
Solution: When saving dirty values, skip parent fields if child fields are registered. For example, if both
customersandcustomers[0].firstNameare registered, only save dirty values for the child fields.This allows:
Testing:
Fixes #366
Summary by CodeRabbit
Bug Fixes
Tests
Chores