Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using ngrx-forms with createReducer? #147

Closed
magnattic opened this issue May 16, 2019 · 17 comments
Closed

Using ngrx-forms with createReducer? #147

magnattic opened this issue May 16, 2019 · 17 comments

Comments

@magnattic
Copy link

Is there a recommended way how to use ngrx-forms with the new reducer creators from ngrx 8?

export const reducer = createReducer(
  initialState,
  on(AuthApiActions.loginSuccess, (state, { user }) => ({ ...state, user })),
  on(AuthActions.logout, () => initialState)
);

How would I link this up with the formsReducer? Is it even possible or do we have to stick with the old fashioned way for now?

@MrWolfZ
Copy link
Owner

MrWolfZ commented May 19, 2019

Thanks for making me aware of this. It's good to see ngrx is finally also trying to remove some of its boilerplatiness.

Sadly, it is not possible out of the box right now, but as soon as ngrx 8 is released I can provide a reducer that fits the new pattern. For now, you can do that yourself, though it is a bit ugly. Something like this should work (though I haven't tested it):

export const ALL_NGRX_FORMS_ACTION_TYPES: Actions<any>['type'][] = [
  SetValueAction.TYPE,
  SetErrorsAction.TYPE,
  SetAsyncErrorAction.TYPE,
  ClearAsyncErrorAction.TYPE,
  StartAsyncValidationAction.TYPE,
  MarkAsDirtyAction.TYPE,
  MarkAsPristineAction.TYPE,
  EnableAction.TYPE,
  DisableAction.TYPE,
  MarkAsTouchedAction.TYPE,
  MarkAsUntouchedAction.TYPE,
  FocusAction.TYPE,
  UnfocusAction.TYPE,
  MarkAsSubmittedAction.TYPE,
  MarkAsUnsubmittedAction.TYPE,
  AddGroupControlAction.TYPE,
  RemoveGroupControlAction.TYPE,
  AddArrayControlAction.TYPE,
  RemoveArrayControlAction.TYPE,
  SetUserDefinedPropertyAction.TYPE,
  ResetAction.TYPE,
  SwapArrayControlAction.TYPE,
  MoveArrayControlAction.TYPE,
];

export const onNgrxForms: On<FormState<any>> = {
  reducer: formStateReducer,
  types: ALL_NGRX_FORMS_ACTION_TYPES,
}

@wbhob
Copy link
Contributor

wbhob commented May 19, 2019

@MrWolfZ I was looking at the same thing. Our company uses ngrx-forms a lot, and we're excited for ngrx v8.

If you want, the beta versions for ngrx 8 are already out, so I'd be glad to work with you on a PR for ngrx-forms that supports it.

@magnattic
Copy link
Author

magnattic commented Jun 3, 2019

That looks awesome, is it in 5.0 already?

One problem: What if you additionally need/want to react to a ngrxForm Action yourself?

We have a use case where we run the SetValueAction through the formsReducer() and then do some custom stuff in the switch-case statement, when certain conditions are met. Is this an antipattern?

I am wondering if running the formsReducer in a metareducer might be an idea to decouple things?

@magnattic
Copy link
Author

This is what works for us:

export const onNgrxForms = <T, U>(formGroupLocator: (s: T) => FormState<U>): On<T> => ({
  reducer: (state, action) => {
    const oldFormState = formGroupLocator(state);
    const newFormState = formStateReducer(oldFormState, action);
    return { ...state, formGroup: newFormState };
  },
  types: ALL_NGRX_FORMS_ACTION_TYPES
});

export const onNgrxFormsWithUpdate = <T, U>(
  formGroupLocator: (s: T) => FormState<U>,
  updateFn: (fs: FormState<U>) => FormState<U>
): On<T> => ({
  reducer: (state, action) => {
    const oldFormState = formGroupLocator(state);
    const newFormState = createFormStateReducerWithUpdate(updateFn)(oldFormState, action);
    return { ...state, formGroup: newFormState };
  },
  types: ALL_NGRX_FORMS_ACTION_TYPES
});

This can be used on States that have a property with a FormState in them.
formGroupLocator is the function that tells you how to access the FormState.

Usage is like this:

export const contactReducer = createReducer(initialState, onNgrxFormsWithUpdate(state => state.formGroup, validateContactForm));

@MrWolfZ Would be great if you could put ALL_NGRX_FORMS_ACTION_TYPES into the lib. :)

