Skip to content

Commit 656916f

Browse files
erikras-richard-agentErik Rasmussen
andauthored
Fix #44: Reset field state at insertion index in insert/unshift (#103)
* Fix #44: Reset field state at insertion index in insert/unshift Fixes #44 (implements solution from PR #96) ## Problem When insert() or unshift() is called on an array field, React reuses the component at the insertion index because the 'name' prop stays the same. This causes stale data to appear and field updates to fail. Example: If foo[0] and foo[1] exist, and you unshift a new item: - Values correctly become: [NEW, old-foo[0], old-foo[1]] - But field state for foo[0] still has old-foo[0]'s data - React doesn't remount the component, so registerField doesn't fire - Result: foo[0] shows old data and doesn't respond to changes ## Solution When shifting fields at index >= insertionIndex: 1. Copy field state to the incremented position (foo[0] → foo[1]) 2. Keep the original field at the insertion index (foo[0]) 3. Reset that field to default state via resetFieldState() This ensures the field at the insertion index has fresh state, forcing React to recognize it as a new field when the component re-renders. ## Tests - Updated insert.test.ts to expect field at insertion index - Updated unshift.test.ts to expect field at insertion index - Added unshift.regression-44.test.ts with regression tests All 68 tests passing ✅ Credit: Solution from @Elecweb's PR #96 * Add regression test for #47 (submitErrors removal) Verifies that PR #48's fix continues to work correctly. Test confirms submitErrors are properly removed when array items are removed via fields.remove(). --------- Co-authored-by: Erik Rasmussen <erik@mini.local>
1 parent b115aa3 commit 656916f

5 files changed

Lines changed: 203 additions & 1 deletion

File tree

