Skip to content

Commit f69ab19

Browse files
committed
Fix remove mutator removing more fields than expected.
Revert changes from #39 that moves `fieldSubscribers`, causing the wrong field to be notified.
1 parent 4a65647 commit f69ab19

File tree

3 files changed

+84
-110
lines changed

3 files changed

+84
-110
lines changed

src/copyField.js

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// @flow
2+
import type { InternalFieldState } from 'final-form/dist/types'
3+
4+
function copyField(
5+
oldFields: { [string]: InternalFieldState },
6+
oldKey: string,
7+
newFields: { [string]: InternalFieldState },
8+
newKey: string
9+
) {
10+
newFields[newKey] = {
11+
...oldFields[oldKey],
12+
name: newKey,
13+
// prevent functions from being overwritten
14+
// if the newFields[newKey] does not exist, it will be created
15+
// when that field gets registered, with its own change/blur/focus callbacks
16+
change: oldFields[newKey] && oldFields[newKey].change,
17+
blur: oldFields[newKey] && oldFields[newKey].blur,
18+
focus: oldFields[newKey] && oldFields[newKey].focus,
19+
lastFieldState: undefined // clearing lastFieldState forces renotification
20+
}
21+
22+
if (!newFields[newKey].change) {
23+
delete newFields[newKey].change
24+
}
25+
26+
if (!newFields[newKey].blur) {
27+
delete newFields[newKey].blur
28+
}
29+
30+
if (!newFields[newKey].focus) {
31+
delete newFields[newKey].focus
32+
}
33+
}
34+
35+
export default copyField

src/remove.js

+17-14
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
// @flow
22
import type { MutableState, Mutator, Tools } from 'final-form'
3-
import moveFieldState from './moveFieldState'
3+
import copyField from './copyField'
44
import { escapeRegexTokens } from './utils'
55

