MrWolfZ / ngrx-forms

Enhance your forms in Angular applications with the power of ngrx
MIT License
374 stars 110 forks source link

Adding simple Array Values to Forms ngrx 10 #214

Closed astukanov closed 4 years ago

astukanov commented 4 years ago

Hi, after after thorough research and many conclusions I reached an issue, which I am not able to solve. For the sake of consistency it would be best to use the same code style proposed by ngrx 10 actions in all of my reducers.
Here is a working example of my ngrx reducer:

const actionPrefix = '[CUSTOM_TYPE] ';

export const addCustomType =
  createAction(actionPrefix + 'ADD', props<CustomType>());
export const removeLastCustomType =
  createAction(actionPrefix + 'REMOVE_LAST');
export const clearCustomTypes =
  createAction(actionPrefix + 'CLEAR');
export const featureKey = 'featureKey';

interface State {
  types: CustomType[];
}

const initialState: State = {
  types: new Array<CustomType>()
};

export const stateReducer = createReducer(
  initialState,
  on(addCustomType, (state, type) =>
    ({ ...state, types: [...state.types, type] })
  ),
  on(removeLastCustomType, state =>
    ({...state, types: state.types.filter( (item, i) => i !== state.types.length - 1 )})
  ),
  on(clearCustomTypes, state =>
    ({...state, types: []})
  )

Unfortunately I am already stuck with the first implementation of the add functionality to an array element with ngrx-forms.

Here is the code i wrote so far, which should work in my opinion after reading How to dynamically add formgroup controls to formarray in angular while the state is managed by ngrx-forms? and Dynamic Form Arrays Questions

export const addCustomType1 = createAction('[CUSTOM_FORM] ADD_CUSTOM_TYPE_1',
  props<CustomType1>()
);
export const removeCustomType1 = createAction('[CUSTOM_FORM] REMOVE_CUSTOM_TYPE_1',
  props<CustomType1>()
);
export const customTypesFormFeatureKey = 'customTypesForm';

export interface CustomTypeInterface1 {
  customTypes1: CustomType1[]
}

export interface CustomTypeInterface2 {
  customTypes2: CustomType2[]
}

export interface CustomTypesForm extends
  CustomTypeInterface1,
  CustomTypeInterface2

export const validateCustomTypeForm = updateGroup<CustomTypesForm>({
  customTypes1: validate(required, minLength(1)),
  customTypes2: validate(required, minLength(1))
});

export const initialCustomTypesFormState = createFormGroupState<CustomTypesForm>(customTypesFormFeatureKey, {
  customTypes1: new Array<CustomType1>(),
  customTypes2: new Array<CustomType2>()
});

export const customTypesFormReducer = createReducer(
  initialCustomTypesFormState,
  onNgrxForms(),
  on(addCustomType1, (state, type1) =>       //
    updateGroup<CustomTypesForm>(state,  //
        {                                                              //
          customTypes1: addArrayControl(type1)          // basically here the compiler errors out, as can be seen below
        }                                                              //
      )                                                                //
);                                                                     //

export const customTypesFormReducerWithUpdate = wrapReducerWithFormStateUpdate(
  customTypesFormReducer,
  (state) => state,
  validateCustomTypeForm
);
Overload 1 of 11, '(creator1: ActionCreator<"[CUSTOM_FORM] ADD_CUSTOM_TYPE_1", (props: { type1: CustomType1; }) => { type1: CustomType1; } & TypedAction<"[CUSTOM_FORM] ADD_CUSTOM_TYPE_1">>, reducer: OnReducer<...>): On<...>', gave the following error.
        Type '(state: FormGroupState<{ readonly value: unknown; readonly isValid: unknown; readonly isInvalid: unknown; readonly errors: unknown; readonly pendingValidations: unknown; readonly isValidationPending: unknown; readonly isEnabled: unknown; readonly isDisabled: unknown; ... 8 more ...; readonly userDefinedProperties: u...' is missing the following properties from type 'FormGroupState<CustomTypesForm>': value, isValid, isInvalid, errors, and 13 more.
      Overload 2 of 11, '(creator: ActionCreator<string, FunctionWithParametersType<any[], object>>, ...rest: (ActionCreator<string, FunctionWithParametersType<any[], object>> | OnReducer<...>)[]): On<...>', gave the following error.
        Argument of type '(state: FormGroupState<CustomTypesForm>,  type1: { type1: CustomType1; } & TypedAction<"[CUSTOM_FORM] ADD_CUSTOM_TYPE_1"> & { type: "[CUSTOM_FORM] ADD_CUSTOM_TYPE_1"; }) => (state: FormGroupState<...>) => FormGroupState<...>' is not assignable to parameter of type 'ActionCreator<string, FunctionWithParametersType<any[], object>> | OnReducer<FormGroupState<CustomTypesForm>, [...]>'.
          Type '(state: FormGroupState<CustomTypesForm>,  type1: { type1: CustomType1; } & TypedAction<"[CUSTOM_FORM] ADD_CUSTOM_TYPE_1"> & { type: "[CUSTOM_FORM] ADD_CUSTOM_TYPE_1"; }) => (state: FormGroupState<...>) => FormGroupState<...>' is not assignable to type 'ActionCreator<string, FunctionWithParametersType<any[], object>>'.
            Property 'type' is missing in type '(state: FormGroupState<CustomTypesForm>, type1: { type1: CustomType1; } & TypedAction<"[CUSTOM_FORM] ADD_CUSTOM_TYPE_1"> & { type: "[CUSTOM_FORM] ADD_CUSTOM_TYPE_1"; }) => (state: FormGroupState<...>) => FormGroupState<...>' but required in type 'TypedAction<string>'.

I've tried a lot of variations to solve the issue, unfortunately non of them worked.

NgRx versions:

"@ngrx/entity": "^10.0.0",
"@ngrx/store": "^10.0.0",

ngrx-forms version:

 "ngrx-forms": "^6.3.3"
astukanov commented 4 years ago

Eventually figured it out:

export const customTypesFormReducer = createReducer(
  initialCustomTypesFormState,
  onNgrxForms(),
  on(addCustomType1, (state, type1) =>       
    updateGroup<CustomTypesForm>(state,  
        {                                                              
          customTypes1: addArrayControl<CustomType1>(type1)         
        }                                                              
      )                                                                
); 

Providing the type is enough to make it work as desired. Although automatic type inference would be a bliss if possible.

MrWolfZ commented 4 years ago

Wow, yeah, that's a rather unfortunate error message that is hard to parse. In theory the lib should be able to infer this due to the type of type1 being inferred by ngrx and also you providing CustomTypesForm to updateGroup. The only thing you could try is to use the long form, i.e. the following. Let me know if this infers the type, and if not, what the IDE shows as the type for control (it should be FormArrayState<CustomType1>).

export const customTypesFormReducer = createReducer(
  initialCustomTypesFormState,
  onNgrxForms(),
  on(addCustomType1, (state, type1) =>       
    updateGroup<CustomTypesForm>(state,  
        {                                                              
          customTypes1: control => addArrayControl(control, type1)         
        }                                                              
      )                                                                
); 

Also, in the short form without the specified type for addArrayControl, what does the IDE infer the generic type of addArrayControl as?

MrWolfZ commented 4 years ago

Alright, I checked this myself. I see that the long form works as expected. The reason the short form does not work is an unfortunate design by ngrx that makes the second parameter an intersection between the props and the action type, which confuses the type inference. There is actually even a runtime impact since the object set in the form will have the type property of the action itself. My suggestion is to use a dedicated property in the props:

export const addCustomType1 = createAction('[CUSTOM_FORM] ADD_CUSTOM_TYPE_1',
  props<{ type1: CustomType1 }>()
);

export const customTypesFormReducer = createReducer(
  initialCustomTypesFormState,
  onNgrxForms(),
  on(addCustomType1, (state, { type1 }) =>
    updateGroup<CustomTypesForm>(state,
      {
        customTypes1: addArrayControl(type1),
      }
    )
  ),
);

With this code the correct value is set, type inference works as expected, and it is only a little bit more code.

astukanov commented 4 years ago

@MrWolfZ Thanks a lot for taking a look at this and the provided approach. I will then switch to the solution with dedicated properties for my objects as it is definetly not that much of an overhead.