Skip to content
This repository was archived by the owner on Aug 23, 2022. It is now read-only.

Commit 5367a7a

Browse files
davidkpianodavidkpiano
committed
Simplifying and merging behavior of validity and errors (refactor). Fixes #151 
1 parent 90c216f commit 5367a7a

File tree

7 files changed

+133
-74
lines changed

7 files changed

+133
-74
lines changed

‎src/components/form-component.js

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,16 @@ import React, { Component, PropTypes } from 'react';
22
import connect from 'react-redux/lib/components/connect';
33
import _get from 'lodash/get';
44
import mapValues from 'lodash/mapValues';
5+
import merge from 'lodash/merge';
56

67
import actions from '../actions';
7-
import { getValidity, getForm, invertValidators } from '../utils';
8+
import {
9+
getValidity,
10+
getForm,
11+
invertValidators,
12+
invertValidity,
13+
} from '../utils';
14+
import { getField } from '../reducers/form-reducer';
815

916
function dispatchValidCallback(modelValue, callback) {
1017
return callback
@@ -41,35 +48,47 @@ class Form extends Component {
4148
errors,
4249
model,
4350
dispatch,
51+
formValue,
4452
} = this.props;
4553

46-
/* eslint-disable consistent-return */
47-
mapValues(validators, (validator, field) => {
54+
let validityChanged = false;
55+
56+
const fieldsValidity = mapValues(validators, (validator, field) => {
4857
const fieldModel = [model, field].join('.');
4958
const value = _get(nextProps, fieldModel);
5059

51-
if (!initial && (value === _get(this.props, fieldModel))) return;
60+
if (!initial && (value === _get(this.props, fieldModel))) {
61+
return getField(formValue, fieldModel).validity;
62+
}
5263

53-
const fieldValidity = getValidity(validator, value);
64+
validityChanged = true;
5465

55-
dispatch(actions.setValidity(fieldModel, fieldValidity));
66+
const fieldValidity = getValidity(validator, value);
5667

5768
return fieldValidity;
5869
});
5970

60-
mapValues(errors, (errorValidator, field) => {
71+
const fieldsErrorsValidity = mapValues(errors, (errorValidator, field) => {
6172
const fieldModel = [model, field].join('.');
6273
const value = _get(nextProps, fieldModel);
6374

64-
if (!initial && (value === _get(this.props, fieldModel))) return;
75+
if (!initial && (value === _get(this.props, fieldModel))) {
76+
return getField(formValue, fieldModel).errors;
77+
}
6578

66-
const fieldErrors = getValidity(errorValidator, value);
79+
validityChanged = true;
6780

68-
dispatch(actions.setErrors(fieldModel, fieldErrors));
81+
const fieldErrors = getValidity(errorValidator, value);
6982

7083
return fieldErrors;
7184
});
72-
/* eslint-enable consistent-return */
85+
86+
if (validityChanged) {
87+
dispatch(actions.setFieldsErrors(model, merge(
88+
invertValidity(fieldsValidity),
89+
fieldsErrorsValidity
90+
)));
91+
}
7392
}
7493

7594
handleSubmit(e) {
@@ -103,10 +122,10 @@ class Form extends Component {
103122

104123
dispatch(actions.validateFieldsErrors(
105124
model,
106-
{
107-
...invertValidators(validators),
108-
...errors,
109-
},
125+
merge(
126+
invertValidators(validators),
127+
errors
128+
),
110129
validationOptions));
111130

112131
return modelValue;

‎src/reducers/form-reducer.js

Lines changed: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,19 @@ function setField(state, localPath, props) {
7171
});
7272
}
7373

74+
function setInField(state, localPath, props) {
75+
if (!localPath.length) {
76+
return icepick.assign(state, props);
77+
}
78+
79+
const field = _get(state, ['fields', localPath.join('.')], initialFieldState);
80+
81+
return icepick.setIn(
82+
state,
83+
['fields', localPath.join('.')],
84+
icepick.assign(field, props));
85+
}
86+
7487
function resetField(state, localPath) {
7588
return icepick.setIn(
7689
state,
@@ -181,23 +194,14 @@ function _createFormReducer(model, initialState) {
181194

182195
case actionTypes.SET_VALIDITY: {
183196
if (isPlainObject(action.validity)) {
184-
validity = icepick.merge(
185-
getField(state, localPath).validity,
186-
action.validity
187-
);
188-
189-
errors = {
190-
...getField(state, localPath).errors,
191-
...mapValues(action.validity, valid => !valid),
192-
};
197+
errors = mapValues(action.validity, valid => !valid);
193198
} else {
194-
validity = action.validity;
195199
errors = !action.validity;
196200
}
197201

198-
const formIsValidState = setField(state, localPath, {
202+
const formIsValidState = setInField(state, localPath, {
199203
errors,
200-
validity,
204+
validity: action.validity,
201205
valid: isBoolean(errors) ? !errors : every(errors, error => !error),
202206
});
203207

@@ -208,27 +212,18 @@ function _createFormReducer(model, initialState) {
208212

209213
case actionTypes.SET_FIELDS_VALIDITY:
210214
return map(action.fieldsValidity, (fieldValidity, field) =>
211-
actions.setValidity(`${model}.${field}`, fieldValidity, action.options))
212-
.reduce(formReducer, state);
215+
actions.setValidity(`${model}.${field}`, fieldValidity, action.options)
216+
).reduce(formReducer, state);
213217

214218
case actionTypes.SET_ERRORS: {
215219
if (isPlainObject(action.errors)) {
216-
validity = {
217-
...getField(state, localPath).validity,
218-
...mapValues(action.errors, error => !error),
219-
};
220-
221-
errors = icepick.merge(
222-
getField(state, localPath).errors,
223-
action.errors
224-
);
220+
validity = mapValues(action.errors, error => !error);
225221
} else {
226222
validity = !action.errors;
227-
errors = action.errors;
228223
}
229224

230-
const setErrorsState = setField(state, localPath, {
231-
errors,
225+
const setErrorsState = setInField(state, localPath, {
226+
errors: action.errors,
232227
validity,
233228
valid: isValid(validity),
234229
});

‎src/utils/index.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,14 @@ function invertValidators(validators) {
8282
return mapValues(validators, invertValidators);
8383
}
8484

85+
function invertValidity(validity) {
86+
if (isPlainObject(validity)) {
87+
return mapValues(validity, invertValidity);
88+
}
89+
90+
return !validity;
91+
}
92+
8593
function isValid(validity) {
8694
if (isPlainObject(validity)) {
8795
return every(validity, isValid);
@@ -111,4 +119,5 @@ export {
111119
getForm,
112120
getFieldFromState,
113121
invertValidators,
122+
invertValidity,
114123
};

‎src/utils/sequence.js

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,17 @@ import mapValues from 'lodash/mapValues';
55
import compose from 'lodash/fp/compose';
66
import isEqual from 'lodash/isEqual';
77
import memoize from 'lodash/memoize';
8+
import merge from 'lodash/merge';
89
import icepick from 'icepick';
910

10-
import { isMulti, getValue, getValidity } from './index';
11+
import { isMulti, getValue, getValidity, invertValidity } from './index';
1112
import actions from '../actions';
1213

1314
const {
1415
asyncSetValidity,
1516
blur,
1617
focus,
17-
setValidity,
18+
setErrors,
1819
} = actions;
1920

2021
function persistEventWithCallback(callback) {
@@ -109,15 +110,13 @@ function sequenceEventActions(props) {
109110

110111
if (props.validators || props.errors) {
111112
const dispatchValidate = value => {
112-
if (props.validators) {
113-
dispatch(setValidity(model, getValidity(props.validators, value)));
114-
}
115-
116-
if (props.errors) {
117-
dispatch(setValidity(model, getValidity(props.errors, value), {
118-
errors: true,
119-
}));
120-
}
113+
const fieldValidity = getValidity(props.validators, value);
114+
const fieldErrors = getValidity(props.errors, value);
115+
116+
dispatch(setErrors(model, merge(
117+
invertValidity(fieldValidity),
118+
fieldErrors
119+
)));
121120

122121
return value;
123122
};

‎test/field-actions-spec.js

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -628,11 +628,9 @@ describe('field actions', () => {
628628
valid: false,
629629
validity: {
630630
foo: false,
631-
baz: null,
632631
},
633632
errors: {
634633
foo: true,
635-
baz: true,
636634
},
637635
});
638636

@@ -655,6 +653,42 @@ describe('field actions', () => {
655653
},
656654
});
657655
});
656+
657+
it('should overwrite the previous validity', () => {
658+
const reducer = formReducer('test');
659+
660+
const oldValidity = {
661+
existing: true,
662+
};
663+
664+
const oldState = reducer(
665+
undefined,
666+
actions.setValidity('test', oldValidity));
667+
668+
assert.deepEqual(
669+
oldState.validity,
670+
oldValidity);
671+
672+
const newValidity = {
673+
foo: true,
674+
bar: false,
675+
};
676+
677+
const newState = reducer(
678+
oldState,
679+
actions.setValidity('test', newValidity));
680+
681+
assert.deepEqual(
682+
newState.validity,
683+
newValidity);
684+
685+
assert.deepEqual(
686+
newState.errors,
687+
{
688+
foo: false,
689+
bar: true,
690+
});
691+
});
658692
});
659693

660694
describe('setErrors()', () => {
@@ -826,11 +860,9 @@ describe('field actions', () => {
826860
valid: true,
827861
validity: {
828862
foo: true,
829-
baz: true,
830863
},
831864
errors: {
832865
foo: false,
833-
baz: null,
834866
},
835867
});
836868

‎test/field-component-spec.js

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -585,30 +585,24 @@ describe('<Field /> component', () => {
585585
TestUtils.Simulate.change(checkboxes[0]);
586586

587587
assert.isTrue(store.getState().testForm.fields.items.valid);
588-
assert.equal(
589-
store.getState().testForm.fields.items.validity.required,
590-
1);
588+
assert.isTrue(
589+
store.getState().testForm.fields.items.validity.required);
591590

592591
TestUtils.Simulate.change(checkboxes[1]);
593-
assert.equal(
594-
store.getState().testForm.fields.items.validity.required,
595-
2);
596-
assert.deepEqual(
597-
store.getState().testForm.fields.items.validity.values,
598-
['first', 'second']);
592+
assert.isTrue(
593+
store.getState().testForm.fields.items.validity.required);
594+
assert.isTrue(
595+
store.getState().testForm.fields.items.validity.values);
599596

600597
TestUtils.Simulate.change(checkboxes[0]);
601-
assert.equal(
602-
store.getState().testForm.fields.items.validity.required,
603-
1);
604-
assert.deepEqual(
605-
store.getState().testForm.fields.items.validity.values,
606-
['second']);
598+
assert.isTrue(
599+
store.getState().testForm.fields.items.validity.required);
600+
assert.isTrue(
601+
store.getState().testForm.fields.items.validity.values);
607602

608603
TestUtils.Simulate.change(checkboxes[1]);
609-
assert.equal(
610-
store.getState().testForm.fields.items.validity.required,
611-
0);
604+
assert.isFalse(
605+
store.getState().testForm.fields.items.validity.required);
612606
assert.isFalse(store.getState().testForm.fields.items.valid);
613607
});
614608
});

‎test/form-component-spec.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,9 @@ describe('<Form> component', () => {
527527

528528
TestUtils.Simulate.change(fooControl);
529529

530+
assert.isTrue(
531+
store.getState().testForm.fields.foo.valid);
532+
530533
assert.isNull(submitValue);
531534
});
532535

@@ -550,6 +553,14 @@ describe('<Form> component', () => {
550553

551554
TestUtils.Simulate.change(bazControl);
552555

556+
assert.deepEqual(
557+
store.getState().test,
558+
{
559+
foo: 'valid',
560+
bar: 'bar',
561+
baz: 'valid',
562+
});
563+
553564
TestUtils.Simulate.submit(formElement);
554565

555566
assert.deepEqual(

0 commit comments

Comments
 (0)