Fixes unshift and insert row not update field's state correctly

[Elecweb]

as reported in #44 and final-form/react-final-form-arrays#138.

This applied both unshift and insert because unshift use insert as this

final-form-arrays/src/unshift.js

Lines 5 to 9 in fe3c260

	const unshift: Mutator<any> = (
	[name, value]: any[],
	state: MutableState<any>,
	tools: Tools<any>
	) => insert([name, 0, value], state, tools)

I've investigated and have assumed the issue is in the following.

final-form-arrays/src/insert.js

Lines 18 to 35 in fe3c260

	const pattern = new RegExp(`^${escapeRegexTokens(name)}\\[(\\d+)\\](.*)`)
	const newFields = {}
	Object.keys(state.fields).forEach(key => {
	const tokens = pattern.exec(key)
	if (tokens) {
	const fieldIndex = Number(tokens[1])
	if (fieldIndex >= index) {
	// Shift all higher indices up
	const incrementedKey = `${name}[${fieldIndex + 1}]${tokens[2]}`
	copyField(state.fields, key, newFields, incrementedKey)
	return
	}
	}
	// Keep this field that does not match the name,
	// or has index smaller than what is being inserted
	newFields[key] = state.fields[key]
	})

the logic is trying to shift the field's state to one for all fields that are greater than or equal to inserted index.
For the field's state at inserted index, it will leave as empty because there's a return statement here.

final-form-arrays/src/insert.js

Lines 26 to 37 in fe3c260

	const incrementedKey = `${name}[${fieldIndex + 1}]${tokens[2]}`
	copyField(state.fields, key, newFields, incrementedKey)
	return
	}
	}
	// Keep this field that does not match the name,
	// or has index smaller than what is being inserted
	newFields[key] = state.fields[key]
	})
	state.fields = newFields

For example, if there're 4 field array items and we used insert(2, value), the field's state will shifted like this

newState[0] = currentState[0]
newState[1] = currentState[1]
// newState[2] will be empty
newState[3] = currentState[2]
newState[4] = currentState[3]

this behavior, as I understand, it's intended and should be correct. because when the state is empty, it'll get the default state from here

https://github.com/final-form/final-form/blob/d20c44be8766b93424e7754fe2629820fd93d21a/src/FinalForm.js#L850-L866

which should set the default for the missing state. but the problem is related to React component's key prop

Since the component is rendered by each item in fields, as a convention and it should be, provided with name from fields. so the field component is reused with the same component as the previous, one before insert(2, value) is called and after because name are the same. So method registerField which should set the field's state as default, doesn't trigger. this makes the field's state incorrect and leads to rendering problems.

I've tried to make the key unique and found that it render correctly. but as we know, this is not a good solution.

Screen.Recording.2566-06-19.at.00.30.43.mov

Here are the possible solutions I can think of

1. name should not be used as a key and the final form array provides another variable for this purpose instead.

Since the problem is about the rendering of components, we might need to provide a key which is different before and after insert. But I can't think of how we should generate a key that is new every time we insert a new index but also it should be generated based on an index. This is quite a conflict. so I have no idea about this. anyways, this should be done in https://github.com/final-form/final-form-arrays and shouldn't relate here.

2. Just remove the shifting state and keep only copy value
   as suggested but this Insert does not cause array children to refresh correctly. react-final-form-arrays#138 (comment) which I still do not understand why it works as the field's state at inserted index just missing. Anyways, It might be some edge cases that lead to errors.

3. Add default state for the inserted field.
   IMO, this might be the safest solution, which try to add default as intended. Fortunately, in the mutator, there's a helper function resetFieldState for resetting the field's state.

My PR is to use solution 3, as far as I have checked, it has no problem. I checked with validation for the array, shows an error at submit (this is required the use of the field's state).

(the validation is, the name field must have string "t")
https://github.com/final-form/final-form-arrays/assets/13183413/5c055b9e-9046-49f9-8974-d98a99df5954

For anyone, who can't wait for the fix, and need a quick solution now (I can feel that), I provided the gist you can copy and use the functions as the mutator.

https://gist.github.com/Elecweb/7f3478b01126e40ce98492de32d3f995

I hope this can help anyone get some insight into the issue and solutions 🙏

[makhnatkin]

I had similar problems.

After many experiments, I came to the conclusion that I should abandon using an array in favor of normalized data - an array of IDs and a data object. This also allowed me to solve the issues with the unshift and move mutators.
https://codesandbox.io/s/react-final-form-rows-field-array-alternative-react-dnd-as-drag-drop-6vrpy6
https://github.com/makhnatkin/react-final-form-rows/blob/main/src/useRows.ts

it also helped me solve performance problems.
final-form/react-final-form#336 (comment)

[minh-le-jh]

@erikras Hello, could you have a look at this?

[dingyixxx]

I didn't succeed by introducing the suggested finalFormArray.ts file(https://gist.github.com/Elecweb/7f3478b01126e40ce98492de32d3f995), importing the "insert" function and putting it in Form's mutators...The errors are still there...Instead, i tried to manually "push and update fields value" in order to avoid using splice-relevant array API and succeed.

[erikras-richard-agent]

Thanks for fixing this long-standing bug! 🙏

Bug: unshift and insert mutators don't properly reset field state at the insertion index (issue #44 from 2019!)

Your fix:
Calls resetFieldState(key) for the field being inserted, ensuring clean state.

Observations:
✅ Adds test coverage for the fix
✅ Addresses a real reported bug
⚠️ 2+ years without review - unfortunately sat too long
⚠️ No CI checks - needs rebase to trigger CI

Code review question:
The fix resets state for the inserted field but also keeps the old field state (newFields[key] = state.fields[key]). Can you explain why both operations are needed? Is this preserving state while also resetting it?

@erikras - This fixes a 5+ year old bug. Worth reviewing if the fix is correct.

@Elecweb - If you're still interested, please rebase against master. The community would appreciate this fix! 💙