ngrx / platform

Reactive State for Angular
https://ngrx.io
Other
8.03k stars 1.97k forks source link

Cannot generically handle actions with new creator functions #1990

Closed a88zach closed 5 years ago

a88zach commented 5 years ago

With previous version of NGRX we could easily extend a base class when creating new Actions and then ensure those actions are handled in a generic way. A good example for this scenario is handling FAIL actions.

// Base
export abstract class FailAction implements Action {
  readonly type: string;

  constructor(public readonly payload: FailActionPayload) {}
}

export class UncaughtErrorFail extends FailAction {
  readonly type = ActionTypes.UNCAUGHT_ERROR;
}

export class NavigationFail extends FailAction {
  readonly type = ActionTypes.NAVIGATION_FAIL;
}

Then we could create an effect like:

@Effect()
  readonly fail$: Observable<Action> = this.actions$.pipe(
    filter(action => action instanceof FailAction),
    tap((action: FailAction) => this.logError(action.payload)),
   ......
  );

With the new creator methods, this becomes much more complicated. We need the ability to handle similar actions in a generic way without having to provide all the FAIL actions types in the effects ofType operator

Describe any alternatives/workarounds you're currently using

The only current workaround I've found is to ensure that all the FAIL actions are created in the exact same way and then to check if the action has a certain property. This is fragile at best

  readonly fail$: Observable<Action> = createEffect(() =>
    this.actions$.pipe(
      filter(action => {
        // tslint:disable-next-line: no-any
        const failAction: FailAction = (action as any).failAction;

        return failAction && failAction instanceof FailAction;
      }),
      concatMap(action =>
        // tslint:disable-next-line: no-any
        of((action as any).failAction.payload as FailActionPayload)
      ),
      tap(payload => {
        this.logError(payload);
      }),
      filter(payload => !payload.suppress),
      ...
    )
  );

Other information:

If accepted, I would be willing to submit a PR for this feature

[ ] Yes (Assistance is provided if you need help submitting a pull request) [X] No

timdeschryver commented 5 years ago

The closest solution to the current one that you're using would be to create a fail action helper.

export const failAction(payload) {
  return { __isFailure: true, payload }
}

export const loadBookFailure = createAction(
  '[Book Exists Guard] Load Book',
  (payload) => failAction(payload)
);

Another possibility is to create higher-order function that wraps your effects. This higher-order effect will process the errors.

a88zach commented 5 years ago

Thanks @timdeschryver for the response, but this doesn't solve the issue. Think about a FAIL action where the failure is handled in an effect in a generic way, but the FAIL action also needs to do something specific in a reducer. For instance an http failure when paging in a result set would update the results state in the reducer to an empty array and the effect would be responsible for showing an error message to the user.

timdeschryver commented 5 years ago

The described scenario can be handled: