DWP-21210: fix broken arrays when mutated#493
Open
theonelucas wants to merge 1 commit into
Open
Conversation
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
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.
TL;DR
Checkboxes/Toggles inside lists, such as tables, break/go into random states when the list content shifts.
Problem
When a field inside an
ArrayFieldwas removed,FormControllercalledObjectMap.delete()which spliced the array in every state map (values,touched,errors,modified, etc.). This shifted the indices of all following sibling fields — theirdefaultValue/initialValueprops no longer matched their actual positions in the array, causing the wrong values to appear or fields to reset unexpectedly.A related side effect: the
useUpdateEffecthook inuseFieldthat resets a field to its initial value whendefaultValue/initialValuechanges was entirely commented out as a workaround, because enabling it on array fields would compound the problem. This meant non-array fields also lost the ability to react to initial value changes.Root cause
ObjectMap.delete()callslddelete(lodash-style delete), which splices arrays. For field removal the intended behavior is to clear the slot, not collapse the array — the array's index structure must stay intact so sibling fields remain at their declared positions.Solution
1. New
ObjectMap.unset()method (src/ObjectMap.js)Adds a
static unset()method that usesldunsetinstead oflddelete. Unlikedelete,unsetsets the value toundefinedwithout splicing the surrounding array. The parent-cleanup step (cleanup()) still runs to prune any now-empty objects/arrays above the field, matching existing behavior for non-array paths.2.
FormControllerusesunseton field removal (src/FormController.js)All eight
ObjectMap.delete()calls in the field-removal path are replaced withObjectMap.unset():state.valuesstate.modifiedstate.maskedValuesstate.touchedstate.errorsstate.dirtstate.focusedstate.dataThis keeps sibling field indices stable after a remove.
3.
useUpdateEffectre-enabled inuseField(src/hooks/useField.js)With indices now stable, the previously-commented-out effect that resets a field when
defaultValue/initialValuechanges (while the form is pristine) is safely re-enabled.A guard is added via
ArrayFieldItemStateContext: fields inside anArrayFieldskip this reset because their values are managed by theArrayField's own index-shifting logic — applying the prop-based reset on top would conflict.Test plan
npm test)ArrayFielddoes not corrupt the values/state of sibling fields at higher indicesdefaultValueresets correctly when the value prop changes and the form is pristineArrayFielddoes not reset whendefaultValuechanges (index management is handled byArrayField)