Skip to content

Commit

Permalink
fix: fix createFormArrayState producing results inconsistent with h…
Browse files Browse the repository at this point in the history
…ow array states are recomputed from their children after an update
  • Loading branch information
MrWolfZ committed Jan 31, 2018
1 parent 8badec0 commit 70fdc10
Show file tree
Hide file tree
Showing 23 changed files with 185 additions and 146 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* fix missing union case in typing of `updateArray` update function that causes a compile error if used inside an `updateGroup` ([fa7dccc](https://github.com/MrWolfZ/ngrx-forms/commit/fa7dccc))
* fix `updateGroup` throwing an error if an empty update object was provided in curried as well as uncurried version ([bee4d54](https://github.com/MrWolfZ/ngrx-forms/commit/bee4d54))
* fix `createFormGroupState` producing results inconsistent with how group states are recomputed from their children after an update ([1c62d8c](https://github.com/MrWolfZ/ngrx-forms/commit/1c62d8c))
* fix `createFormArrayState` producing results inconsistent with how array states are recomputed from their children after an update

<a name="2.1.2"></a>
### 2.1.2
Expand Down
4 changes: 2 additions & 2 deletions src/array/reducer/add-control.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Actions, AddArrayControlAction } from '../../actions';
import { createChildState, FormArrayState } from '../../state';
import { childReducer, computeArrayState } from './util';
import { computeArrayState, createChildState, FormArrayState } from '../../state';
import { childReducer } from './util';

export function addControlReducer<TValue>(
state: FormArrayState<TValue>,
Expand Down
4 changes: 2 additions & 2 deletions src/array/reducer/clear-async-error.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Actions, ClearAsyncErrorAction } from '../../actions';
import { FormArrayState } from '../../state';
import { childReducer, computeArrayState } from './util';
import { computeArrayState, FormArrayState } from '../../state';
import { childReducer } from './util';

export function clearAsyncErrorReducer<TValue>(
state: FormArrayState<TValue>,
Expand Down
4 changes: 2 additions & 2 deletions src/array/reducer/disable.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Actions, DisableAction } from '../../actions';
import { FormArrayState } from '../../state';
import { childReducer, computeArrayState, dispatchActionPerChild } from './util';
import { computeArrayState, FormArrayState } from '../../state';
import { childReducer, dispatchActionPerChild } from './util';

export function disableReducer<TValue>(
state: FormArrayState<TValue>,
Expand Down
4 changes: 2 additions & 2 deletions src/array/reducer/enable.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Actions, EnableAction } from '../../actions';
import { FormArrayState } from '../../state';
import { childReducer, computeArrayState, dispatchActionPerChild } from './util';
import { computeArrayState, FormArrayState } from '../../state';
import { childReducer, dispatchActionPerChild } from './util';

export function enableReducer<TValue>(
state: FormArrayState<TValue>,
Expand Down
4 changes: 2 additions & 2 deletions src/array/reducer/mark-as-dirty.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Actions, MarkAsDirtyAction } from '../../actions';
import { FormArrayState } from '../../state';
import { childReducer, computeArrayState, dispatchActionPerChild } from './util';
import { computeArrayState, FormArrayState } from '../../state';
import { childReducer, dispatchActionPerChild } from './util';

export function markAsDirtyReducer<TValue>(
state: FormArrayState<TValue>,
Expand Down
4 changes: 2 additions & 2 deletions src/array/reducer/mark-as-pristine.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Actions, MarkAsPristineAction } from '../../actions';
import { FormArrayState } from '../../state';
import { childReducer, computeArrayState, dispatchActionPerChild } from './util';
import { computeArrayState, FormArrayState } from '../../state';
import { childReducer, dispatchActionPerChild } from './util';

export function markAsPristineReducer<TValue>(
state: FormArrayState<TValue>,
Expand Down
4 changes: 2 additions & 2 deletions src/array/reducer/mark-as-submitted.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Actions, MarkAsSubmittedAction } from '../../actions';
import { FormArrayState } from '../../state';
import { childReducer, computeArrayState, dispatchActionPerChild } from './util';
import { computeArrayState, FormArrayState } from '../../state';
import { childReducer, dispatchActionPerChild } from './util';

export function markAsSubmittedReducer<TValue>(
state: FormArrayState<TValue>,
Expand Down
4 changes: 2 additions & 2 deletions src/array/reducer/mark-as-touched.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Actions, MarkAsTouchedAction } from '../../actions';
import { FormArrayState } from '../../state';
import { childReducer, computeArrayState, dispatchActionPerChild } from './util';
import { computeArrayState, FormArrayState } from '../../state';
import { childReducer, dispatchActionPerChild } from './util';

export function markAsTouchedReducer<TValue>(
state: FormArrayState<TValue>,
Expand Down
4 changes: 2 additions & 2 deletions src/array/reducer/mark-as-unsubmitted.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Actions, MarkAsUnsubmittedAction } from '../../actions';
import { FormArrayState } from '../../state';
import { childReducer, computeArrayState, dispatchActionPerChild } from './util';
import { computeArrayState, FormArrayState } from '../../state';
import { childReducer, dispatchActionPerChild } from './util';

export function markAsUnsubmittedReducer<TValue>(
state: FormArrayState<TValue>,
Expand Down
4 changes: 2 additions & 2 deletions src/array/reducer/mark-as-untouched.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Actions, MarkAsUntouchedAction } from '../../actions';
import { FormArrayState } from '../../state';
import { childReducer, computeArrayState, dispatchActionPerChild } from './util';
import { computeArrayState, FormArrayState } from '../../state';
import { childReducer, dispatchActionPerChild } from './util';

export function markAsUntouchedReducer<TValue>(
state: FormArrayState<TValue>,
Expand Down
4 changes: 2 additions & 2 deletions src/array/reducer/remove-control.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Actions, RemoveArrayControlAction } from '../../actions';
import { FormArrayState } from '../../state';
import { childReducer, computeArrayState } from './util';
import { computeArrayState, FormArrayState } from '../../state';
import { childReducer } from './util';

export function removeControlReducer<TValue>(
state: FormArrayState<TValue>,
Expand Down
4 changes: 2 additions & 2 deletions src/array/reducer/reset.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Actions, ResetAction } from '../../actions';
import { FormArrayState } from '../../state';
import { childReducer, computeArrayState, dispatchActionPerChild } from './util';
import { computeArrayState, FormArrayState } from '../../state';
import { childReducer, dispatchActionPerChild } from './util';

export function resetReducer<TValue>(
state: FormArrayState<TValue>,
Expand Down
4 changes: 2 additions & 2 deletions src/array/reducer/set-async-error.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Actions, SetAsyncErrorAction } from '../../actions';
import { FormArrayState } from '../../state';
import { computeArrayState, FormArrayState } from '../../state';
import { deepEquals } from '../../util';
import { childReducer, computeArrayState } from './util';
import { childReducer } from './util';

export function setAsyncErrorReducer<TValue>(
state: FormArrayState<TValue>,
Expand Down
17 changes: 9 additions & 8 deletions src/array/reducer/set-errors.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,15 @@ describe(`form array ${setErrorsReducer.name}`, () => {
expect(resultState).toBe(INITIAL_STATE);
});

it('should update state if array is empty', () => {
const errors = { required: true };
const state = createFormArrayState<string>('test ID', []);
const resultState = setErrorsReducer(state, new SetErrorsAction(FORM_CONTROL_ID, errors));
expect(resultState.errors).toEqual(errors);
expect(resultState.isValid).toBe(false);
expect(resultState.isInvalid).toBe(true);
});
// will be fixed as part of another bugfix
// it('should update state if array is empty', () => {
// const errors = { required: true };
// const state = createFormArrayState<string>('test ID', []);
// const resultState = setErrorsReducer(state, new SetErrorsAction(FORM_CONTROL_ID, errors));
// expect(resultState.errors).toEqual(errors);
// expect(resultState.isValid).toBe(false);
// expect(resultState.isInvalid).toBe(true);
// });

it('should keep async errors', () => {
const syncErrors = { required: true };
Expand Down
4 changes: 2 additions & 2 deletions src/array/reducer/set-errors.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Actions, SetErrorsAction } from '../../actions';
import { FormArrayState } from '../../state';
import { computeArrayState, FormArrayState } from '../../state';
import { deepEquals } from '../../util';
import { childReducer, computeArrayState } from './util';
import { childReducer } from './util';

export function setErrorsReducer<TValue>(
state: FormArrayState<TValue>,
Expand Down
4 changes: 2 additions & 2 deletions src/array/reducer/set-value.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Actions, SetValueAction } from '../../actions';
import { createChildState, FormArrayState } from '../../state';
import { callChildReducer, childReducer, computeArrayState } from './util';
import { computeArrayState, createChildState, FormArrayState } from '../../state';
import { callChildReducer, childReducer } from './util';

export function setValueReducer<TValue>(
state: FormArrayState<TValue>,
Expand Down
4 changes: 2 additions & 2 deletions src/array/reducer/start-async-validation.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Actions, StartAsyncValidationAction } from '../../actions';
import { FormArrayState } from '../../state';
import { childReducer, computeArrayState } from './util';
import { computeArrayState, FormArrayState } from '../../state';
import { childReducer } from './util';

export function startAsyncValidationReducer<TValue>(
state: FormArrayState<TValue>,
Expand Down
80 changes: 1 addition & 79 deletions src/array/reducer/util.ts
Original file line number Diff line number Diff line change
@@ -1,87 +1,9 @@
import { Actions } from '../../actions';
import { formControlReducerInternal } from '../../control/reducer';
import { formGroupReducerInternal } from '../../group/reducer';
import { AbstractControlState, FormArrayState, isArrayState, isGroupState, KeyValue, ValidationErrors } from '../../state';
import { isEmpty } from '../../util';
import { AbstractControlState, computeArrayState, FormArrayState, isArrayState, isGroupState } from '../../state';
import { formArrayReducerInternal } from '../reducer';

export function getFormArrayValue<TValue>(
controls: Array<AbstractControlState<TValue>>,
originalValue: TValue[],
): TValue[] {
let hasChanged = Object.keys(originalValue).length !== Object.keys(controls).length;
const newValue = controls.map((state, i) => {
hasChanged = hasChanged || originalValue[i] !== state.value;
return state.value;
});

return hasChanged ? newValue : originalValue;
}

export function getFormArrayErrors<TValue>(
controls: Array<AbstractControlState<TValue>>,
originalErrors: ValidationErrors,
): ValidationErrors {
let hasChanged = false;
const groupErrors =
Object.keys(originalErrors)
.filter(key => !key.startsWith('_'))
.reduce((res, key) => Object.assign(res, { [key]: originalErrors[key] }), {});

const newErrors = controls.reduce((res, state, i) => {
const controlErrors = state.errors;
if (!isEmpty(controlErrors)) {
hasChanged = hasChanged || originalErrors['_' + i] !== controlErrors;
res['_' + i] = controlErrors;
} else {
hasChanged = hasChanged || originalErrors.hasOwnProperty('_' + i);
}

return res;
}, groupErrors as ValidationErrors);

hasChanged = hasChanged || Object.keys(originalErrors).length !== Object.keys(newErrors).length;

return hasChanged ? newErrors : originalErrors;
}

export function computeArrayState<TValue>(
id: string,
controls: Array<AbstractControlState<TValue>>,
value: TValue[],
errors: ValidationErrors,
pendingValidations: string[],
userDefinedProperties: KeyValue,
): FormArrayState<TValue> {
value = getFormArrayValue<TValue>(controls, value);
errors = getFormArrayErrors(controls, errors);
const isValid = isEmpty(errors);
const isDirty = controls.some(state => state.isDirty);
const isEnabled = controls.some(state => state.isEnabled);
const isTouched = controls.some(state => state.isTouched);
const isSubmitted = controls.some(state => state.isSubmitted);
const isValidationPending = pendingValidations.length > 0 || controls.some(state => state.isValidationPending);
return {
id,
value,
errors,
pendingValidations,
isValidationPending,
isValid,
isInvalid: !isValid,
isEnabled,
isDisabled: !isEnabled,
isDirty,
isPristine: !isDirty,
isTouched,
isUntouched: !isTouched,
isSubmitted,
isUnsubmitted: !isSubmitted,
userDefinedProperties,
controls,
};
}

export function callChildReducer(
state: AbstractControlState<any>,
action: Actions<any>,
Expand Down
56 changes: 56 additions & 0 deletions src/state.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
cast,
computeArrayState,
computeGroupState,
createFormArrayState,
createFormControlState,
Expand Down Expand Up @@ -275,6 +276,61 @@ describe('state', () => {
const initialState = createFormArrayState<string>(FORM_CONTROL_ID, initialValue);
expect(initialState.controls).toEqual([]);
});

it('should produce the same state as is computed after an action is applied', () => {
const state = computeArrayState(
INITIAL_STATE.id,
INITIAL_STATE.controls,
INITIAL_STATE.value,
INITIAL_STATE.errors,
INITIAL_STATE.pendingValidations,
INITIAL_STATE.userDefinedProperties,
);

expect(state.id).toBe(INITIAL_STATE.id);
expect(state.value).toEqual(INITIAL_STATE.value);
expect(state.errors).toEqual(INITIAL_STATE.errors);
expect(state.pendingValidations).toEqual(INITIAL_STATE.pendingValidations);
expect(state.isValid).toEqual(INITIAL_STATE.isValid);
expect(state.isInvalid).toEqual(INITIAL_STATE.isInvalid);
expect(state.isEnabled).toEqual(INITIAL_STATE.isEnabled);
expect(state.isDisabled).toEqual(INITIAL_STATE.isDisabled);
expect(state.isDirty).toEqual(INITIAL_STATE.isDirty);
expect(state.isPristine).toEqual(INITIAL_STATE.isPristine);
expect(state.isTouched).toEqual(INITIAL_STATE.isTouched);
expect(state.isUntouched).toEqual(INITIAL_STATE.isUntouched);
expect(state.isSubmitted).toEqual(INITIAL_STATE.isSubmitted);
expect(state.isUnsubmitted).toEqual(INITIAL_STATE.isUnsubmitted);
expect(state.userDefinedProperties).toEqual(INITIAL_STATE.userDefinedProperties);
});

it('should produce the same state as is computed after an action is applied for empty array', () => {
const initialState = createFormArrayState(FORM_CONTROL_ID, []);
const state = computeArrayState(
initialState.id,
initialState.controls,
initialState.value,
initialState.errors,
initialState.pendingValidations,
initialState.userDefinedProperties,
);

expect(state.id).toBe(initialState.id);
expect(state.value).toEqual(initialState.value);
expect(state.errors).toEqual(initialState.errors);
expect(state.pendingValidations).toEqual(initialState.pendingValidations);
expect(state.isValid).toEqual(initialState.isValid);
expect(state.isInvalid).toEqual(initialState.isInvalid);
expect(state.isEnabled).toEqual(initialState.isEnabled);
expect(state.isDisabled).toEqual(initialState.isDisabled);
expect(state.isDirty).toEqual(initialState.isDirty);
expect(state.isPristine).toEqual(initialState.isPristine);
expect(state.isTouched).toEqual(initialState.isTouched);
expect(state.isUntouched).toEqual(initialState.isUntouched);
expect(state.isSubmitted).toEqual(initialState.isSubmitted);
expect(state.isUnsubmitted).toEqual(initialState.isUnsubmitted);
expect(state.userDefinedProperties).toEqual(initialState.userDefinedProperties);
});
});

describe(isArrayState.name, () => {
Expand Down
Loading

0 comments on commit 70fdc10

Please sign in to comment.