@MrWolfZ
Copy link
Owner

MrWolfZ commented Jul 7, 2019

I have just released version 5.1.0 which provides a onNgrxForms utility function (see the link for more details about the usage).

I decided to use a different implementation than @magnattic suggested above since that approach does not work if you have multiple forms in your state (due to how createReducer is implemented).

For those interested in the code, note that I have intentionally not referenced the On interface from ngrx 8 to keep the code backwards compatible to ngrx 7 (therefore there is also no bump in the peer dependency version).

@MrWolfZ MrWolfZ closed this as completed Jul 7, 2019
@dzonatan
Copy link
Contributor

dzonatan commented Jul 8, 2019

Nice work @MrWolfZ 👍

Still, I can't find a nice way to use pure ngrx-forms actions with this new syntax.
I wish to write custom on statements for actions like SetValueAction.
If your plan is to be compatible with ngrx 7 for some time (and therefore no plans to drop actions classes anytime soon) maybe it's worth thinking about one more reduce-helper something like onNgrxFormsAction?

Rough example:

export const reducer = createReducer(
  initialState,
  onNgrxForms(),
  onNgrxFormsAction(SetValueAction.TYPE, (state, action) => /* ... do something ... */),
);

export function onNgrxFormsAction<TState, TAction extends Actions<any>>(
  action: TAction['type'],
  reducer: (fs: TState, a: TAction) => TState
): { reducer: ActionReducer<TState>; types: string[] } {
  return {
    reducer,
    types: [action],
  };
}

@wbhob
Copy link
Contributor

wbhob commented Jul 10, 2019

@MrWolfZ can you also include the example in the main docs? I couldn't find the proper usage until I came here.

@wbhob
Copy link
Contributor

wbhob commented Jul 10, 2019

additionally, i'll add that AoT may not play nice with exporting the reducer as a constant – the ngrx team said this, and encourages wrapping in a reducer function. If that's the case, I'd also include the following in the docs:

export function reducer(state = initialState, action: actions.Actions): State {
  return wrapReducerWithFormStateUpdate(taskFormReducer, (s: State) => s.taskForm, taskFormValidator)(state, action)
}

@MrWolfZ
Copy link
Owner

MrWolfZ commented Jul 11, 2019

@wbhob the example is already in the main docs here. I would also expect people to look at the changelog when they update to a new version, and the changelog contains the example.

Regarding exporting the reducer, that completely depends on your particular reducer structure. That AoT issue only applies in certain situations. I don't think it is the responsibility of this library to ensure people know how to use ngrx 8 properly.

@dzonatan I just release version 5.2.0 which adds onNgrxFormsAction that allows to do exactly what you want.

@dzonatan
Copy link
Contributor

@MrWolfZ Thank you!
However, I found that current solution has one pitfall: using both onNgrxForms and onNgrxFormsAction in the same reducer won't work as expected - only one will be called depending on the order they are placed in reducer.
This is a limitation of the ngrx itself. ngrx/platform#1956

So if you're using onNgrxFormsAction you have to manually reduce it (call formGroupReducer).

export const reducer = createReducer(
  initialState,
  onNgrxForms(),
  onNgrxFormsAction(SetValueAction.TYPE, (state, action) => {
    // manually reduce SetValueAction as it was skipped on `onNgrxForms`
    state = formGroupReducer(state, action); 
    
    // do what you need..
    return state;
  }),
);

Not sure how this can be improved at the moment - just want to point this out for others to don't miss.

@MrWolfZ
Copy link
Owner

MrWolfZ commented Jul 12, 2019

@dzonatan indeed, you are absolutely right. Interesting that I missed it here even though I considered it when building onNgrxForms itself. In any case, I have just published version 5.2.1 which fixes the issue as long as you use onNgrxFormsAction after onNgrxForms.

@dzonatan
Copy link
Contributor

dzonatan commented Jul 16, 2019

@MrWolfZ thanks! One thing though, if I'm not wrong, the onNgrxForms is unnecessary when onNgrxFormsAction is used.

Also I found another limitation: all these helpers (wrapReducerWithFormStateUpdate, onNgrxForms, onNgrxFormsAction) works only on reducers that stores form state under nested property. Explicit reducer for the form state as in example below has some issues:

export interface FormModel {
  controlA: string;
  controlB: boolean;
}

