From e1dfd226790d51c711aa764caac961c567ad1a7a Mon Sep 17 00:00:00 2001 From: Erik Rasmussen Date: Wed, 17 Jul 2019 08:37:48 -0400 Subject: [PATCH] Fixed myriad bugs with moving field state --- package-lock.json | 6 +-- package.json | 4 +- src/insert.js | 8 ++-- src/move.js | 38 ++++++------------- src/moveFieldState.js | 23 ++++++++++++ src/remove.js | 7 ++-- src/remove.test.js | 33 +++++++++++++++++ src/removeBatch.js | 11 +++--- src/removeBatch.test.js | 81 +++++++++++++++++++++++++++++++++++++++++ src/swap.js | 22 ++--------- 10 files changed, 168 insertions(+), 65 deletions(-) create mode 100644 src/moveFieldState.js diff --git a/package-lock.json b/package-lock.json index 170599a..40c3593 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4188,9 +4188,9 @@ } }, "final-form": { - "version": "4.18.0", - "resolved": "https://registry.npmjs.org/final-form/-/final-form-4.18.0.tgz", - "integrity": "sha512-0ROl7GOFRWV3Pg6FmHx5G8tCZ8jAF6laVD81e9buxEocHbWMml3fYSpKZRQIOMh6SosYtQTk/tDf/vnamksO2w==", + "version": "4.18.2", + "resolved": "https://registry.npmjs.org/final-form/-/final-form-4.18.2.tgz", + "integrity": "sha512-VQx/5x9M4CiC8fG678Dm1IS3mXvBl7ZNIUx5tUZCk00lFImJzQix4KO0+eGtl49sha2bYOxuYn8jRJiq6sazXA==", "dev": true, "requires": { "@babel/runtime": "^7.3.1" diff --git a/package.json b/package.json index 04cf79b..49c374f 100644 --- 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 index 30797b0..421f3f3 100644 --- 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 = ( [name, index, value]: any[], @@ -14,7 +15,7 @@ const insert: Mutator = ( // 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) { @@ -22,16 +23,13 @@ const insert: Mutator = ( 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 index 3aae096..69c4bde 100644 --- 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 = ( [name, from, to]: any[], @@ -27,43 +28,28 @@ const move: Mutator = ( // 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 new file mode 100644 index 0000000..56c93dc --- /dev/null +++ b/src/moveFieldState.js @@ -0,0 +1,23 @@ +// @flow +import type { MutableState } from 'final-form' + +function moveFieldState( + state: MutableState, + source: Object, + destKey: string, + oldState: MutableState = 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 index 6704737..98ef5fc 100644 --- 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 = ( [name, index]: any[], @@ -17,7 +18,7 @@ const remove: Mutator = ( // 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 = ( // 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 index 94ce9a9..4a22ad4 100644 --- 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 index 3bae81e..80f836e 100644 --- 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 = ( // 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 = ( // 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 } diff --git a/src/removeBatch.test.js b/src/removeBatch.test.js index c69209b..81741ea 100644 --- a/src/removeBatch.test.js +++ b/src/removeBatch.test.js @@ -34,6 +34,15 @@ describe('removeBatch', () => { 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() {} const state = { formState: { values: { @@ -43,16 +52,25 @@ describe('removeBatch', () => { fields: { 'foo[0]': { name: 'foo[0]', + blur: blur0, + change: change0, + focus: focus0, touched: true, error: 'First Error' }, 'foo[1]': { name: 'foo[1]', + blur: blur1, + change: change1, + focus: focus1, touched: false, error: 'Second Error' }, 'foo[2]': { name: 'foo[2]', + blur: blur2, + change: change2, + focus: focus2, touched: true, error: 'Third Error' } @@ -75,6 +93,9 @@ describe('removeBatch', () => { fields: { 'foo[0]': { name: 'foo[0]', + blur: blur0, + change: change0, + focus: focus0, touched: true, error: 'First Error', lastFieldState: undefined @@ -90,6 +111,15 @@ describe('removeBatch', () => { 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() {} const state = { formState: { values: { @@ -99,16 +129,25 @@ describe('removeBatch', () => { fields: { 'foo[0]': { name: 'foo[0]', + blur: blur0, + change: change0, + focus: focus0, touched: true, error: 'First Error' }, 'foo[1]': { name: 'foo[1]', + blur: blur1, + change: change1, + focus: focus1, touched: false, error: 'Second Error' }, 'foo[2]': { name: 'foo[2]', + blur: blur2, + change: change2, + focus: focus2, touched: true, error: 'Third Error' } @@ -131,6 +170,9 @@ describe('removeBatch', () => { fields: { 'foo[0]': { name: 'foo[0]', + blur: blur0, + change: change0, + focus: focus0, touched: false, error: 'Second Error', lastFieldState: undefined @@ -174,6 +216,21 @@ describe('removeBatch', () => { const after = mutate(before) state.formState.values = setIn(state.formState.values, name, after) || {} } + function blur0() {} + function blur1() {} + function blur2() {} + function blur3() {} + function blur4() {} + function change0() {} + function change1() {} + function change2() {} + function change3() {} + function change4() {} + function focus0() {} + function focus1() {} + function focus2() {} + function focus3() {} + function focus4() {} const state = { formState: { values: { @@ -184,26 +241,41 @@ describe('removeBatch', () => { 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' }, 'foo[4]': { name: 'foo[4]', + blur: blur4, + change: change4, + focus: focus4, touched: true, error: 'E Error' }, @@ -226,18 +298,27 @@ describe('removeBatch', () => { fields: { 'foo[0]': { name: 'foo[0]', + blur: blur0, + change: change0, + focus: focus0, touched: true, error: 'A Error', lastFieldState: undefined }, 'foo[1]': { name: 'foo[1]', + blur: blur1, + change: change1, + focus: focus1, touched: false, error: 'D Error', lastFieldState: undefined }, 'foo[2]': { name: 'foo[2]', + blur: blur2, + change: change2, + focus: focus2, touched: true, error: 'E Error', lastFieldState: undefined diff --git a/src/swap.js b/src/swap.js index d9abf39..0b33d40 100644 --- a/src/swap.js +++ b/src/swap.js @@ -1,5 +1,6 @@ // @flow import type { MutableState, Mutator, Tools } from 'final-form' +import moveFieldState from './moveFieldState' const swap: Mutator = ( [name, indexA, indexB]: any[], @@ -26,27 +27,10 @@ const swap: Mutator = ( const bKey = bPrefix + suffix const fieldA = state.fields[aKey] - moveFieldState({ - destKey: aKey, - source: state.fields[bKey] - }) - moveFieldState({ - destKey: bKey, - source: fieldA - }) + moveFieldState(state, state.fields[bKey], aKey) + moveFieldState(state, fieldA, bKey) } }) - - function moveFieldState({ destKey, source }) { - state.fields[destKey] = { - ...source, - name: destKey, - change: state.fields[destKey].change, // prevent functions from being overwritten - blur: state.fields[destKey].blur, - focus: state.fields[destKey].focus, - lastFieldState: undefined // clearing lastFieldState forces renotification - } - } } export default swap