From 03a3a70a95930c450f222ca901cc7425f73d60fe Mon Sep 17 00:00:00 2001 From: timdeschryver <28659384+timdeschryver@users.noreply.github.com> Date: Fri, 25 Jan 2019 21:31:12 +0100 Subject: [PATCH 1/3] test(StoreDevTools): add ignore action tests These actions being predicate, actionsBlacklist and actionsWhitelist --- .../store-devtools/spec/integration.spec.ts | 95 +++++++++++++++---- 1 file changed, 75 insertions(+), 20 deletions(-) diff --git a/modules/store-devtools/spec/integration.spec.ts b/modules/store-devtools/spec/integration.spec.ts index 23e88d95c0..9e03866bdd 100644 --- a/modules/store-devtools/spec/integration.spec.ts +++ b/modules/store-devtools/spec/integration.spec.ts @@ -1,36 +1,49 @@ import { NgModule } from '@angular/core'; import { TestBed } from '@angular/core/testing'; -import { StoreModule, Store, ActionsSubject } from '@ngrx/store'; -import { StoreDevtoolsModule, StoreDevtools } from '@ngrx/store-devtools'; +import { StoreModule, Store, Action } from '@ngrx/store'; +import { + StoreDevtoolsModule, + StoreDevtools, + StoreDevtoolsOptions, +} from '@ngrx/store-devtools'; describe('Devtools Integration', () => { - let store: Store; - - @NgModule({ - imports: [StoreModule.forFeature('a', (state: any, action: any) => state)], - }) - class EagerFeatureModule {} - - @NgModule({ - imports: [ - StoreModule.forRoot({}), - EagerFeatureModule, - StoreDevtoolsModule.instrument(), - ], - }) - class RootModule {} - - beforeEach(() => { + function setup(options: Partial = {}) { + @NgModule({ + imports: [ + StoreModule.forFeature('a', (state: any, action: any) => state), + ], + }) + class EagerFeatureModule {} + + @NgModule({ + imports: [ + StoreModule.forRoot({}), + EagerFeatureModule, + StoreDevtoolsModule.instrument(options), + ], + }) + class RootModule {} + TestBed.configureTestingModule({ imports: [RootModule], }); + + const store = TestBed.get(Store) as Store; + const devtools = TestBed.get(StoreDevtools) as StoreDevtools; + return { store, devtools }; + } + + afterEach(() => { + const devtools = TestBed.get(StoreDevtools) as StoreDevtools; + devtools.reset(); }); it('should load the store eagerly', () => { let error = false; try { - let store = TestBed.get(Store); + const { store } = setup(); store.subscribe(); } catch (e) { error = true; @@ -38,4 +51,46 @@ describe('Devtools Integration', () => { expect(error).toBeFalsy(); }); + + it('should not throw if actions are ignored', (done: any) => { + const { store, devtools } = setup({ + predicate: (_, { type }: Action) => type !== 'FOO', + }); + store.subscribe(); + devtools.dispatcher.subscribe((action: Action) => { + if (action.type === 'REFRESH') { + done(); + } + }); + store.dispatch({ type: 'FOO' }); + devtools.refresh(); + }); + + it('should not throw if actions are blacklisted', (done: any) => { + const { store, devtools } = setup({ + actionsBlacklist: ['FOO'], + }); + store.subscribe(); + devtools.dispatcher.subscribe((action: Action) => { + if (action.type === 'REFRESH') { + done(); + } + }); + store.dispatch({ type: 'FOO' }); + devtools.refresh(); + }); + + it('should not throw if actions are whitelisted', (done: any) => { + const { store, devtools } = setup({ + actionsWhitelist: ['@ngrx/store/update-reducers'], + }); + store.subscribe(); + devtools.dispatcher.subscribe((action: Action) => { + if (action.type === 'REFRESH') { + done(); + } + }); + store.dispatch({ type: 'FOO' }); + devtools.refresh(); + }); }); From 07cd1de308ce6cf25f2f95f57800ed39d9609eb8 Mon Sep 17 00:00:00 2001 From: timdeschryver <28659384+timdeschryver@users.noreply.github.com> Date: Fri, 25 Jan 2019 21:35:24 +0100 Subject: [PATCH 2/3] fix(StoreDevTools): take ignored actions into account --- modules/store-devtools/spec/store.spec.ts | 119 ++++++++++++++++++++++ modules/store-devtools/src/reducer.ts | 16 ++- modules/store-devtools/src/utils.ts | 13 +-- 3 files changed, 139 insertions(+), 9 deletions(-) diff --git a/modules/store-devtools/spec/store.spec.ts b/modules/store-devtools/spec/store.spec.ts index 42e9e5ba41..126f8391b7 100644 --- a/modules/store-devtools/spec/store.spec.ts +++ b/modules/store-devtools/spec/store.spec.ts @@ -438,6 +438,125 @@ describe('Store Devtools', () => { }); }); + describe('Filtered actions', () => { + it('should respect the predicate option', () => { + const fixture = createStore(counter, { + predicate: (s, a) => a.type !== 'INCREMENT', + }); + + expect(fixture.getState()).toBe(0); + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'DECREMENT' }); + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'DECREMENT' }); + expect(fixture.getState()).toBe(5); + + // init, decrement, decrement + const { + stagedActionIds, + actionsById, + computedStates, + currentStateIndex, + } = fixture.getLiftedState(); + expect(stagedActionIds.length).toBe(3); + expect(Object.keys(actionsById).length).toBe(3); + expect(computedStates.length).toBe(3); + expect(currentStateIndex).toBe(2); + + fixture.devtools.jumpToAction(0); + expect(fixture.getState()).toBe(1); + + fixture.devtools.jumpToAction(1); + expect(fixture.getState()).toBe(6); + + fixture.devtools.jumpToAction(2); + expect(fixture.getState()).toBe(5); + }); + + it('should respect the blacklist option', () => { + const fixture = createStore(counter, { + actionsBlacklist: ['INCREMENT'], + }); + + expect(fixture.getState()).toBe(0); + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'DECREMENT' }); + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'DECREMENT' }); + expect(fixture.getState()).toBe(5); + + // init, decrement, decrement + const { + stagedActionIds, + actionsById, + computedStates, + currentStateIndex, + } = fixture.getLiftedState(); + expect(stagedActionIds.length).toBe(3); + expect(Object.keys(actionsById).length).toBe(3); + expect(computedStates.length).toBe(3); + expect(currentStateIndex).toBe(2); + + fixture.devtools.jumpToAction(0); + expect(fixture.getState()).toBe(1); + + fixture.devtools.jumpToAction(1); + expect(fixture.getState()).toBe(6); + + fixture.devtools.jumpToAction(2); + expect(fixture.getState()).toBe(5); + }); + + it('should respect the whitelist option', () => { + const fixture = createStore(counter, { + actionsWhitelist: ['DECREMENT'], + }); + + expect(fixture.getState()).toBe(0); + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'DECREMENT' }); + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'DECREMENT' }); + expect(fixture.getState()).toBe(5); + + // init, decrement, decrement + const { + stagedActionIds, + actionsById, + computedStates, + currentStateIndex, + } = fixture.getLiftedState(); + expect(stagedActionIds.length).toBe(3); + expect(Object.keys(actionsById).length).toBe(3); + expect(computedStates.length).toBe(3); + expect(currentStateIndex).toBe(2); + + fixture.devtools.jumpToAction(0); + expect(fixture.getState()).toBe(1); + + fixture.devtools.jumpToAction(1); + expect(fixture.getState()).toBe(6); + + fixture.devtools.jumpToAction(2); + expect(fixture.getState()).toBe(5); + }); + }); + describe('maxAge option', () => { it('should auto-commit earliest non-@@INIT action when maxAge is reached', () => { const fixture = createStore(counter, { maxAge: 3 }); diff --git a/modules/store-devtools/src/reducer.ts b/modules/store-devtools/src/reducer.ts index 9fd203de42..27f9f68680 100644 --- a/modules/store-devtools/src/reducer.ts +++ b/modules/store-devtools/src/reducer.ts @@ -7,7 +7,7 @@ import { UPDATE, INIT, } from '@ngrx/store'; -import { difference, liftAction } from './utils'; +import { difference, liftAction, isActionFiltered } from './utils'; import * as DevtoolsActions from './actions'; import { StoreDevtoolsConfig, StateSanitizer } from './config'; import { PerformAction } from './actions'; @@ -362,8 +362,18 @@ export function liftReducerWith( return liftedState || initialLiftedState; } - if (isPaused) { - // If recording is paused, overwrite the last state + if ( + isPaused || + (liftedState && + isActionFiltered( + liftedState.computedStates[currentStateIndex], + liftedAction, + options.predicate, + options.actionsWhitelist, + options.actionsBlacklist + )) + ) { + // If recording is paused or if the action should be ignored, overwrite the last state // (corresponds to the pause action) and keep everything else as is. // This way, the app gets the new current state while the devtools // do not record another action. diff --git a/modules/store-devtools/src/utils.ts b/modules/store-devtools/src/utils.ts index 23255a1650..017cc893a1 100644 --- a/modules/store-devtools/src/utils.ts +++ b/modules/store-devtools/src/utils.ts @@ -14,6 +14,7 @@ import { LiftedActions, LiftedState, } from './reducer'; +import { black } from '@angular-devkit/core/src/terminal/colors'; export function difference(first: any[], second: any[]) { return first.filter(item => second.indexOf(item) < 0); @@ -25,7 +26,6 @@ export function difference(first: any[], second: any[]) { export function unliftState(liftedState: LiftedState) { const { computedStates, currentStateIndex } = liftedState; const { state } = computedStates[currentStateIndex]; - return state; } @@ -155,9 +155,10 @@ export function isActionFiltered( whitelist?: string[], blacklist?: string[] ) { - return ( - (predicate && !predicate(state, action.action)) || - (whitelist && !action.action.type.match(whitelist.join('|'))) || - (blacklist && action.action.type.match(blacklist.join('|'))) - ); + const predicateMatch = predicate && !predicate(state, action.action); + const whitelistMatch = + whitelist && !action.action.type.match(whitelist.join('|')); + const blacklistMatch = + blacklist && action.action.type.match(blacklist.join('|')); + return predicateMatch || whitelistMatch || blacklistMatch; } From 3d3f6f044302d6fc35d945b4024c3e0a84d2e572 Mon Sep 17 00:00:00 2001 From: timdeschryver <28659384+timdeschryver@users.noreply.github.com> Date: Sat, 26 Jan 2019 16:59:56 +0100 Subject: [PATCH 3/3] fix(StoreDevTools): fall back to last known state when out of bounds --- modules/store-devtools/spec/integration.spec.ts | 2 +- modules/store-devtools/src/utils.ts | 11 ++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/modules/store-devtools/spec/integration.spec.ts b/modules/store-devtools/spec/integration.spec.ts index 9e03866bdd..8160ad3c5c 100644 --- a/modules/store-devtools/spec/integration.spec.ts +++ b/modules/store-devtools/spec/integration.spec.ts @@ -82,7 +82,7 @@ describe('Devtools Integration', () => { it('should not throw if actions are whitelisted', (done: any) => { const { store, devtools } = setup({ - actionsWhitelist: ['@ngrx/store/update-reducers'], + actionsWhitelist: ['BAR'], }); store.subscribe(); devtools.dispatcher.subscribe((action: Action) => { diff --git a/modules/store-devtools/src/utils.ts b/modules/store-devtools/src/utils.ts index 017cc893a1..1c753c53d8 100644 --- a/modules/store-devtools/src/utils.ts +++ b/modules/store-devtools/src/utils.ts @@ -14,7 +14,6 @@ import { LiftedActions, LiftedState, } from './reducer'; -import { black } from '@angular-devkit/core/src/terminal/colors'; export function difference(first: any[], second: any[]) { return first.filter(item => second.indexOf(item) < 0); @@ -25,6 +24,16 @@ export function difference(first: any[], second: any[]) { */ export function unliftState(liftedState: LiftedState) { const { computedStates, currentStateIndex } = liftedState; + + // At start up NgRx dispatches init actions, + // When these init actions are being filtered out by the predicate or black/white list options + // we don't have a complete computed states yet. + // At this point it could happen that we're out of bounds, when this happens we fall back to the last known state + if (currentStateIndex >= computedStates.length) { + const { state } = computedStates[computedStates.length - 1]; + return state; + } + const { state } = computedStates[currentStateIndex]; return state; }