export const initialState = createFormGroupState<FormModel>('EXAMPLE', { controlA: 'alpha', controlB: true });

export const rawReducer = createReducer(
  initialState,
  onNgrxForms(), // does nothing as it goes only through keys of the state, so never finds a form
);

// this does not work either, although its not a huge deal as you can write this purely[*]
export const reducer = wrapReducerWithFormStateUpdate(
  rawReducer,
  s => s,
  validateForm,
);

// [*] 
//export const reducer = (state: State, action: Action) =>
//  validateAndUpdateForm(rawReducer(state, action));

@wbhob
Copy link
Contributor

wbhob commented Jul 26, 2019

@MrWolfZ while current users will read the changelog, new users will not (and should not be expected to) read the whole changelog when they install this package. Up to you, of course, but it could prevent user frustration in the future.

@boehmers
Copy link

boehmers commented Aug 8, 2019

Hey @MrWolfZ

I started using the library for a very complex and large form and bumped into the issue mentioned in the last comment by @dzonatan .
I want to update form values based on another form value (in particular set a text value to 2 controls if a flag is set). Therefore, I used createReducer with onNgrxForms and onNgrxFormsAction, but this leads to the state not being updated at all anymore. Reducer and state are designed exactly as in the mentioned comment.

How can I solve this? Any help would be much appreciated!

Thanks in advance!

@MrWolfZ
Copy link
Owner

MrWolfZ commented Aug 8, 2019

@boehmers You mean you want to create a reducer for the form state itself with createReducer and onNgrxForms? The reasoning for not implementing this feature was that there already is a reducer for form states, i.e. formStateReducer. However, I guess there would be no harm in adding this feature. I'll put it on my TODO list (I can't guarantee when this will happen though). In the meantime this should be a workaround:

// type annotations left out for simplicity

// create your reducer how you want it without using onNgrxForms
const myReducer = createReducer()

// create your update function, e.g. for validation
const validateMyForm = updateGroup({
  // ...
})

// then create a combined reducer that first calls formStateReducer,
// then the custom reducer and finally the form state update
const combinedReducer = (state, action) =>
  validateMyForm(
    myReducer(
      formStateReducer(state, action),
      action,
    )
  )

@boehmers
Copy link

boehmers commented Aug 9, 2019

@MrWolfZ that worked!

Thanks a lot for your effort and the quick answer, appreciate it!

@boehmers
Copy link

boehmers commented Aug 9, 2019

Hello,

I re-thought my idea and wanted to share the result here, because other people might have the same problem.

ngrx-forms creates a huge, nested state, which is IMHO fine, as long as you don't start to try to write custom reducers and modify this state by hand.

See this example case:
I had a huge form with a pretty large and deeply nested object as value. The form value somewhere contains a flag, if two different data-fields on completely different branches in the nested object should be of the same value or not (which is a good example why redux in general is useful here). Example:

interface State {
  ...
  isAequalBandC: boolean;
  {
    ...
    {
      ...
      A: string;
    }
    {
      ...
      {
         ...
         B: string;
         C: string;
      }
    }
  }
}

My initial idea was to write a reducer that creates a new form state based on the values, which means fun with the spread-operater and huge nested reducers.
This eliminates a part of what makes ngrx-forms great: put the form state of reactive forms in the store and don't need to care about it yourself but with some simple actions to modify it.

As I already used effects to make api calls, why not use them to handle form-side-effects?

So the better solution was to write effects that listen to ngrx-forms-actions and fire actions that do what I desired, e.g. an effect that equals the values of the above example if the flag is set:
(code smiplyfied!)

@Injectable()
export class FormEffects {
  isAequalBandC: boolean;
  a: string;
  b: string;
  c: string;

  @Effect()
  syncAtoBandC = this.actions.pipe(
    ofType<SetValueAction<any>>(SetValueAction.TYPE),
    filter(action => action.controlId === 'form.a'),
    filter(() => !this.isAequalBandC),
    switchMap(() => {
      return [
        new SetValueAction('form.b', a), // be careful to not create infinite loops!
        new SetValueAction('form.c', a)
      ]
    })
  );

  ...

  constructor(private actions: Actions, private store: Store<State>) {
    const isAequalBandC$ = store.pipe(
      select(s => s.form.controls.isAequalBandC.value)
    );
    isAequalBandC$.subscribe(
      x => (this.isAequalBandC = x)
    );

    // select and subscribe to all desired values
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants