Fixed myriad bugs with moving field state (#35)

final-form · Jul 17, 2019 · d0a198f · d0a198f
1 parent 629a7c0
commit d0a198f
Show file tree

Hide file tree

Showing 10 changed files with 168 additions and 65 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -52,7 +52,7 @@
     "eslint-plugin-import": "^2.16.0",
     "eslint-plugin-jsx-a11y": "^6.2.1",
     "eslint-plugin-react": "^7.13.0",
-    "final-form": "^4.18.0",
+    "final-form": "^4.18.2",
     "flow-bin": "^0.102.0",
     "glow": "^1.2.2",
     "husky": "^3.0.0",
@@ -73,7 +73,7 @@
     "typescript": "^3.5.3"
   },
   "peerDependencies": {
-    "final-form": "^4.18.0"
+    "final-form": "^4.18.2"
   },
   "lint-staged": {
     "*.{js*,ts*,json,md,css}": [

diff --git a/src/insert.js b/src/insert.js
@@ -1,5 +1,6 @@
 // @flow
 import type { MutableState, Mutator, Tools } from 'final-form'
+import moveFieldState from './moveFieldState'
 
 const insert: Mutator<any> = (
   [name, index, value]: any[],
@@ -14,24 +15,21 @@ const insert: Mutator<any> = (
 
   // now we have increment any higher indexes
   const pattern = new RegExp(`^${name}\\[(\\d+)\\](.*)`)
-  const changes = {}
+  const backup = { ...state.fields }
   Object.keys(state.fields).forEach(key => {
     const tokens = pattern.exec(key)
     if (tokens) {
       const fieldIndex = Number(tokens[1])
       if (fieldIndex >= index) {
         // inc index one higher
         const incrementedKey = `${name}[${fieldIndex + 1}]${tokens[2]}`
-        changes[incrementedKey] = { ...state.fields[key] } // make copy of field state
-        changes[incrementedKey].name = incrementedKey
-        changes[incrementedKey].lastFieldState = undefined
+        moveFieldState(state, backup[key], incrementedKey)
       }
       if (fieldIndex === index) {
         resetFieldState(key)
       }
     }
   })
-  state.fields = { ...state.fields, ...changes }
 }
 
 export default insert
diff --git a/src/move.js b/src/move.js
@@ -1,5 +1,6 @@
 // @flow
 import type { MutableState, Mutator, Tools } from 'final-form'
+import moveFieldState from './moveFieldState'
 
 const move: Mutator<any> = (
   [name, from, to]: any[],
@@ -27,43 +28,28 @@ const move: Mutator<any> = (
         // decrement all indices between from and to
         for (let i = from; i < to; i++) {
           const destKey = `${name}[${i}]${suffix}`
-          moveFieldState({
-            destKey,
-            source: state.fields[`${name}[${i + 1}]${suffix}`]
-          })
+          moveFieldState(
+            state,
+            state.fields[`${name}[${i + 1}]${suffix}`],
+            destKey
+          )
         }
       } else {
         // moving to a lower index
         // increment all indices between to and from
         for (let i = from; i > to; i--) {
           const destKey = `${name}[${i}]${suffix}`
-          moveFieldState({
-            destKey,
-            source: state.fields[`${name}[${i - 1}]${suffix}`]
-          })
+          moveFieldState(
+            state,
+            state.fields[`${name}[${i - 1}]${suffix}`],
+            destKey
+          )
         }
       }
       const toKey = `${name}[${to}]${suffix}`
-      moveFieldState({
-        destKey: toKey,
-        source: backup
-      })
+      moveFieldState(state, backup, toKey)
     }
   })
-
-  function moveFieldState({ destKey, source }) {
-    state.fields[destKey] = {
-      ...source,
-      name: destKey,
-      // prevent functions from being overwritten
-      // if the state.fields[destKey] does not exist, it will be created
-      // when that field gets registered, with its own change/blur/focus callbacks
-      change: state.fields[destKey] && state.fields[destKey].change,
-      blur: state.fields[destKey] && state.fields[destKey].blur,
-      focus: state.fields[destKey] && state.fields[destKey].focus,
-      lastFieldState: undefined // clearing lastFieldState forces renotification
-    }
-  }
 }
 
 export default move
diff --git a/src/moveFieldState.js b/src/moveFieldState.js
@@ -0,0 +1,23 @@
+// @flow
+import type { MutableState } from 'final-form'
+
+function moveFieldState(
+  state: MutableState<any>,
+  source: Object,
+  destKey: string,
+  oldState: MutableState<any> = state
+) {
+  state.fields[destKey] = {
+    ...source,
+    name: destKey,
+    // prevent functions from being overwritten
+    // if the state.fields[destKey] does not exist, it will be created
+    // when that field gets registered, with its own change/blur/focus callbacks
+    change: oldState.fields[destKey] && oldState.fields[destKey].change,
+    blur: oldState.fields[destKey] && oldState.fields[destKey].blur,
+    focus: oldState.fields[destKey] && oldState.fields[destKey].focus,
+    lastFieldState: undefined // clearing lastFieldState forces renotification
+  }
+}
+
+export default moveFieldState
diff --git a/src/remove.js b/src/remove.js
@@ -1,5 +1,6 @@
 // @flow
 import type { MutableState, Mutator, Tools } from 'final-form'