src/insert.test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,11 @@ describe('insert', () => {
149149
touched: true,
150150
error: 'A Error'
151151
} as any,
152+
'foo[1]': {
153+
name: 'foo[1]',
154+
touched: false,
155+
error: 'B Error'
156+
} as any,
152157
'foo[2]': {
153158
name: 'foo[2]',
154159
touched: true,
@@ -226,6 +231,11 @@ describe('insert', () => {
226231
touched: true,
227232
error: 'A Error'
228233
} as any,
234+
'foo[0][1]': {
235+
name: 'foo[0][1]',
236+
touched: false,
237+
error: 'B Error'
238+
} as any,
229239
'foo[0][2]': {
230240
name: 'foo[0][2]',
231241
touched: true,

src/insert.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { escapeRegexTokens } from './utils'
55
const insert: Mutator<any> = (
66
[name, index, value]: any[],
77
state: MutableState<any>,
8-
{ changeValue }: Tools<any>
8+
{ changeValue, resetFieldState }: Tools<any>
99
): void => {
1010
changeValue(state, name, (array?: any[]): any[] => {
1111
const copy = [...(array || [])]
@@ -24,6 +24,11 @@ const insert: Mutator<any> = (
2424
// Shift all higher indices up
2525
const incrementedKey = `${name}[${fieldIndex + 1}]${tokens[2]}`
2626
copyField(state.fields, key, newFields, incrementedKey)
27+
if (fieldIndex === index) {
28+
// Keep field at insertion index and reset to defaults
29+
newFields[key] = state.fields[key]
30+
resetFieldState(key)
31+
}
2732
return
2833
}
2934
}

src/remove.submiterrors-47.test.ts

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
import remove from './remove'
2+
import { getIn, setIn, MutableState } from 'final-form'
3+
import { createMockTools } from './testUtils'
4+
5+
describe('remove submitErrors regression #47', () => {
6+
it('should remove submitErrors when removing array item', () => {
7+
const changeValue = jest.fn((state: any, name: string, mutate: (value: any) => any) => {
8+
const before = getIn(state.formState.values, name)
9+
const after = mutate(before)
10+
state.formState.values = setIn(state.formState.values, name, after) || {} as any
11+
})
12+
13+
const state: MutableState<any> = {
14+
formState: {
15+
values: {
16+
customers: [
17+
{ name: 'Alice' },
18+
{ name: 'Bob' },
19+
{ name: 'Charlie' }
20+
]
21+
} as any,
22+
submitErrors: {
23+
customers: [
24+
{ name: 'Error for Alice' },
25+
{ name: 'Error for Bob' },
26+
{ name: 'Error for Charlie' }
27+
]
28+
}
29+
},
30+
fields: {
31+
'customers[0]': { name: 'customers[0]' } as any,
32+
'customers[0].name': { name: 'customers[0].name' } as any,
33+
'customers[1]': { name: 'customers[1]' } as any,
34+
'customers[1].name': { name: 'customers[1].name' } as any,
35+
'customers[2]': { name: 'customers[2]' } as any,
36+
'customers[2].name': { name: 'customers[2].name' } as any
37+
}
38+
}
39+
40+
// Remove middle item (index 1 - Bob)
41+
remove(['customers', 1], state, createMockTools({ changeValue, getIn, setIn }))
42+
43+
// Values should have Bob removed
44+
expect(state.formState.values.customers).toHaveLength(2)
45+
expect(state.formState.values.customers[0].name).toBe('Alice')
46+
expect(state.formState.values.customers[1].name).toBe('Charlie')
47+
48+
// Submit errors should also have Bob's error removed
49+
expect(state.formState.submitErrors.customers).toHaveLength(2)
50+
expect(state.formState.submitErrors.customers[0].name).toBe('Error for Alice')
51+
expect(state.formState.submitErrors.customers[1].name).toBe('Error for Charlie')
52+
})
53+
54+
it('should handle removing first item with submitErrors', () => {
55+
const changeValue = jest.fn((state: any, name: string, mutate: (value: any) => any) => {
56+
const before = getIn(state.formState.values, name)
57+
const after = mutate(before)
58+
state.formState.values = setIn(state.formState.values, name, after) || {} as any
59+
})
60+
61+
const state: MutableState<any> = {
62+
formState: {
63+
values: {
64+
items: ['A', 'B', 'C']
65+
} as any,
66+
submitErrors: {
67+
items: ['Error A', 'Error B', 'Error C']
68+
}
69+
},
70+
fields: {
71+
'items[0]': { name: 'items[0]' } as any,
72+
'items[1]': { name: 'items[1]' } as any,
73+
'items[2]': { name: 'items[2]' } as any
74+
}
75+
}
76+
77+
// Remove first item
78+
remove(['items', 0], state, createMockTools({ changeValue, getIn, setIn }))
79+
80+
expect(state.formState.values.items).toEqual(['B', 'C'])
81+
expect(state.formState.submitErrors.items).toEqual(['Error B', 'Error C'])
82+
})
83+
84+
it('should not crash when no submitErrors exist', () => {
85+
const changeValue = jest.fn((state: any, name: string, mutate: (value: any) => any) => {
86+
const before = getIn(state.formState.values, name)
87+
const after = mutate(before)
88+
state.formState.values = setIn(state.formState.values, name, after) || {} as any
89+
})
90+
91+
const state: MutableState<any> = {
92+
formState: {
93+
values: {
94+
items: ['A', 'B']
95+
} as any
96+
// No submitErrors
97+
},
98+
fields: {
99+
'items[0]': { name: 'items[0]' } as any,
100+
'items[1]': { name: 'items[1]' } as any
101+
}
102+
}
103+
104+
// Should not crash
105+
expect(() => {
106+
remove(['items', 0], state, createMockTools({ changeValue, getIn, setIn }))
107+
}).not.toThrow()
108+
109+
expect(state.formState.values.items).toEqual(['B'])
110+
})
111+
})

src/unshift.regression-44.test.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import unshift from './unshift'
2+
import { getIn, setIn, MutableState } from 'final-form'
3+
import { createMockTools } from './testUtils'
4+
5+
describe('unshift regression #44', () => {
6+
it('should properly initialize unshifted field values', () => {
7+
const changeValue = jest.fn((state: any, name: string, mutate: (value: any) => any) => {
8+
const before = getIn(state.formState.values, name)
9+
const after = mutate(before)
10+
state.formState.values = setIn(state.formState.values, name, after) || {} as any
11+
})
12+
13+
// Start with one customer
14+
const state: MutableState<any> = {
15+
formState: {
16+
values: {
17+
customers: [{ firstName: 'John', lastName: 'Doe' }]
18+
} as any
19+
},
20+
fields: {
21+
'customers[0]': { name: 'customers[0]' } as any,
22+
'customers[0].firstName': { name: 'customers[0].firstName' } as any,
23+
'customers[0].lastName': { name: 'customers[0].lastName' } as any
24+
}
25+
}
26+
27+
// Unshift a new customer
28+
unshift(['customers', { firstName: 'Jane', lastName: 'Smith' }], state, createMockTools({ changeValue }))
29+
30+
const customers = state.formState.values.customers
31+
32+
// Verify new customer is at index 0
33+
expect(customers).toHaveLength(2)
34+
expect(customers[0]).toEqual({ firstName: 'Jane', lastName: 'Smith' })
35+
expect(customers[1]).toEqual({ firstName: 'John', lastName: 'Doe' })
36+
37+
// Verify field keys were shifted
38+
expect(state.fields['customers[0]']).toBeDefined()
39+
expect(state.fields['customers[1]']).toBeDefined()
40+
expect(state.fields['customers[0].firstName']).toBeDefined()
41+
expect(state.fields['customers[0].lastName']).toBeDefined()
42+
expect(state.fields['customers[1].firstName']).toBeDefined()
43+
expect(state.fields['customers[1].lastName']).toBeDefined()
44+
})
45+
46+
it('should not have null values after unshift', () => {
47+
const changeValue = jest.fn((state: any, name: string, mutate: (value: any) => any) => {
48+
const before = getIn(state.formState.values, name)
49+
const after = mutate(before)
50+
state.formState.values = setIn(state.formState.values, name, after) || {} as any
51+
})
52+
53+
const state: MutableState<any> = {
54+
formState: {
55+
values: {
56+
items: ['first']
57+
} as any
58+
},
59+
fields: {
60+
'items[0]': { name: 'items[0]' } as any
61+
}
62+
}
63+
64+
// Unshift should add the value, not null
65+
unshift(['items', 'prepended'], state, createMockTools({ changeValue }))
66+
67+
expect(state.formState.values.items[0]).toBe('prepended')
68+
expect(state.formState.values.items[0]).not.toBeNull()
69+
expect(state.formState.values.items[1]).toBe('first')
70+
})
71+
})

src/unshift.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,11 @@ describe('unshift', () => {
113113
}
114114
},
115115
fields: {
116+
'foo[0]': {
117+
name: 'foo[0]',
118+
touched: false,
119+
error: 'A Error'
120+
} as any,
116121
'foo[1]': {
117122
name: 'foo[1]',
118123
touched: true,

0 commit comments

Comments
 (0)