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

Commit 3599862

Browse files
davidkpianodavidkpiano
committed
Adding merge behavior for setErrors to prevent Control from clobbering validity. #585 
1 parent 4e72f72 commit 3599862

File tree

6 files changed

+58
-67
lines changed

6 files changed

+58
-67
lines changed

‎src/actions/field-actions.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ function createFieldActions(s = defaultStrategies) {
8282
? actionTypes.SET_ERRORS
8383
: actionTypes.SET_VALIDITY,
8484
model,
85+
...options,
8586
[options.errors ? 'errors' : 'validity']: validity,
8687
});
8788

‎src/components/control-component.js

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,12 @@ function getReadOnlyValue(props) {
4747
}
4848
}
4949

50+
function mergeOrSetErrors(model, errors) {
51+
return actions.setErrors(model, errors, {
52+
merge: isPlainObject(errors),
53+
});
54+
}
55+
5056
const propTypes = {
5157
model: PropTypes.oneOfType([
5258
PropTypes.func,
@@ -259,10 +265,10 @@ function createControlClass(customControlPropsMap = {}, s = defaultStrategy) {
259265
: fieldErrors;
260266

261267
if (!fieldValue || !shallowEqual(mergedErrors, fieldValue.errors)) {
262-
return actions.setErrors(model, mergedErrors);
268+
return mergeOrSetErrors(model, mergedErrors);
263269
}
264270
} else if (nodeErrors && Object.keys(nodeErrors).length) {
265-
return actions.setErrors(model, nodeErrors);
271+
return mergeOrSetErrors(model, nodeErrors);
266272
}
267273

268274
return false;
@@ -566,7 +572,7 @@ function createControlClass(customControlPropsMap = {}, s = defaultStrategy) {
566572
: fieldErrors;
567573

568574
if (!shallowEqual(errors, fieldValue.errors)) {
569-
dispatch(actions.setErrors(model, errors));
575+
dispatch(mergeOrSetErrors(model, errors));
570576
}
571577

572578
return modelValue;

‎src/reducers/form-actions-reducer.js

Lines changed: 16 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import map from '../utils/map';
88
import isPlainObject from '../utils/is-plain-object';
99
import mapValues from '../utils/map-values';
1010
import inverse from '../utils/inverse';
11+
import merge from '../utils/merge';
1112
import isValid, { fieldsValid } from '../form/is-valid';
1213
import isValidityValid from '../utils/is-validity-valid';
1314
import isValidityInvalid from '../utils/is-validity-invalid';
@@ -157,33 +158,20 @@ export default function formActionsReducer(state, action, localPath) {
157158
case actionTypes.SET_VALIDITY:
158159
case actionTypes.SET_ERRORS: {
159160
const isErrors = action.type === actionTypes.SET_ERRORS;
160-
const validity = isErrors ? action.errors : action.validity;
161-
162-
console.log();
163-
console.log(validity);
164-
165-
const validityIsPlainObject = isPlainObject(validity);
166-
167-
const inverseValidity = validityIsPlainObject
168-
? mapValues(validity, inverse)
169-
: !validity;
170-
171-
let newErrors = {};
172-
let newValidity = {};
173-
174-
161+
let validity;
175162
if (isErrors) {
176-
newErrors = validityIsPlainObject ? Object.assign({}, field.errors, validity) : validity;
177-
newValidity = validityIsPlainObject ? Object.assign({}, field.validity, inverseValidity) : inverseValidity;
178-
}
179-
else {
180-
newValidity = validityIsPlainObject ? Object.assign({}, field.validity, validity) : validity;
181-
newErrors = validityIsPlainObject ? Object.assign({}, field.errors, inverseValidity) : inverseValidity;
163+
validity = action.merge
164+
? merge({ ...fieldState.errors }, action.errors)
165+
: action.errors;
166+
} else {
167+
validity = action.merge
168+
? merge({ ...fieldState.validity }, action.validity)
169+
: action.validity;
182170
}
183171

184-
console.log(newValidity);
185-
console.log(newErrors);
186-
console.log();
172+
const inverseValidity = isPlainObject(validity)
173+
? mapValues(validity, inverse)
174+
: !validity;
187175

188176
// If the field is a form, its validity is
189177
// also based on whether its fields are all valid.
@@ -192,13 +180,13 @@ export default function formActionsReducer(state, action, localPath) {
192180
: true;
193181

194182
fieldUpdates = {
195-
'errors': newErrors,
196-
'validity': newValidity,
183+
[isErrors ? 'errors' : 'validity']: validity,
184+
[isErrors ? 'validity' : 'errors']: inverseValidity,
197185
validating: false,
198186
validated: true,
199187
valid: areFieldsValid && (isErrors
200-
? !isValidityInvalid(newErrors)
201-
: isValidityValid(newValidity)),
188+
? !isValidityInvalid(validity)
189+
: isValidityValid(validity)),
202190
};
203191

204192
parentFormUpdates = (form) => ({ valid: isValid(form) });

‎src/utils/merge.js

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,5 @@
1-
function isObject(item) {
2-
return (item && typeof item === 'object' && !Array.isArray(item) && item !== null);
3-
}
1+
import i from 'icepick';
42

53
export default function mergeDeep(target, source) {
6-
if (isObject(target) && isObject(source)) {
7-
Object.keys(source).forEach(key => {
8-
if (isObject(source[key])) {
9-
if (!target[key]) Object.assign(target, { [key]: {} });
10-
mergeDeep(target[key], source[key]);
11-
} else {
12-
Object.assign(target, { [key]: source[key] });
13-
}
14-
});
15-
}
16-
17-
return target;
4+
return i.merge(target, source);
185
}

‎test/errors-component-spec.js

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -294,17 +294,19 @@ Object.keys(testContexts).forEach((testKey) => {
294294
const form = TestUtils.renderIntoDocument(
295295
<Provider store={store}>
296296
<form>
297-
<Errors model="test.foo"
298-
messages={{
299-
length: (val, {length}) => `${val && val.length} chars is too short (must be at least ${length} chars)`,
300-
doNotShow: () => false,
301-
}}
297+
<Errors
298+
model="test.foo"
299+
messages={{
300+
length: (val, {length}) => `${val && val.length} chars is too short (must be at least ${length} chars)`,
301+
doNotShow: () => false,
302+
}}
302303
/>
303-
<Field model="test.foo"
304-
errors={{
305-
length: (v) => v && v.length && v.length > 5 ? false : {length: 5},
306-
doNotShow: () => false,
307-
}}
304+
<Field
305+
model="test.foo"
306+
errors={{
307+
length: (v) => v && v.length && v.length > 5 ? false : {length: 5},
308+
doNotShow: () => false,
309+
}}
308310
>
309311
<input type="text"/>
310312
</Field>

‎test/form-component-spec.js

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1772,7 +1772,7 @@ Object.keys(testContexts).forEach((testKey) => {
17721772
{ name: 'one' },
17731773
{ name: 'two' },
17741774
{ name: 'three' },
1775-
{ name: 'four' }
1775+
{ name: 'four' },
17761776
],
17771777
});
17781778
const store = testCreateStore({
@@ -1806,20 +1806,27 @@ Object.keys(testContexts).forEach((testKey) => {
18061806
required: notRequired(),
18071807
};
18081808
validations[field4] = {
1809-
required: hasValue(field4Value)
1810-
}
1809+
required: hasValue(field4Value),
1810+
};
18111811
return validations;
18121812
}}
18131813
errors={{
1814-
'items[2].name': (value) => { // eslint-disable-line arrow-body-style
1815-
return value.includes('three') ? false : { invalidThree: 'invalid three' };
1816-
},
1814+
'items[2].name': (value) => (
1815+
value.includes('three')
1816+
? false
1817+
: { invalidThree: 'invalid three' }
1818+
),
18171819
}}
18181820
>
1819-
<Control model={`test.items[0].name`} />
1820-
<Control model={`test.items[1].name`} />
1821-
<Control model={`test.items[2].name`} />
1822-
<Control model={`test.items[3].name`} validators={{needsFour: (val) => val.includes('four')}} />
1821+
<Control model="test.items[0].name" />
1822+
<Control model="test.items[1].name" />
1823+
<Control model="test.items[2].name" />
1824+
<Control
1825+
model="test.items[3].name"
1826+
validators={{
1827+
needsFour: (val) => val.includes('four'),
1828+
}}
1829+
/>
18231830
</Form>
18241831
</Provider>
18251832
);
@@ -1894,7 +1901,7 @@ Object.keys(testContexts).forEach((testKey) => {
18941901
it('should aggregate form validations and field validations.', () => {
18951902
input4.value = '';
18961903
TestUtils.Simulate.change(input4);
1897-
1904+
18981905
const { $form, items } = store.getState().testForm;
18991906

19001907
assert.isFalse(items[3].name.validity.required);

0 commit comments

Comments
 (0)