+import moveFieldState from './moveFieldState'
 
 const remove: Mutator<any> = (
   [name, index]: any[],
@@ -17,7 +18,7 @@ const remove: Mutator<any> = (
   // now we have to remove any subfields for our index,
   // and decrement all higher indexes.
   const pattern = new RegExp(`^${name}\\[(\\d+)\\](.*)`)
-  const backup = { ...state.fields }
+  const backup = { ...state, fields: { ...state.fields } }
   Object.keys(state.fields).forEach(key => {
     const tokens = pattern.exec(key)
     if (tokens) {
@@ -29,9 +30,7 @@ const remove: Mutator<any> = (
         // shift all higher ones down
         delete state.fields[key]
         const decrementedKey = `${name}[${fieldIndex - 1}]${tokens[2]}`
-        state.fields[decrementedKey] = backup[key]
-        state.fields[decrementedKey].name = decrementedKey
-        state.fields[decrementedKey].lastFieldState = undefined
+        moveFieldState(state, backup.fields[key], decrementedKey, backup)
       }
     }
   })

diff --git a/src/remove.test.js b/src/remove.test.js
@@ -58,6 +58,18 @@ describe('remove', () => {
       const after = mutate(before)
       state.formState.values = setIn(state.formState.values, name, after) || {}
     }
+    function blur0() {}
+    function change0() {}
+    function focus0() {}
+    function blur1() {}
+    function change1() {}
+    function focus1() {}
+    function blur2() {}
+    function change2() {}
+    function focus2() {}
+    function blur3() {}
+    function change3() {}
+    function focus3() {}
     const state = {
       formState: {
         values: {
@@ -68,21 +80,33 @@ describe('remove', () => {
       fields: {
         'foo[0]': {
           name: 'foo[0]',
+          blur: blur0,
+          change: change0,
+          focus: focus0,
           touched: true,
           error: 'A Error'
         },
         'foo[1]': {
           name: 'foo[1]',
+          blur: blur1,
+          change: change1,
+          focus: focus1,
           touched: false,
           error: 'B Error'
         },
         'foo[2]': {
           name: 'foo[2]',
+          blur: blur2,
+          change: change2,
+          focus: focus2,
           touched: true,
           error: 'C Error'
         },
         'foo[3]': {
           name: 'foo[3]',
+          blur: blur3,
+          change: change3,
+          focus: focus3,
           touched: false,
           error: 'D Error'
         },
@@ -105,17 +129,26 @@ describe('remove', () => {
       fields: {
         'foo[0]': {
           name: 'foo[0]',
+          blur: blur0,
+          change: change0,
+          focus: focus0,
           touched: true,
           error: 'A Error'
         },
         'foo[1]': {
           name: 'foo[1]',
+          blur: blur1,
+          change: change1,
+          focus: focus1,
           touched: true,
           error: 'C Error',
           lastFieldState: undefined
         },
         'foo[2]': {
           name: 'foo[2]',
+          blur: blur2,
+          change: change2,
+          focus: focus2,
           touched: false,
           error: 'D Error',
           lastFieldState: undefined

diff --git a/src/removeBatch.js b/src/removeBatch.js
@@ -1,5 +1,6 @@
 // @flow
 import type { MutableState, Mutator, Tools } from 'final-form'
+import moveFieldState from './moveFieldState'
 
 const countBelow = (array, value) =>
   array.reduce((count, item) => (item < value ? count + 1 : count), 0)
@@ -38,7 +39,7 @@ const removeBatch: Mutator<any> = (
   // now we have to remove any subfields for our indexes,
   // and decrement all higher indexes.
   const pattern = new RegExp(`^${name}\\[(\\d+)\\](.*)`)
-  const newFields = {}
+  const newState = { ...state, fields: {} }
   Object.keys(state.fields).forEach(key => {
     const tokens = pattern.exec(key)
     if (tokens) {
@@ -48,15 +49,13 @@ const removeBatch: Mutator<any> = (
         // shift all higher ones down
         const decrementedKey = `${name}[${fieldIndex -
           countBelow(sortedIndexes, fieldIndex)}]${tokens[2]}`
-        newFields[decrementedKey] = state.fields[key]
-        newFields[decrementedKey].name = decrementedKey
-        newFields[decrementedKey].lastFieldState = undefined
+        moveFieldState(newState, state.fields[key], decrementedKey, state)
       }
     } else {
-      newFields[key] = state.fields[key]
+      newState.fields[key] = state.fields[key]
     }
   })
-  state.fields = newFields
+  state.fields = newState.fields
   return returnValue
 }