66
const remove: Mutator<any> = (
77
[name, index]: any[],
88
state: MutableState<any>,
9-
{ changeValue, renameField, getIn, setIn }: Tools<any>
9+
{ changeValue, getIn, setIn }: Tools<any>
1010
) => {
1111
let returnValue
1212
changeValue(state, name, (array: ?(any[])): any[] => {
@@ -19,14 +19,12 @@ const remove: Mutator<any> = (
1919
// now we have to remove any subfields for our index,
2020
// and decrement all higher indexes.
2121
const pattern = new RegExp(`^${escapeRegexTokens(name)}\\[(\\d+)\\](.*)`)
22-
const backup = { ...state, fields: { ...state.fields } }
22+
const newFields = {}
2323
Object.keys(state.fields).forEach(key => {
2424
const tokens = pattern.exec(key)
2525
if (tokens) {
2626
const fieldIndex = Number(tokens[1])
2727
if (fieldIndex === index) {
28-
// delete any subfields for this array item
29-
delete state.fields[key]
3028
// delete any submitErrors for this array item
3129
// if the root key of the delete index
3230
if (key === `${name}[${index}]`) {
@@ -38,19 +36,24 @@ const remove: Mutator<any> = (
3836
state = setIn(state, path, submitErrors)
3937
}
4038
}
41-
} else if (fieldIndex > index) {
42-
// shift all higher ones down
43-
delete state.fields[key]
39+
40+
return
41+
}
42+
43+
if (fieldIndex > index) {
44+
// Shift all higher indices down
4445
const decrementedKey = `${name}[${fieldIndex - 1}]${tokens[2]}`
45-
if (backup.fields[decrementedKey]) {
46-
moveFieldState(state, backup.fields[key], decrementedKey, backup)
47-
} else {
48-
// take care of setting the correct change, blur, focus, validators on new field
49-
renameField(state, key, decrementedKey)
50-
}
46+
copyField(state.fields, key, newFields, decrementedKey)
47+
return
5148
}
5249
}
50+
51+
// Keep this field that does not match the name,
52+
// or has index smaller than what is being removed
53+
newFields[key] = state.fields[key]
5354
})
55+
56+
state.fields = newFields
5457
return returnValue
5558
}
5659

src/remove.test.js

+32-96
Original file line numberDiff line numberDiff line change
@@ -94,14 +94,6 @@ describe('remove', () => {
9494
touched: false,
9595
error: 'B Error'
9696
},
97-
'foo[2]': {
98-
name: 'foo[2]',
99-
blur: blur2,
100-
change: change2,
101-
focus: focus2,
102-
touched: true,
103-
error: 'C Error'
104-
},
10597
'foo[3]': {
10698
name: 'foo[3]',
10799
blur: blur3,
@@ -110,6 +102,14 @@ describe('remove', () => {
110102
touched: false,
111103
error: 'D Error'
112104
},
105+
'foo[2]': {
106+
name: 'foo[2]',
107+
blur: blur2,
108+
change: change2,
109+
focus: focus2,
110+
touched: true,
111+
error: 'C Error'
112+
},
113113
anotherField: {
114114
name: 'anotherField',
115115
touched: false
@@ -135,15 +135,6 @@ describe('remove', () => {
135135
touched: true,
136136
error: 'A Error'
137137
},
138-
'foo[1]': {
139-
name: 'foo[1]',
140-
blur: blur1,
141-
change: change1,
142-
focus: focus1,
143-
touched: true,
144-
error: 'C Error',
145-
lastFieldState: undefined
146-
},
147138
'foo[2]': {
148139
name: 'foo[2]',
149140
blur: blur2,
@@ -153,6 +144,15 @@ describe('remove', () => {
153144
error: 'D Error',
154145
lastFieldState: undefined
155146
},
147+
'foo[1]': {
148+
name: 'foo[1]',
149+
blur: blur1,
150+
change: change1,
151+
focus: focus1,
152+
touched: true,
153+
error: 'C Error',
154+
lastFieldState: undefined
155+
},
156156
anotherField: {
157157
name: 'anotherField',
158158
touched: false
@@ -189,6 +189,14 @@ describe('remove', () => {
189189
}
190190
},
191191
fields: {
192+
'foo[0][3]': {
193+
name: 'foo[0][3]',
194+
blur: blur3,
195+
change: change3,
196+
focus: focus3,
197+
touched: false,
198+
error: 'D Error'
199+
},
192200
'foo[0][0]': {
193201
name: 'foo[0][0]',
194202
blur: blur0,
@@ -213,14 +221,6 @@ describe('remove', () => {
213221
touched: true,
214222
error: 'C Error'
215223
},
216-
'foo[0][3]': {
217-
name: 'foo[0][3]',
218-
blur: blur3,
219-
change: change3,
220-
focus: focus3,
221-
touched: false,
222-
error: 'D Error'
223-
},
224224
anotherField: {
225225
name: 'anotherField',
226226
touched: false
@@ -242,23 +242,6 @@ describe('remove', () => {
242242
}
243243
},
244244
fields: {
245-
'foo[0][0]': {
246-
name: 'foo[0][0]',
247-
blur: blur0,
248-
change: change0,
249-
focus: focus0,
250-
touched: true,
251-
error: 'A Error'
252-
},
253-
'foo[0][1]': {
254-
name: 'foo[0][1]',
255-
blur: blur1,
256-
change: change1,
257-
focus: focus1,
258-
touched: true,
259-
error: 'C Error',
260-
lastFieldState: undefined
261-
},
262245
'foo[0][2]': {
263246
name: 'foo[0][2]',
264247
blur: blur2,
@@ -268,76 +251,29 @@ describe('remove', () => {
268251
error: 'D Error',
269252
lastFieldState: undefined
270253
},
271-
anotherField: {
272-
name: 'anotherField',
273-
touched: false
274-
}
275-
}
276-
})
277-
})
278-
279-
it('should remove value from the specified index, and handle new fields', () => {
280-
const array = ['a', { key: 'val' }]
281-
const changeValue = jest.fn()
282-
const renameField = jest.fn()
283-
function blur0() {}
284-
function change0() {}
285-
function focus0() {}
286-
function blur1() {}
287-
function change1() {}
288-
function focus1() {}
289-
function blur2() {}
290-
function change2() {}
291-
function focus2() {}
292-
const state = {
293-
formState: {
294-
values: {
295-
foo: array,
296-
anotherField: 42
297-
}
298-
},
299-
fields: {
300-
'foo[0]': {
301-
name: 'foo[0]',
254+
'foo[0][0]': {
255+
name: 'foo[0][0]',
302256
blur: blur0,
303257
change: change0,
304258
focus: focus0,
305259
touched: true,
306260
error: 'A Error'
307261
},
308-
'foo[1]': {
309-
name: 'foo[1]',
262+
'foo[0][1]': {
263+
name: 'foo[0][1]',
310264
blur: blur1,
311265
change: change1,
312266
focus: focus1,
313-
touched: false,
314-
error: 'B Error'
315-
},
316-
'foo[1].key': {
317-
name: 'foo[1].key',
318-
blur: blur2,
319-
change: change2,
320-
focus: focus2,
321-
touched: false,
322-
error: 'B Error'
267+
touched: true,
268+
error: 'C Error',
269+
lastFieldState: undefined
323270
},
324271
anotherField: {
325272
name: 'anotherField',
326273
touched: false
327274
}
328275
}
329-
}
330-
const returnValue = remove(['foo', 0], state, {
331-
renameField,
332-
changeValue,
333-
getIn,
334-
setIn
335276
})
336-
expect(returnValue).toBeUndefined()
337-
expect(renameField).toHaveBeenCalledTimes(1)
338-
expect(renameField.mock.calls[0][0]).toEqual(state)
339-
expect(renameField.mock.calls[0][1]).toEqual('foo[1].key')
340-
expect(renameField.mock.calls[0][2]).toEqual('foo[0].key')
341277
})
342278

343279
it('should remove value from the specified index with submitError if one error in array', () => {

0 commit comments

Comments
 (0)