Fix #487: Track dirty state for unregistered fields (FieldArray)#528
Conversation
When using FieldArray, only individual array items are registered as fields, not the array itself. When all items are removed, no registered fields exist, so the form incorrectly shows pristine=true/dirty=false even though the array value changed from initialValues. Root cause: Form only checked dirty state for registered fields, ignoring changes to unregistered fields. Solution: After checking registered fields, also check all unregistered top-level keys in values vs initialValues. This ensures that changes to unregistered fields (like the array field when using FieldArray) are properly tracked as dirty. Changes: - In calculateNextFormState, check unregistered fields for dirty state - Use shallowEqual to compare unregistered field values - Only applies to unregistered fields (preserves custom isEqual for registered) Added test to verify form remains dirty after removing all array items when the array field itself is not registered. All existing tests pass (340/340).
Address CodeRabbit feedback: use Set to collect unique keys from both values and initialValues, avoiding redundant allocations and comparisons while preserving existing dirty-check behavior.
📝 WalkthroughWalkthroughAdds fallback dirtiness detection inside Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
src/FinalForm.ts (1)
592-624:⚠️ Potential issue | 🟠 MajorCustom
isEqualcan be bypassed for nested fieldsWhen
foundDirtyis false (Line 608), the new top‑levelshallowEqualcomparison can mark the form dirty even if all registered nested fields are “pristine” per their customisEqual. Example: a registered fielduser.namewith a case‑insensitiveisEqualwould still flip the form to dirty because the top‑leveluserobject differs. If preserving customisEqualsemantics is required, consider scoping the fallback to only unregistered structural changes (e.g., array length deltas) or otherwise excluding paths covered by registered fields with customisEqual, and add a regression test to confirm the intended behavior.
|
Published in |
2 participants
Fixes #487
Problem
When using FieldArray, only individual array items are registered as fields, not the array itself. When all items are removed, no registered fields exist, so the form incorrectly shows pristine=true/dirty=false even though the array value changed from initialValues.
Root cause: Form only checked dirty state for registered fields, ignoring changes to unregistered fields.
Solution
After checking registered fields, also check all unregistered top-level keys in values vs initialValues using Set deduplication for performance. This ensures that changes to unregistered fields (like the array field when using FieldArray) are properly tracked as dirty.
Changes
Tests
Added test to verify form remains dirty after removing all array items when the array field itself is not registered.
All existing tests pass (342/342).
Note
This is a clean rewrite of #525, focusing only on the #487 fix without the #482 changes.
Summary by CodeRabbit
Bug Fixes
Tests
Chores