From 8549f8403f57439bc608d97792d38051a2ed4b54 Mon Sep 17 00:00:00 2001 From: aokrushko Date: Sun, 31 Mar 2019 18:18:31 -0400 Subject: [PATCH 1/2] feat(effects): allow ofType to handle ActionCreator --- modules/effects/spec/actions.spec.ts | 214 ++++++++++++++++++++++++--- modules/effects/src/actions.ts | 98 ++++++++---- 2 files changed, 264 insertions(+), 48 deletions(-) diff --git a/modules/effects/spec/actions.spec.ts b/modules/effects/spec/actions.spec.ts index e66f65331e..4773a7a117 100644 --- a/modules/effects/spec/actions.spec.ts +++ b/modules/effects/spec/actions.spec.ts @@ -1,9 +1,10 @@ import { Injector } from '@angular/core'; import { Action, - StoreModule, + props, ScannedActionsSubject, ActionsSubject, + createAction, } from '@ngrx/store'; import { Actions, ofType } from '../'; import { map, toArray, switchMap } from 'rxjs/operators'; @@ -25,16 +26,12 @@ describe('Actions', function() { type: 'SUBTRACT'; } - function reducer(state: number = 0, action: Action) { - switch (action.type) { - case ADD: - return state + 1; - case SUBTRACT: - return state - 1; - default: - return state; - } - } + const square = createAction('SQUARE'); + const multiply = createAction('MULTYPLY', props<{ by: number }>()); + const divide = createAction('DIVIDE', props<{ by: number }>()); + + // Class-based Action types + const actions = [ADD, ADD, SUBTRACT, ADD, SUBTRACT]; beforeEach(function() { const injector = Injector.create([ @@ -69,12 +66,12 @@ describe('Actions', function() { }); actions.forEach(action => dispatcher.next(action)); + dispatcher.complete(); }); - const actions = [ADD, ADD, SUBTRACT, ADD, SUBTRACT]; - const expected = actions.filter(type => type === ADD); + it('should filter out actions', () => { + const expected = actions.filter(type => type === ADD); - it('should let you filter out actions', function() { actions$ .pipe( ofType(ADD), @@ -83,7 +80,7 @@ describe('Actions', function() { ) .subscribe({ next(actual) { - expect(actual).toEqual(expected as any[]); + expect(actual).toEqual(expected); }, }); @@ -91,7 +88,9 @@ describe('Actions', function() { dispatcher.complete(); }); - it('should let you filter out actions and ofType can take an explicit type argument', function() { + it('should filter out actions and ofType can take an explicit type argument', () => { + const expected = actions.filter(type => type === ADD); + actions$ .pipe( ofType(ADD), @@ -100,11 +99,192 @@ describe('Actions', function() { ) .subscribe({ next(actual) { - expect(actual).toEqual(expected as any[]); + expect(actual).toEqual(expected); + }, + }); + + actions.forEach(action => dispatcher.next({ type: action })); + dispatcher.complete(); + }); + + it('should let you filter out multiple action types with explicit type argument', () => { + const expected = actions.filter(type => type === ADD || type === SUBTRACT); + + actions$ + .pipe( + ofType(ADD, SUBTRACT), + map(update => update.type), + toArray() + ) + .subscribe({ + next(actual) { + expect(actual).toEqual(expected); + }, + }); + + actions.forEach(action => dispatcher.next({ type: action })); + dispatcher.complete(); + }); + + it('should filter out actions by action creator', () => { + actions$ + .pipe( + ofType(square), + map(update => update.type), + toArray() + ) + .subscribe({ + next(actual) { + expect(actual).toEqual(['SQUARE']); + }, + }); + + [...actions, square.type].forEach(action => + dispatcher.next({ type: action }) + ); + dispatcher.complete(); + }); + + it('should infer the type for the action when it is filter by action creator with property', () => { + const MULTYPLY_BY = 5; + + actions$ + .pipe( + ofType(multiply), + map(update => update.by), + toArray() + ) + .subscribe({ + next(actual) { + expect(actual).toEqual([MULTYPLY_BY]); }, }); + // Unrelated Actions actions.forEach(action => dispatcher.next({ type: action })); + // Action under test + dispatcher.next(multiply({ by: MULTYPLY_BY })); + dispatcher.complete(); + }); + + it('should infer the type for the action when it is filter by action creator', () => { + // Types are not provided for generic Actions + const untypedActions$: Actions = actions$; + const MULTYPLY_BY = 5; + + untypedActions$ + .pipe( + ofType(multiply), + // Type is infered, even though untypedActions$ is Actions + map(update => update.by), + toArray() + ) + .subscribe({ + next(actual) { + expect(actual).toEqual([MULTYPLY_BY]); + }, + }); + + // Unrelated Actions + actions.forEach(action => dispatcher.next({ type: action })); + // Action under test + dispatcher.next(multiply({ by: MULTYPLY_BY })); + dispatcher.complete(); + }); + + it('should filter out multiple actions by action creator', () => { + const DIVIDE_BY = 3; + const MULTYPLY_BY = 5; + const expected = [DIVIDE_BY, MULTYPLY_BY]; + + actions$ + .pipe( + ofType(divide, multiply), + // Both have 'by' property + map(update => update.by), + toArray() + ) + .subscribe({ + next(actual) { + expect(actual).toEqual(expected); + }, + }); + + // Unrelated Actions + actions.forEach(action => dispatcher.next({ type: action })); + // Actions under test, in specific order + dispatcher.next(divide({ by: DIVIDE_BY })); + dispatcher.next(divide({ by: MULTYPLY_BY })); + dispatcher.complete(); + }); + + it('should filter out actions by action creator and type string', () => { + const expected = [...actions.filter(type => type === ADD), square.type]; + + actions$ + .pipe( + ofType(ADD, square), + map(update => update.type), + toArray() + ) + .subscribe({ + next(actual) { + expect(actual).toEqual(expected); + }, + }); + + [...actions, square.type].forEach(action => + dispatcher.next({ type: action }) + ); + + dispatcher.complete(); + }); + + it('should filter out actions by action creator and type string, with explicit type argument', () => { + const expected = [...actions.filter(type => type === ADD), square.type]; + + actions$ + .pipe( + // Provided type overrides any inference from arguments + ofType>(ADD, square), + map(update => update.type), + toArray() + ) + .subscribe({ + next(actual) { + expect(actual).toEqual(expected); + }, + }); + + [...actions, square.type].forEach(action => + dispatcher.next({ type: action }) + ); + + dispatcher.complete(); + }); + + it('should filter out up to 5 actions with type inference', () => { + // Mixing all of them, up to 5 + const expected = [divide.type, ADD, square.type, SUBTRACT, multiply.type]; + + actions$ + .pipe( + ofType(divide, ADD, square, SUBTRACT, multiply), + map(update => update.type), + toArray() + ) + .subscribe({ + next(actual) { + expect(actual).toEqual(expected); + }, + }); + + // Actions under test, in specific order + dispatcher.next(divide({ by: 1 })); + dispatcher.next({ type: ADD }); + dispatcher.next(square()); + dispatcher.next({ type: SUBTRACT }); + dispatcher.next(multiply({ by: 2 })); dispatcher.complete(); }); }); diff --git a/modules/effects/src/actions.ts b/modules/effects/src/actions.ts index 7d24ed9a2d..a76cc0de05 100644 --- a/modules/effects/src/actions.ts +++ b/modules/effects/src/actions.ts @@ -1,5 +1,10 @@ import { Inject, Injectable } from '@angular/core'; -import { Action, ScannedActionsSubject } from '@ngrx/store'; +import { + Action, + ActionCreator, + Creator, + ScannedActionsSubject, +} from '@ngrx/store'; import { Observable, OperatorFunction, Operator } from 'rxjs'; import { filter } from 'rxjs/operators'; @@ -21,6 +26,12 @@ export class Actions extends Observable { } } +// Module-private helper type +type ActionExtractor< + T extends string | AC, + AC extends ActionCreator, + E +> = T extends string ? E : ReturnType>; /** * 'ofType' filters an Observable of Actions into an observable of the actions * whose type strings are passed to it. @@ -44,39 +55,49 @@ export class Actions extends Observable { * like `actions.ofType('add')`. */ export function ofType< - V extends Extract, - T1 extends string = string, - U extends Action = Action + E extends Extract, + AC extends ActionCreator, + T1 extends string | AC, + U extends Action = Action, + V = T1 extends string ? E : ReturnType> >(t1: T1): OperatorFunction; export function ofType< - V extends Extract, - T1 extends string = string, - T2 extends string = string, - U extends Action = Action + E extends Extract, + AC extends ActionCreator, + T1 extends string | AC, + T2 extends string | AC, + U extends Action = Action, + V = ActionExtractor >(t1: T1, t2: T2): OperatorFunction; export function ofType< - V extends Extract, - T1 extends string = string, - T2 extends string = string, - T3 extends string = string, - U extends Action = Action + E extends Extract, + AC extends ActionCreator, + T1 extends string | AC, + T2 extends string | AC, + T3 extends string | AC, + U extends Action = Action, + V = ActionExtractor >(t1: T1, t2: T2, t3: T3): OperatorFunction; export function ofType< - V extends Extract, - T1 extends string = string, - T2 extends string = string, - T3 extends string = string, - T4 extends string = string, - U extends Action = Action + E extends Extract, + AC extends ActionCreator, + T1 extends string | AC, + T2 extends string | AC, + T3 extends string | AC, + T4 extends string | AC, + U extends Action = Action, + V = ActionExtractor >(t1: T1, t2: T2, t3: T3, t4: T4): OperatorFunction; export function ofType< - V extends Extract, - T1 extends string = string, - T2 extends string = string, - T3 extends string = string, - T4 extends string = string, - T5 extends string = string, - U extends Action = Action + E extends Extract, + AC extends ActionCreator, + T1 extends string | AC, + T2 extends string | AC, + T3 extends string | AC, + T4 extends string | AC, + T5 extends string | AC, + U extends Action = Action, + V = ActionExtractor >(t1: T1, t2: T2, t3: T3, t4: T4, t5: T5): OperatorFunction; /** * Fallback for more than 5 arguments. @@ -87,12 +108,27 @@ export function ofType< * arguments, to preserve backwards compatibility with old versions of ngrx. */ export function ofType( - ...allowedTypes: string[] + ...allowedTypes: Array> ): OperatorFunction; export function ofType( - ...allowedTypes: string[] + ...allowedTypes: Array> ): OperatorFunction { - return filter((action: Action) => - allowedTypes.some(type => type === action.type) - ); + return filter((action: Action) => { + for (let typeOrActionCreator of allowedTypes) { + const actionCreatorType = (typeOrActionCreator as ActionCreator< + string, + Creator + >).type; + if (actionCreatorType !== undefined) { + // We are filtering by ActionCreator + if (actionCreatorType === action.type) { + return true; + } + // Comparing the string to type + } else if (typeOrActionCreator === action.type) { + return true; + } + } + return false; + }); } From 531d115973831e27746e1934ed1a520bfd720619 Mon Sep 17 00:00:00 2001 From: aokrushko Date: Mon, 1 Apr 2019 06:35:39 -0400 Subject: [PATCH 2/2] Use Array.some and compare to string first --- modules/effects/src/actions.ts | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/modules/effects/src/actions.ts b/modules/effects/src/actions.ts index a76cc0de05..0cd6857f3b 100644 --- a/modules/effects/src/actions.ts +++ b/modules/effects/src/actions.ts @@ -113,22 +113,15 @@ export function ofType( export function ofType( ...allowedTypes: Array> ): OperatorFunction { - return filter((action: Action) => { - for (let typeOrActionCreator of allowedTypes) { - const actionCreatorType = (typeOrActionCreator as ActionCreator< - string, - Creator - >).type; - if (actionCreatorType !== undefined) { - // We are filtering by ActionCreator - if (actionCreatorType === action.type) { - return true; - } + return filter((action: Action) => + allowedTypes.some(typeOrActionCreator => { + if (typeof typeOrActionCreator === 'string') { // Comparing the string to type - } else if (typeOrActionCreator === action.type) { - return true; + return typeOrActionCreator === action.type; } - } - return false; - }); + + // We are filtering by ActionCreator + return typeOrActionCreator.type === action.type; + }) + ); }