feat(effects): catch action creators being returned in effect without…

… being called (#2536)

modules/effects/spec/effect_sources.spec.ts

@@ -375,7 +375,7 @@ describe('EffectSources', () => {
  }
 
  class SourceError {
- e$ = createEffect(() => throwError(error));
+ e$ = createEffect(() => throwError(error) as any);
  }
 
  class SourceH {

modules/effects/spec/effects_error_handler.spec.ts

@@ -93,7 +93,7 @@ describe('Effects Error Handler', () => {
  }
 
  class AlwaysErrorEffect {
- effect$ = createEffect(() => throwError('always an error'));
+ effect$ = createEffect(() => throwError('always an error') as any);
  }
 
  /**

modules/effects/spec/types/effect_creator.spec.ts

@@ -6,6 +6,7 @@ describe('createEffect()', () => {
  code => `
  import { Action } from '@ngrx/store';
  import { createEffect } from '@ngrx/effects';
+ import { createAction } from '@ngrx/store';
  import { of } from 'rxjs';
 
  ${code}`,
@@ -21,7 +22,23 @@ describe('createEffect()', () => {
  expectSnippet(`
  const effect = createEffect(() => of({ foo: 'a' }));
  `).toFail(
- /Type 'Observable<{ foo: string; }>' is not assignable to type 'Observable<Action> | ((...args: any[]) => Observable<Action>)'./
+ /Type 'Observable<{ foo: string; }>' is not assignable to type 'EffectResult<Action>'./
+ );
+ });
+
+ it('should help with action creator that is not called', () => {
+ // Action creator is called with parentheses.
+ expectSnippet(`
+ const action = createAction('action without props');
+ const effect = createEffect(() => of(action()));
+ `).toSucceed();
+
+ // Action creator is not called (no parentheses).
+ expectSnippet(`
+ const action = createAction('action without props');
+ const effect = createEffect(() => of(action));
+ `).toFail(
+ /ActionCreator cannot be dispatched. Did you forget to call the action creator function/
  );
  });
 
@@ -33,7 +50,7 @@ describe('createEffect()', () => {
  expectSnippet(`
  const effect = createEffect(() => of({ foo: 'a' }), { dispatch: true });
  `).toFail(
- /Type 'Observable<{ foo: string; }>' is not assignable to type 'Observable<Action> | ((...args: any[]) => Observable<Action>)'./
+ /Type 'Observable<{ foo: string; }>' is not assignable to type 'EffectResult<Action>'./
  );
  });
  });
@@ -47,8 +64,16 @@ describe('createEffect()', () => {
  expectSnippet(`
  const effect = createEffect(() => ({ foo: 'a' }), { dispatch: false });
  `).toFail(
- /Type '{ foo: string; }' is not assignable to type 'Observable<unknown> | ((...args: any[]) => Observable<unknown>)'./
+ /Type '{ foo: string; }' is not assignable to type 'EffectResult<unknown>'./
  );
  });
+
+ it('should allow action creator even if it is not called', () => {
+ // Action creator is not called (no parentheses), but we have no-dispatch.
+ expectSnippet(`
+ const action = createAction('action without props');
+ const effect = createEffect(() => of(action), { dispatch: false });
+ `).toSucceed();
+ });
  });
 });

modules/effects/src/effect_creator.ts

@@ -1,5 +1,5 @@
 import { Observable } from 'rxjs';
-import { Action } from '@ngrx/store';
+import { Action, ActionCreator } from '@ngrx/store';
 import {
  EffectMetadata,
  EffectConfig,
@@ -10,6 +10,15 @@ import {
 
 type DispatchType<T> = T extends { dispatch: infer U } ? U : true;
 type ObservableType<T, OriginalType> = T extends false ? OriginalType : Action;
+type EffectResult<OT> = Observable<OT> | ((...args: any[]) => Observable<OT>);
+type ConditionallyDisallowActionCreator<DT, Result> = DT extends false
+ ? unknown // If DT (DispatchType is false, then we don't enforce any return types)
+ : Result extends EffectResult<infer OT>
+ ? OT extends ActionCreator
+ ? 'ActionCreator cannot be dispatched. Did you forget to call the action creator function?'
+ : unknown
+ : unknown;
+
 /**
  * @description
  * Creates an effect from an `Observable` and an `EffectConfig`.
@@ -46,8 +55,11 @@ export function createEffect<
  C extends EffectConfig,
  DT extends DispatchType<C>,
  OT extends ObservableType<DT, OT>,
- R extends Observable<OT> | ((...args: any[]) => Observable<OT>)
->(source: () => R, config?: Partial<C>): R & CreateEffectMetadata {
+ R extends EffectResult<OT>
+>(
+ source: () => R & ConditionallyDisallowActionCreator<DT, R>,
+ config?: Partial<C>
+): R & CreateEffectMetadata {
  const effect = source();
  const value: EffectConfig = {
  ...DEFAULT_EFFECT_CONFIG,

modules/store/spec/types/store.spec.ts

@@ -16,7 +16,7 @@ describe('Store', () => {
 
  it('should not allow passing action creator function without calling it', () => {
  expectSnippet(`store.dispatch(fooAction);`).toFail(
- /is not assignable to type '"Functions are not allowed to be dispatched. Did you forget to call action creator function/
+ /is not assignable to type '"Functions are not allowed to be dispatched. Did you forget to call the action creator function/
  );
  });
 });

modules/store/src/store.ts

@@ -98,7 +98,7 @@ export class Store<T = object> extends Observable<T>
  action: V &
  FunctionIsNotAllowed<
  V,
- 'Functions are not allowed to be dispatched. Did you forget to call action creator function?'
+ 'Functions are not allowed to be dispatched. Did you forget to call the action creator function?'
  >
  ) {
  this.actionsObserver.next(action);