From a5dcdb143ca1e75d291bc04338fd3225d793ac7f Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Mon, 5 Mar 2018 04:51:01 +0100 Subject: [PATCH] fix(StoreDevtools): Fix bug when exporting/importing state history (#855) * Also refactors state / action sanitization * Improved consistency for config object provided to extension * Moved action/state sanitization functions into utils --- modules/store-devtools/spec/extension.spec.ts | 332 +++- modules/store-devtools/spec/store.spec.ts | 1358 ++++++++--------- modules/store-devtools/src/devtools.ts | 26 +- modules/store-devtools/src/extension.ts | 99 +- modules/store-devtools/src/reducer.ts | 53 +- modules/store-devtools/src/utils.ts | 69 +- 6 files changed, 1072 insertions(+), 865 deletions(-) diff --git a/modules/store-devtools/spec/extension.spec.ts b/modules/store-devtools/spec/extension.spec.ts index 0fc724a3fb..ae80bc5674 100644 --- a/modules/store-devtools/spec/extension.spec.ts +++ b/modules/store-devtools/spec/extension.spec.ts @@ -1,85 +1,343 @@ +import { LiftedActions, ComputedState, LiftedAction } from './../src/reducer'; +import { PerformAction, PERFORM_ACTION } from './../src/actions'; +import { ActionSanitizer, StateSanitizer } from './../src/config'; +import { + ReduxDevtoolsExtensionConnection, + ReduxDevtoolsExtensionConfig, +} from './../src/extension'; import { Action } from '@ngrx/store'; -import { of } from 'rxjs/observable/of'; import { LiftedState } from '../'; import { DevtoolsExtension, ReduxDevtoolsExtension } from '../src/extension'; import { createConfig, noMonitor } from '../src/instrument'; +import { unliftState } from '../src/utils'; + +function createOptions( + name: string = 'NgRx Store DevTools', + features: any = false, + serialize: boolean = false, + maxAge: false | number = false +) { + const options: ReduxDevtoolsExtensionConfig = { + instanceId: 'ngrx-store-1509655064369', + name, + features, + serialize, + }; + if (maxAge !== false /* support === 0 */) { + options.maxAge = maxAge; + } + return options; +} + +function createState( + actionsById?: LiftedActions, + computedStates?: ComputedState[] +) { + return { + monitorState: null, + nextActionId: 1, + actionsById: actionsById || { + 0: { type: PERFORM_ACTION, action: { type: 'SOME_ACTION' } }, + }, + stagedActionIds: [0], + skippedActionIds: [], + committedState: { count: 0 }, + currentStateIndex: 0, + computedStates: computedStates || [ + { + state: 1, + error: null, + }, + ], + }; +} describe('DevtoolsExtension', () => { let reduxDevtoolsExtension: ReduxDevtoolsExtension; + let extensionConnection: ReduxDevtoolsExtensionConnection; let devtoolsExtension: DevtoolsExtension; beforeEach(() => { + extensionConnection = jasmine.createSpyObj( + 'reduxDevtoolsExtensionConnection', + ['init', 'subscribe', 'unsubscribe', 'send'] + ); reduxDevtoolsExtension = jasmine.createSpyObj('reduxDevtoolsExtension', [ 'send', 'connect', ]); - (reduxDevtoolsExtension.connect as jasmine.Spy).and.returnValue(of({})); + (reduxDevtoolsExtension.connect as jasmine.Spy).and.returnValue( + extensionConnection + ); spyOn(Date, 'now').and.returnValue('1509655064369'); }); + function myActionSanitizer(action: Action, idx: number) { + return action; + } + + function myStateSanitizer(state: any, idx: number) { + return state; + } + + it('should connect with default options', () => { + devtoolsExtension = new DevtoolsExtension( + reduxDevtoolsExtension, + createConfig({}) + ); + // Subscription needed or else extension connection will not be established. + devtoolsExtension.actions$.subscribe(() => null); + const defaultOptions = createOptions(); + expect(reduxDevtoolsExtension.connect).toHaveBeenCalledWith(defaultOptions); + }); + + it('should connect with given options', () => { + devtoolsExtension = new DevtoolsExtension( + reduxDevtoolsExtension, + createConfig({ + name: 'ngrx-store-devtool-todolist', + features: 'some features', + maxAge: 10, + serialize: true, + // these two should not be added + actionSanitizer: myActionSanitizer, + stateSanitizer: myStateSanitizer, + }) + ); + // Subscription needed or else extension connection will not be established. + devtoolsExtension.actions$.subscribe(() => null); + const options = createOptions( + 'ngrx-store-devtool-todolist', + 'some features', + true, + 10 + ); + expect(reduxDevtoolsExtension.connect).toHaveBeenCalledWith(options); + }); + describe('notify', () => { it('should send notification with default options', () => { devtoolsExtension = new DevtoolsExtension( reduxDevtoolsExtension, createConfig({}) ); - const defaultOptions = { - maxAge: false, - monitor: noMonitor, - actionSanitizer: undefined, - stateSanitizer: undefined, - name: 'NgRx Store DevTools', - serialize: false, - logOnly: false, - features: false, - }; - const action = {} as Action; - const state = {} as LiftedState; + const defaultOptions = createOptions(); + const action = {} as LiftedAction; + const state = createState(); devtoolsExtension.notify(action, state); expect(reduxDevtoolsExtension.send).toHaveBeenCalledWith( null, - {}, + state, defaultOptions, 'ngrx-store-1509655064369' ); }); - function myActionSanitizer() { - return { type: 'sanitizer' }; - } - function myStateSanitizer() { - return { state: 'new state' }; - } - it('should send notification with given options', () => { devtoolsExtension = new DevtoolsExtension( reduxDevtoolsExtension, createConfig({ + name: 'ngrx-store-devtool-todolist', + features: 'some features', + maxAge: 10, + serialize: true, + // these two should not be added actionSanitizer: myActionSanitizer, stateSanitizer: myStateSanitizer, - name: 'ngrx-store-devtool-todolist', }) ); - const defaultOptions = { - maxAge: false, - monitor: noMonitor, - actionSanitizer: myActionSanitizer, - stateSanitizer: myStateSanitizer, - name: 'ngrx-store-devtool-todolist', - serialize: false, - logOnly: false, - features: false, - }; - const action = {} as Action; - const state = {} as LiftedState; + const options = createOptions( + 'ngrx-store-devtool-todolist', + 'some features', + true, + 10 + ); + const action = {} as LiftedAction; + const state = createState(); devtoolsExtension.notify(action, state); expect(reduxDevtoolsExtension.send).toHaveBeenCalledWith( null, - {}, - defaultOptions, + state, + options, 'ngrx-store-1509655064369' ); }); + + describe('[with Action and State Sanitizer]', () => { + const UNSANITIZED_TOKEN = 'UNSANITIZED_ACTION'; + const SANITIZED_TOKEN = 'SANITIZED_ACTION'; + const SANITIZED_COUNTER = 42; + + function createPerformAction() { + return new PerformAction({ type: UNSANITIZED_TOKEN }); + } + + function testActionSanitizer(action: Action, id: number) { + return { type: SANITIZED_TOKEN }; + } + + function testStateSanitizer(state: any, index: number) { + return SANITIZED_COUNTER; + } + + describe('should function normally with no sanitizers', () => { + beforeEach(() => { + devtoolsExtension = new DevtoolsExtension( + reduxDevtoolsExtension, + createConfig({}) + ); + // Subscription needed or else extension connection will not be established. + devtoolsExtension.actions$.subscribe(() => null); + }); + + it('for normal action', () => { + const action = createPerformAction(); + const state = createState(); + + devtoolsExtension.notify(action, state); + expect(extensionConnection.send).toHaveBeenCalledWith( + action, + unliftState(state) + ); + }); + + it('for action that requires full state update', () => { + const options = createOptions(); + const action = {} as LiftedAction; + const state = createState(); + + devtoolsExtension.notify(action, state); + expect(reduxDevtoolsExtension.send).toHaveBeenCalledWith( + null, + state, + options, + 'ngrx-store-1509655064369' + ); + }); + }); + + describe('should run the action sanitizer on actions', () => { + beforeEach(() => { + devtoolsExtension = new DevtoolsExtension( + reduxDevtoolsExtension, + createConfig({ + actionSanitizer: testActionSanitizer, + }) + ); + // Subscription needed or else extension connection will not be established. + devtoolsExtension.actions$.subscribe(() => null); + }); + + it('for normal action', () => { + const options = createOptions(); + const action = createPerformAction(); + const state = createState(); + const sanitizedAction = { + ...action, + action: testActionSanitizer(createPerformAction().action, 0), + }; + + devtoolsExtension.notify(action, state); + expect(extensionConnection.send).toHaveBeenCalledWith( + sanitizedAction, + unliftState(state) + ); + }); + + it('for action that requires full state update', () => { + const options = createOptions(); + const action = {} as LiftedAction; + const state = createState(); + const sanitizedState = createState({ + 0: { type: PERFORM_ACTION, action: { type: SANITIZED_TOKEN } }, + }); + + devtoolsExtension.notify(action, state); + expect(reduxDevtoolsExtension.send).toHaveBeenCalledWith( + null, + sanitizedState, + options, + 'ngrx-store-1509655064369' + ); + }); + }); + + describe('should run the state sanitizer on store state', () => { + beforeEach(() => { + devtoolsExtension = new DevtoolsExtension( + reduxDevtoolsExtension, + createConfig({ + stateSanitizer: testStateSanitizer, + }) + ); + // Subscription needed or else extension connection will not be established. + devtoolsExtension.actions$.subscribe(() => null); + }); + + it('for normal action', () => { + const action = createPerformAction(); + const state = createState(); + const sanitizedState = createState(undefined, [ + { state: SANITIZED_COUNTER, error: null }, + ]); + + devtoolsExtension.notify(action, state); + expect(extensionConnection.send).toHaveBeenCalledWith( + action, + unliftState(sanitizedState) + ); + }); + + it('for action that requires full state update', () => { + const options = createOptions(); + const action = {} as LiftedAction; + const state = createState(); + const sanitizedState = createState(undefined, [ + { state: SANITIZED_COUNTER, error: null }, + ]); + + devtoolsExtension.notify(action, state); + expect(reduxDevtoolsExtension.send).toHaveBeenCalledWith( + null, + sanitizedState, + options, + 'ngrx-store-1509655064369' + ); + }); + }); + + it('sanitizers should not modify original state or actions', () => { + beforeEach(() => { + devtoolsExtension = new DevtoolsExtension( + reduxDevtoolsExtension, + createConfig({ + actionSanitizer: testActionSanitizer, + stateSanitizer: testStateSanitizer, + }) + ); + // Subscription needed or else extension connection will not be established. + devtoolsExtension.actions$.subscribe(() => null); + }); + + it('for normal action', () => { + const action = createPerformAction(); + const state = createState(); + + devtoolsExtension.notify(action, state); + expect(state).toEqual(createState()); + expect(action).toEqual(createPerformAction()); + }); + + it('for action that requires full state update', () => { + const action = {} as LiftedAction; + const state = createState(); + + devtoolsExtension.notify(action, state); + expect(state).toEqual(createState()); + expect(action).toEqual({} as LiftedAction); + }); + }); + }); }); }); diff --git a/modules/store-devtools/spec/store.spec.ts b/modules/store-devtools/spec/store.spec.ts index 681b319a27..5e1c334763 100644 --- a/modules/store-devtools/spec/store.spec.ts +++ b/modules/store-devtools/spec/store.spec.ts @@ -1,745 +1,613 @@ -import 'rxjs/add/operator/take'; -import { Subscription } from 'rxjs/Subscription'; -import { ReflectiveInjector } from '@angular/core'; -import { TestBed, getTestBed } from '@angular/core/testing'; -import { - StoreModule, - Store, - StateObservable, - ActionReducer, - Action, - ReducerManager, -} from '@ngrx/store'; -import { - StoreDevtools, - StoreDevtoolsModule, - LiftedState, - StoreDevtoolsConfig, - StoreDevtoolsOptions, -} from '../'; -import { IS_EXTENSION_OR_MONITOR_PRESENT } from '../src/instrument'; - -const counter = jasmine - .createSpy('counter') - .and.callFake(function(state = 0, action: Action) { - switch (action.type) { - case 'INCREMENT': - return state + 1; - case 'DECREMENT': - return state - 1; - default: - return state; - } - }); - -declare var mistake: any; -function counterWithBug(state = 0, action: Action) { - switch (action.type) { - case 'INCREMENT': - return state + 1; - case 'DECREMENT': - return mistake - 1; // mistake is undefined - case 'SET_UNDEFINED': - return undefined; - default: - return state; - } -} - -function counterWithAnotherBug(state = 0, action: Action) { - switch (action.type) { - case 'INCREMENT': - return mistake + 1; // eslint-disable-line no-undef - case 'DECREMENT': - return state - 1; - case 'SET_UNDEFINED': - return undefined; - default: - return state; - } -} - -function doubleCounter(state = 0, action: Action) { - switch (action.type) { - case 'INCREMENT': - return state + 2; - case 'DECREMENT': - return state - 2; - default: - return state; - } -} - -type Fixture = { - store: Store; - state: StateObservable; - devtools: StoreDevtools; - cleanup: () => void; - getState: () => T; - getLiftedState: () => LiftedState; - replaceReducer: (reducer: ActionReducer) => void; -}; - -function createStore( - reducer: ActionReducer, - options: StoreDevtoolsOptions = {} -): Fixture { - TestBed.configureTestingModule({ - imports: [ - StoreModule.forRoot({ state: reducer }), - StoreDevtoolsModule.instrument(options), - ], - providers: [{ provide: IS_EXTENSION_OR_MONITOR_PRESENT, useValue: true }], - }); - - const testbed: TestBed = getTestBed(); - const store: Store = testbed.get(Store); - const devtools: StoreDevtools = testbed.get(StoreDevtools); - const state: StateObservable = testbed.get(StateObservable); - const reducerManager: ReducerManager = testbed.get(ReducerManager); - let liftedValue: LiftedState; - let value: any; - - const liftedStateSub = devtools.liftedState.subscribe(s => (liftedValue = s)); - const stateSub = devtools.state.subscribe(s => (value = s)); - - const getState = (): T => value.state; - const getLiftedState = (): LiftedState => liftedValue; - - const cleanup = () => { - liftedStateSub.unsubscribe(); - stateSub.unsubscribe(); - }; - - const replaceReducer = (reducer: ActionReducer) => { - reducerManager.addReducer('state', reducer); - }; - - return { - store, - state, - devtools, - cleanup, - getState, - getLiftedState, - replaceReducer, - }; -} - -describe('Store Devtools', () => { - describe('Instrumentation', () => { - let fixture: Fixture; - let store: Store; - let devtools: StoreDevtools; - let getState: () => number; - let getLiftedState: () => LiftedState; - - beforeEach(() => { - fixture = createStore(counter); - store = fixture.store; - devtools = fixture.devtools; - getState = fixture.getState; - getLiftedState = fixture.getLiftedState; - }); - - afterEach(() => { - fixture.cleanup(); - }); - - it("should alias devtools unlifted state to Store's state", () => { - expect(devtools.state).toBe(fixture.state as any); - }); - - it('should perform actions', () => { - expect(getState()).toBe(0); - store.dispatch({ type: 'INCREMENT' }); - expect(getState()).toBe(1); - store.dispatch({ type: 'INCREMENT' }); - expect(getState()).toBe(2); - }); - - it('should rollback state to the last committed state', () => { - store.dispatch({ type: 'INCREMENT' }); - store.dispatch({ type: 'INCREMENT' }); - expect(getState()).toBe(2); - - devtools.commit(); - expect(getState()).toBe(2); - - store.dispatch({ type: 'INCREMENT' }); - store.dispatch({ type: 'INCREMENT' }); - expect(getState()).toBe(4); - - devtools.rollback(); - expect(getState()).toBe(2); - - store.dispatch({ type: 'DECREMENT' }); - expect(getState()).toBe(1); - - devtools.rollback(); - expect(getState()).toBe(2); - }); - - it('should reset to initial state', () => { - store.dispatch({ type: 'INCREMENT' }); - expect(getState()).toBe(1); - - devtools.commit(); - expect(getState()).toBe(1); - - store.dispatch({ type: 'INCREMENT' }); - expect(getState()).toBe(2); - - devtools.rollback(); - expect(getState()).toBe(1); - - store.dispatch({ type: 'INCREMENT' }); - expect(getState()).toBe(2); - - devtools.reset(); - expect(getState()).toBe(0); - }); - - it('should toggle an action', () => { - // actionId 0 = @@INIT - store.dispatch({ type: 'INCREMENT' }); - store.dispatch({ type: 'DECREMENT' }); - store.dispatch({ type: 'INCREMENT' }); - expect(getState()).toBe(1); - - devtools.toggleAction(2); - expect(getState()).toBe(2); - - devtools.toggleAction(2); - expect(getState()).toBe(1); - }); - - it('should sweep disabled actions', () => { - // actionId 0 = @@INIT - store.dispatch({ type: 'INCREMENT' }); - store.dispatch({ type: 'DECREMENT' }); - store.dispatch({ type: 'INCREMENT' }); - store.dispatch({ type: 'INCREMENT' }); - - expect(getState()).toBe(2); - expect(getLiftedState().stagedActionIds).toEqual([0, 1, 2, 3, 4]); - expect(getLiftedState().skippedActionIds).toEqual([]); - - devtools.toggleAction(2); - - expect(getState()).toBe(3); - expect(getLiftedState().stagedActionIds).toEqual([0, 1, 2, 3, 4]); - expect(getLiftedState().skippedActionIds).toEqual([2]); - - devtools.sweep(); - expect(getState()).toBe(3); - expect(getLiftedState().stagedActionIds).toEqual([0, 1, 3, 4]); - expect(getLiftedState().skippedActionIds).toEqual([]); - }); - - it('should jump to state', () => { - store.dispatch({ type: 'INCREMENT' }); - store.dispatch({ type: 'DECREMENT' }); - store.dispatch({ type: 'INCREMENT' }); - expect(getState()).toBe(1); - - devtools.jumpToState(0); - expect(getState()).toBe(0); - - devtools.jumpToState(1); - expect(getState()).toBe(1); - - devtools.jumpToState(2); - expect(getState()).toBe(0); - - store.dispatch({ type: 'INCREMENT' }); - expect(getState()).toBe(0); - - devtools.jumpToState(4); - expect(getState()).toBe(2); - }); - - it('should jump to action', () => { - store.dispatch({ type: 'INCREMENT' }); - store.dispatch({ type: 'DECREMENT' }); - store.dispatch({ type: 'INCREMENT' }); - expect(getState()).toBe(1); - - devtools.jumpToAction(2); - expect(getState()).toBe(0); - }); - - it('should replace the reducer and preserve previous states', () => { - store.dispatch({ type: 'INCREMENT' }); - store.dispatch({ type: 'DECREMENT' }); - store.dispatch({ type: 'INCREMENT' }); - - expect(getState()).toBe(1); - - fixture.replaceReducer(doubleCounter); - - expect(getState()).toBe(1); - }); - - it('should replace the reducer and compute new state with latest reducer', () => { - store.dispatch({ type: 'INCREMENT' }); - store.dispatch({ type: 'DECREMENT' }); - store.dispatch({ type: 'INCREMENT' }); - - expect(getState()).toBe(1); - - fixture.replaceReducer(doubleCounter); - - store.dispatch({ type: 'INCREMENT' }); - - expect(getState()).toBe(3); - }); - - it('should catch and record errors', () => { - spyOn(console, 'error'); - fixture.replaceReducer(counterWithBug); - - store.dispatch({ type: 'INCREMENT' }); - store.dispatch({ type: 'DECREMENT' }); - store.dispatch({ type: 'INCREMENT' }); - - let { computedStates } = fixture.getLiftedState(); - expect(computedStates[3].error).toMatch(/ReferenceError/); - expect(computedStates[4].error).toMatch( - /Interrupted by an error up the chain/ - ); - - expect(console.error).toHaveBeenCalled(); - }); - - it('should catch invalid action type', () => { - expect(() => { - store.dispatch({ type: undefined } as any); - }).toThrowError('Actions must have a type property'); - }); - - it('should not recompute old states when toggling an action', () => { - counter.calls.reset(); - - store.dispatch({ type: 'INCREMENT' }); - store.dispatch({ type: 'INCREMENT' }); - store.dispatch({ type: 'INCREMENT' }); - - expect(counter).toHaveBeenCalledTimes(3); - - devtools.toggleAction(3); - expect(counter).toHaveBeenCalledTimes(3); - - devtools.toggleAction(3); - expect(counter).toHaveBeenCalledTimes(4); - - devtools.toggleAction(2); - expect(counter).toHaveBeenCalledTimes(5); - - devtools.toggleAction(2); - expect(counter).toHaveBeenCalledTimes(7); - - devtools.toggleAction(1); - expect(counter).toHaveBeenCalledTimes(9); - - devtools.toggleAction(2); - expect(counter).toHaveBeenCalledTimes(10); - - devtools.toggleAction(3); - expect(counter).toHaveBeenCalledTimes(10); - - devtools.toggleAction(1); - expect(counter).toHaveBeenCalledTimes(11); - - devtools.toggleAction(3); - expect(counter).toHaveBeenCalledTimes(12); - - devtools.toggleAction(2); - expect(counter).toHaveBeenCalledTimes(14); - }); - - it('should not recompute states when jumping to state', () => { - counter.calls.reset(); - - store.dispatch({ type: 'INCREMENT' }); - store.dispatch({ type: 'INCREMENT' }); - store.dispatch({ type: 'INCREMENT' }); - - expect(counter).toHaveBeenCalledTimes(3); - - let savedComputedStates = getLiftedState().computedStates; - - devtools.jumpToState(0); - - expect(counter).toHaveBeenCalledTimes(3); - - devtools.jumpToState(1); - - expect(counter).toHaveBeenCalledTimes(3); - - devtools.jumpToState(3); - - expect(counter).toHaveBeenCalledTimes(3); - - expect(getLiftedState().computedStates).toBe(savedComputedStates); - }); - - it('should not recompute states on monitor actions', () => { - counter.calls.reset(); - - store.dispatch({ type: 'INCREMENT' }); - store.dispatch({ type: 'INCREMENT' }); - store.dispatch({ type: 'INCREMENT' }); - - expect(counter).toHaveBeenCalledTimes(3); - - let savedComputedStates = getLiftedState().computedStates; - - devtools.dispatch({ type: 'lol' }); - - expect(counter).toHaveBeenCalledTimes(3); - - devtools.dispatch({ type: 'wat' }); - - expect(counter).toHaveBeenCalledTimes(3); - - expect(getLiftedState().computedStates).toBe(savedComputedStates); - }); - }); - - describe('maxAge option', () => { - it('should auto-commit earliest non-@@INIT action when maxAge is reached', () => { - const fixture = createStore(counter, { maxAge: 3 }); - - fixture.store.dispatch({ type: 'INCREMENT' }); - fixture.store.dispatch({ type: 'INCREMENT' }); - let liftedStoreState = fixture.getLiftedState(); - - expect(fixture.getState()).toBe(2); - expect(Object.keys(liftedStoreState.actionsById).length).toBe(3); - expect(liftedStoreState.committedState).toBe(undefined); - expect(liftedStoreState.stagedActionIds).toContain(1); - - // Trigger auto-commit. - fixture.store.dispatch({ type: 'INCREMENT' }); - liftedStoreState = fixture.getLiftedState(); - - expect(fixture.getState()).toBe(3); - expect(Object.keys(liftedStoreState.actionsById).length).toBe(3); - expect(liftedStoreState.stagedActionIds).not.toContain(1); - expect(liftedStoreState.computedStates[0].state).toEqual({ state: 1 }); - expect(liftedStoreState.committedState).toEqual({ state: 1 }); - expect(liftedStoreState.currentStateIndex).toBe(2); - - fixture.cleanup(); - }); - - it('should remove skipped actions once committed', () => { - const fixture = createStore(counter, { maxAge: 3 }); - - fixture.store.dispatch({ type: 'INCREMENT' }); - fixture.devtools.toggleAction(1); - fixture.store.dispatch({ type: 'INCREMENT' }); - expect(fixture.getLiftedState().skippedActionIds.indexOf(1)).not.toBe(-1); - fixture.store.dispatch({ type: 'INCREMENT' }); - expect(fixture.getLiftedState().skippedActionIds.indexOf(1)).toBe(-1); - - fixture.cleanup(); - }); - - it('should not auto-commit errors', () => { - spyOn(console, 'error'); - const fixture = createStore(counterWithBug, { maxAge: 3 }); - - fixture.store.dispatch({ type: 'DECREMENT' }); - fixture.store.dispatch({ type: 'INCREMENT' }); - expect(fixture.getLiftedState().stagedActionIds.length).toBe(3); - - fixture.store.dispatch({ type: 'INCREMENT' }); - expect(fixture.getLiftedState().stagedActionIds.length).toBe(4); - - fixture.cleanup(); - }); - - it('should auto-commit actions after hot reload fixes error', () => { - spyOn(console, 'error'); - const fixture = createStore(counterWithBug, { maxAge: 3 }); - - fixture.store.dispatch({ type: 'DECREMENT' }); - fixture.store.dispatch({ type: 'DECREMENT' }); - fixture.store.dispatch({ type: 'INCREMENT' }); - fixture.store.dispatch({ type: 'DECREMENT' }); - fixture.store.dispatch({ type: 'DECREMENT' }); - fixture.store.dispatch({ type: 'DECREMENT' }); - expect(fixture.getLiftedState().stagedActionIds.length).toBe(7); - - // Auto-commit 2 actions by "fixing" reducer bug, but introducing another. - fixture.replaceReducer(counterWithAnotherBug); - expect(fixture.getLiftedState().stagedActionIds.length).toBe(5); - - // Auto-commit 2 more actions by "fixing" other reducer bug. - fixture.replaceReducer(counter); - expect(fixture.getLiftedState().stagedActionIds.length).toBe(3); - - fixture.cleanup(); - }); - - it('should update currentStateIndex when auto-committing', () => { - const fixture = createStore(counter, { maxAge: 3 }); - - fixture.store.dispatch({ type: 'INCREMENT' }); - fixture.store.dispatch({ type: 'INCREMENT' }); - - expect(fixture.getLiftedState().currentStateIndex).toBe(2); - - // currentStateIndex should stay at 2 as actions are committed. - fixture.store.dispatch({ type: 'INCREMENT' }); - const liftedStoreState = fixture.getLiftedState(); - const currentComputedState = - liftedStoreState.computedStates[liftedStoreState.currentStateIndex]; - - expect(liftedStoreState.currentStateIndex).toBe(2); - expect(currentComputedState.state).toEqual({ state: 3 }); - - fixture.cleanup(); - }); - - it('should continue to increment currentStateIndex while error blocks commit', () => { - spyOn(console, 'error'); - const fixture = createStore(counterWithBug, { maxAge: 3 }); - - fixture.store.dispatch({ type: 'DECREMENT' }); - fixture.store.dispatch({ type: 'DECREMENT' }); - fixture.store.dispatch({ type: 'DECREMENT' }); - fixture.store.dispatch({ type: 'DECREMENT' }); - - let liftedStoreState = fixture.getLiftedState(); - let currentComputedState = - liftedStoreState.computedStates[liftedStoreState.currentStateIndex]; - expect(liftedStoreState.currentStateIndex).toBe(4); - expect(currentComputedState.state).toEqual({ state: 0 }); - expect(currentComputedState.error).toBeDefined(); - - fixture.cleanup(); - }); - - it('should adjust currentStateIndex correctly when multiple actions are committed', () => { - spyOn(console, 'error'); - const fixture = createStore(counterWithBug, { maxAge: 3 }); - - fixture.store.dispatch({ type: 'DECREMENT' }); - fixture.store.dispatch({ type: 'DECREMENT' }); - fixture.store.dispatch({ type: 'DECREMENT' }); - fixture.store.dispatch({ type: 'DECREMENT' }); - - // Auto-commit 2 actions by "fixing" reducer bug. - fixture.replaceReducer(counter); - let liftedStoreState = fixture.getLiftedState(); - let currentComputedState = - liftedStoreState.computedStates[liftedStoreState.currentStateIndex]; - expect(liftedStoreState.currentStateIndex).toBe(2); - expect(currentComputedState.state).toEqual({ state: -4 }); - - fixture.cleanup(); - }); - - it('should not allow currentStateIndex to drop below 0', () => { - spyOn(console, 'error'); - const fixture = createStore(counterWithBug, { maxAge: 3 }); - - fixture.store.dispatch({ type: 'DECREMENT' }); - fixture.store.dispatch({ type: 'DECREMENT' }); - fixture.store.dispatch({ type: 'DECREMENT' }); - fixture.store.dispatch({ type: 'DECREMENT' }); - fixture.devtools.jumpToState(1); - - // Auto-commit 2 actions by "fixing" reducer bug. - fixture.replaceReducer(counter); - let liftedStoreState = fixture.getLiftedState(); - let currentComputedState = - liftedStoreState.computedStates[liftedStoreState.currentStateIndex]; - expect(liftedStoreState.currentStateIndex).toBe(0); - expect(currentComputedState.state).toEqual({ state: -2 }); - - fixture.cleanup(); - }); - - it('should throw error when maxAge < 2', () => { - expect(() => { - createStore(counter, { maxAge: 1 }); - }).toThrowError(/cannot be less than/); - }); - - it('should support a function to return devtools options', () => { - expect(() => { - createStore(counter, function() { - return { maxAge: 1 }; - }); - }).toThrowError(/cannot be less than/); - }); - }); - - describe('Import State', () => { - let fixture: Fixture; - let exportedState: LiftedState; - - beforeEach(() => { - fixture = createStore(counter); - - fixture.store.dispatch({ type: 'INCREMENT' }); - fixture.store.dispatch({ type: 'INCREMENT' }); - fixture.store.dispatch({ type: 'INCREMENT' }); - - exportedState = fixture.getLiftedState(); - }); - - afterEach(() => { - fixture.cleanup(); - }); - - it('should replay all the steps when a state is imported', () => { - fixture.devtools.importState(exportedState); - expect(fixture.getLiftedState()).toEqual(exportedState); - }); - - it('should replace the existing action log with the one imported', () => { - fixture.store.dispatch({ type: 'DECREMENT' }); - fixture.store.dispatch({ type: 'DECREMENT' }); - - fixture.devtools.importState(exportedState); - expect(fixture.getLiftedState()).toEqual(exportedState); - }); - }); - - describe('Action and State Sanitizer', () => { - let fixture: Fixture; - - const SANITIZED_TOKEN = 'SANITIZED_ACTION'; - const SANITIZED_COUNTER = 42; - const testActionSanitizer = (action: Action, id: number) => { - return { type: SANITIZED_TOKEN }; - }; - const incrementActionSanitizer = (action: Action, id: number) => { - return { type: 'INCREMENT' }; - }; - const testStateSanitizer = (state: any, index: number) => { - return { state: SANITIZED_COUNTER }; - }; - - afterEach(() => { - fixture.cleanup(); - }); - - it('should function normally with no sanitizers', () => { - fixture = createStore(counter); - - fixture.store.dispatch({ type: 'INCREMENT' }); - - const liftedState = fixture.getLiftedState(); - const currentLiftedState = - liftedState.computedStates[liftedState.currentStateIndex]; - expect(Object.keys(liftedState.actionsById).length).toBe( - Object.keys(liftedState.sanitizedActionsById).length - ); - expect(liftedState.actionsById).toEqual(liftedState.sanitizedActionsById); - expect(currentLiftedState.state).toEqual({ state: 1 }); - expect(currentLiftedState.sanitizedState).toBeUndefined(); - }); - - it('should run the action sanitizer on actions', () => { - fixture = createStore(counter, { - actionSanitizer: testActionSanitizer, - }); - - fixture.store.dispatch({ type: 'INCREMENT' }); - fixture.store.dispatch({ type: 'DECREMENT' }); - - const liftedState = fixture.getLiftedState(); - const sanitizedAction = - liftedState.sanitizedActionsById[liftedState.nextActionId - 1]; - const sanitizedAction2 = - liftedState.sanitizedActionsById[liftedState.nextActionId - 2]; - const action = liftedState.actionsById[liftedState.nextActionId - 1]; - const action2 = liftedState.actionsById[liftedState.nextActionId - 2]; - - expect(liftedState.actionsById).not.toEqual( - liftedState.sanitizedActionsById - ); - expect(sanitizedAction.action).toEqual({ type: SANITIZED_TOKEN }); - expect(sanitizedAction2.action).toEqual({ type: SANITIZED_TOKEN }); - expect(action.action).toEqual({ type: 'DECREMENT' }); - expect(action2.action).toEqual({ type: 'INCREMENT' }); - }); - - it('should run the state sanitizer on store state', () => { - fixture = createStore(counter, { - stateSanitizer: testStateSanitizer, - }); - - let liftedState = fixture.getLiftedState(); - let currentLiftedState = - liftedState.computedStates[liftedState.currentStateIndex]; - expect(fixture.getState()).toBe(0); - expect(currentLiftedState.state).toEqual({ state: 0 }); - expect(currentLiftedState.sanitizedState).toBeDefined(); - expect(currentLiftedState.sanitizedState).toEqual({ - state: SANITIZED_COUNTER, - }); - - fixture.store.dispatch({ type: 'INCREMENT' }); - - liftedState = fixture.getLiftedState(); - currentLiftedState = - liftedState.computedStates[liftedState.currentStateIndex]; - expect(fixture.getState()).toBe(1); - expect(currentLiftedState.state).toEqual({ state: 1 }); - expect(currentLiftedState.sanitizedState).toEqual({ - state: SANITIZED_COUNTER, - }); - }); - - it('should run transparently to produce a new lifted store state', () => { - const devtoolsOptions: Partial = { - actionSanitizer: testActionSanitizer, - stateSanitizer: testStateSanitizer, - }; - fixture = createStore(counter, devtoolsOptions); - - fixture.store.dispatch({ type: 'INCREMENT' }); - - const liftedState = fixture.getLiftedState(); - const sanitizedLiftedState = fixture.devtools.getSanitizedState( - liftedState, - devtoolsOptions.stateSanitizer - ); - const originalAction = - liftedState.actionsById[liftedState.nextActionId - 1]; - const originalState = - liftedState.computedStates[liftedState.currentStateIndex]; - const sanitizedAction = - sanitizedLiftedState.actionsById[liftedState.nextActionId - 1]; - const sanitizedState = - sanitizedLiftedState.computedStates[liftedState.currentStateIndex]; - - expect(originalAction.action).toEqual({ type: 'INCREMENT' }); - expect(originalState.state).toEqual({ state: 1 }); - expect(sanitizedAction.action).toEqual({ type: SANITIZED_TOKEN }); - expect(sanitizedState.state).toEqual({ state: SANITIZED_COUNTER }); - }); - - it('sanitized actions should not affect the store state', () => { - fixture = createStore(counter, { - actionSanitizer: incrementActionSanitizer, - }); - - fixture.store.dispatch({ type: 'DECREMENT' }); - fixture.store.dispatch({ type: 'DECREMENT' }); - - const liftedState = fixture.getLiftedState(); - expect(fixture.getState()).toBe(-2); - expect( - liftedState.computedStates[liftedState.currentStateIndex].state - ).toEqual({ state: -2 }); - }); - }); -}); +import 'rxjs/add/operator/take'; +import { Subscription } from 'rxjs/Subscription'; +import { ReflectiveInjector } from '@angular/core'; +import { TestBed, getTestBed } from '@angular/core/testing'; +import { + StoreModule, + Store, + StateObservable, + ActionReducer, + Action, + ReducerManager, +} from '@ngrx/store'; +import { + StoreDevtools, + StoreDevtoolsModule, + LiftedState, + StoreDevtoolsConfig, + StoreDevtoolsOptions, +} from '../'; +import { IS_EXTENSION_OR_MONITOR_PRESENT } from '../src/instrument'; + +const counter = jasmine + .createSpy('counter') + .and.callFake(function(state = 0, action: Action) { + switch (action.type) { + case 'INCREMENT': + return state + 1; + case 'DECREMENT': + return state - 1; + default: + return state; + } + }); + +declare var mistake: any; +function counterWithBug(state = 0, action: Action) { + switch (action.type) { + case 'INCREMENT': + return state + 1; + case 'DECREMENT': + return mistake - 1; // mistake is undefined + case 'SET_UNDEFINED': + return undefined; + default: + return state; + } +} + +function counterWithAnotherBug(state = 0, action: Action) { + switch (action.type) { + case 'INCREMENT': + return mistake + 1; // eslint-disable-line no-undef + case 'DECREMENT': + return state - 1; + case 'SET_UNDEFINED': + return undefined; + default: + return state; + } +} + +function doubleCounter(state = 0, action: Action) { + switch (action.type) { + case 'INCREMENT': + return state + 2; + case 'DECREMENT': + return state - 2; + default: + return state; + } +} + +type Fixture = { + store: Store; + state: StateObservable; + devtools: StoreDevtools; + cleanup: () => void; + getState: () => T; + getLiftedState: () => LiftedState; + replaceReducer: (reducer: ActionReducer) => void; +}; + +function createStore( + reducer: ActionReducer, + options: StoreDevtoolsOptions = {} +): Fixture { + TestBed.configureTestingModule({ + imports: [ + StoreModule.forRoot({ state: reducer }), + StoreDevtoolsModule.instrument(options), + ], + providers: [{ provide: IS_EXTENSION_OR_MONITOR_PRESENT, useValue: true }], + }); + + const testbed: TestBed = getTestBed(); + const store: Store = testbed.get(Store); + const devtools: StoreDevtools = testbed.get(StoreDevtools); + const state: StateObservable = testbed.get(StateObservable); + const reducerManager: ReducerManager = testbed.get(ReducerManager); + let liftedValue: LiftedState; + let value: any; + + const liftedStateSub = devtools.liftedState.subscribe(s => (liftedValue = s)); + const stateSub = devtools.state.subscribe(s => (value = s)); + + const getState = (): T => value.state; + const getLiftedState = (): LiftedState => liftedValue; + + const cleanup = () => { + liftedStateSub.unsubscribe(); + stateSub.unsubscribe(); + }; + + const replaceReducer = (reducer: ActionReducer) => { + reducerManager.addReducer('state', reducer); + }; + + return { + store, + state, + devtools, + cleanup, + getState, + getLiftedState, + replaceReducer, + }; +} + +describe('Store Devtools', () => { + describe('Instrumentation', () => { + let fixture: Fixture; + let store: Store; + let devtools: StoreDevtools; + let getState: () => number; + let getLiftedState: () => LiftedState; + + beforeEach(() => { + fixture = createStore(counter); + store = fixture.store; + devtools = fixture.devtools; + getState = fixture.getState; + getLiftedState = fixture.getLiftedState; + }); + + afterEach(() => { + fixture.cleanup(); + }); + + it("should alias devtools unlifted state to Store's state", () => { + expect(devtools.state).toBe(fixture.state as any); + }); + + it('should perform actions', () => { + expect(getState()).toBe(0); + store.dispatch({ type: 'INCREMENT' }); + expect(getState()).toBe(1); + store.dispatch({ type: 'INCREMENT' }); + expect(getState()).toBe(2); + }); + + it('should rollback state to the last committed state', () => { + store.dispatch({ type: 'INCREMENT' }); + store.dispatch({ type: 'INCREMENT' }); + expect(getState()).toBe(2); + + devtools.commit(); + expect(getState()).toBe(2); + + store.dispatch({ type: 'INCREMENT' }); + store.dispatch({ type: 'INCREMENT' }); + expect(getState()).toBe(4); + + devtools.rollback(); + expect(getState()).toBe(2); + + store.dispatch({ type: 'DECREMENT' }); + expect(getState()).toBe(1); + + devtools.rollback(); + expect(getState()).toBe(2); + }); + + it('should reset to initial state', () => { + store.dispatch({ type: 'INCREMENT' }); + expect(getState()).toBe(1); + + devtools.commit(); + expect(getState()).toBe(1); + + store.dispatch({ type: 'INCREMENT' }); + expect(getState()).toBe(2); + + devtools.rollback(); + expect(getState()).toBe(1); + + store.dispatch({ type: 'INCREMENT' }); + expect(getState()).toBe(2); + + devtools.reset(); + expect(getState()).toBe(0); + }); + + it('should toggle an action', () => { + // actionId 0 = @@INIT + store.dispatch({ type: 'INCREMENT' }); + store.dispatch({ type: 'DECREMENT' }); + store.dispatch({ type: 'INCREMENT' }); + expect(getState()).toBe(1); + + devtools.toggleAction(2); + expect(getState()).toBe(2); + + devtools.toggleAction(2); + expect(getState()).toBe(1); + }); + + it('should sweep disabled actions', () => { + // actionId 0 = @@INIT + store.dispatch({ type: 'INCREMENT' }); + store.dispatch({ type: 'DECREMENT' }); + store.dispatch({ type: 'INCREMENT' }); + store.dispatch({ type: 'INCREMENT' }); + + expect(getState()).toBe(2); + expect(getLiftedState().stagedActionIds).toEqual([0, 1, 2, 3, 4]); + expect(getLiftedState().skippedActionIds).toEqual([]); + + devtools.toggleAction(2); + + expect(getState()).toBe(3); + expect(getLiftedState().stagedActionIds).toEqual([0, 1, 2, 3, 4]); + expect(getLiftedState().skippedActionIds).toEqual([2]); + + devtools.sweep(); + expect(getState()).toBe(3); + expect(getLiftedState().stagedActionIds).toEqual([0, 1, 3, 4]); + expect(getLiftedState().skippedActionIds).toEqual([]); + }); + + it('should jump to state', () => { + store.dispatch({ type: 'INCREMENT' }); + store.dispatch({ type: 'DECREMENT' }); + store.dispatch({ type: 'INCREMENT' }); + expect(getState()).toBe(1); + + devtools.jumpToState(0); + expect(getState()).toBe(0); + + devtools.jumpToState(1); + expect(getState()).toBe(1); + + devtools.jumpToState(2); + expect(getState()).toBe(0); + + store.dispatch({ type: 'INCREMENT' }); + expect(getState()).toBe(0); + + devtools.jumpToState(4); + expect(getState()).toBe(2); + }); + + it('should jump to action', () => { + store.dispatch({ type: 'INCREMENT' }); + store.dispatch({ type: 'DECREMENT' }); + store.dispatch({ type: 'INCREMENT' }); + expect(getState()).toBe(1); + + devtools.jumpToAction(2); + expect(getState()).toBe(0); + }); + + it('should replace the reducer and preserve previous states', () => { + store.dispatch({ type: 'INCREMENT' }); + store.dispatch({ type: 'DECREMENT' }); + store.dispatch({ type: 'INCREMENT' }); + + expect(getState()).toBe(1); + + fixture.replaceReducer(doubleCounter); + + expect(getState()).toBe(1); + }); + + it('should replace the reducer and compute new state with latest reducer', () => { + store.dispatch({ type: 'INCREMENT' }); + store.dispatch({ type: 'DECREMENT' }); + store.dispatch({ type: 'INCREMENT' }); + + expect(getState()).toBe(1); + + fixture.replaceReducer(doubleCounter); + + store.dispatch({ type: 'INCREMENT' }); + + expect(getState()).toBe(3); + }); + + it('should catch and record errors', () => { + spyOn(console, 'error'); + fixture.replaceReducer(counterWithBug); + + store.dispatch({ type: 'INCREMENT' }); + store.dispatch({ type: 'DECREMENT' }); + store.dispatch({ type: 'INCREMENT' }); + + let { computedStates } = fixture.getLiftedState(); + expect(computedStates[3].error).toMatch(/ReferenceError/); + expect(computedStates[4].error).toMatch( + /Interrupted by an error up the chain/ + ); + + expect(console.error).toHaveBeenCalled(); + }); + + it('should catch invalid action type', () => { + expect(() => { + store.dispatch({ type: undefined } as any); + }).toThrowError('Actions must have a type property'); + }); + + it('should not recompute old states when toggling an action', () => { + counter.calls.reset(); + + store.dispatch({ type: 'INCREMENT' }); + store.dispatch({ type: 'INCREMENT' }); + store.dispatch({ type: 'INCREMENT' }); + + expect(counter).toHaveBeenCalledTimes(3); + + devtools.toggleAction(3); + expect(counter).toHaveBeenCalledTimes(3); + + devtools.toggleAction(3); + expect(counter).toHaveBeenCalledTimes(4); + + devtools.toggleAction(2); + expect(counter).toHaveBeenCalledTimes(5); + + devtools.toggleAction(2); + expect(counter).toHaveBeenCalledTimes(7); + + devtools.toggleAction(1); + expect(counter).toHaveBeenCalledTimes(9); + + devtools.toggleAction(2); + expect(counter).toHaveBeenCalledTimes(10); + + devtools.toggleAction(3); + expect(counter).toHaveBeenCalledTimes(10); + + devtools.toggleAction(1); + expect(counter).toHaveBeenCalledTimes(11); + + devtools.toggleAction(3); + expect(counter).toHaveBeenCalledTimes(12); + + devtools.toggleAction(2); + expect(counter).toHaveBeenCalledTimes(14); + }); + + it('should not recompute states when jumping to state', () => { + counter.calls.reset(); + + store.dispatch({ type: 'INCREMENT' }); + store.dispatch({ type: 'INCREMENT' }); + store.dispatch({ type: 'INCREMENT' }); + + expect(counter).toHaveBeenCalledTimes(3); + + let savedComputedStates = getLiftedState().computedStates; + + devtools.jumpToState(0); + + expect(counter).toHaveBeenCalledTimes(3); + + devtools.jumpToState(1); + + expect(counter).toHaveBeenCalledTimes(3); + + devtools.jumpToState(3); + + expect(counter).toHaveBeenCalledTimes(3); + + expect(getLiftedState().computedStates).toBe(savedComputedStates); + }); + + it('should not recompute states on monitor actions', () => { + counter.calls.reset(); + + store.dispatch({ type: 'INCREMENT' }); + store.dispatch({ type: 'INCREMENT' }); + store.dispatch({ type: 'INCREMENT' }); + + expect(counter).toHaveBeenCalledTimes(3); + + let savedComputedStates = getLiftedState().computedStates; + + devtools.dispatch({ type: 'lol' }); + + expect(counter).toHaveBeenCalledTimes(3); + + devtools.dispatch({ type: 'wat' }); + + expect(counter).toHaveBeenCalledTimes(3); + + expect(getLiftedState().computedStates).toBe(savedComputedStates); + }); + }); + + describe('maxAge option', () => { + it('should auto-commit earliest non-@@INIT action when maxAge is reached', () => { + const fixture = createStore(counter, { maxAge: 3 }); + + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'INCREMENT' }); + let liftedStoreState = fixture.getLiftedState(); + + expect(fixture.getState()).toBe(2); + expect(Object.keys(liftedStoreState.actionsById).length).toBe(3); + expect(liftedStoreState.committedState).toBe(undefined); + expect(liftedStoreState.stagedActionIds).toContain(1); + + // Trigger auto-commit. + fixture.store.dispatch({ type: 'INCREMENT' }); + liftedStoreState = fixture.getLiftedState(); + + expect(fixture.getState()).toBe(3); + expect(Object.keys(liftedStoreState.actionsById).length).toBe(3); + expect(liftedStoreState.stagedActionIds).not.toContain(1); + expect(liftedStoreState.computedStates[0].state).toEqual({ state: 1 }); + expect(liftedStoreState.committedState).toEqual({ state: 1 }); + expect(liftedStoreState.currentStateIndex).toBe(2); + + fixture.cleanup(); + }); + + it('should remove skipped actions once committed', () => { + const fixture = createStore(counter, { maxAge: 3 }); + + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.devtools.toggleAction(1); + fixture.store.dispatch({ type: 'INCREMENT' }); + expect(fixture.getLiftedState().skippedActionIds.indexOf(1)).not.toBe(-1); + fixture.store.dispatch({ type: 'INCREMENT' }); + expect(fixture.getLiftedState().skippedActionIds.indexOf(1)).toBe(-1); + + fixture.cleanup(); + }); + + it('should not auto-commit errors', () => { + spyOn(console, 'error'); + const fixture = createStore(counterWithBug, { maxAge: 3 }); + + fixture.store.dispatch({ type: 'DECREMENT' }); + fixture.store.dispatch({ type: 'INCREMENT' }); + expect(fixture.getLiftedState().stagedActionIds.length).toBe(3); + + fixture.store.dispatch({ type: 'INCREMENT' }); + expect(fixture.getLiftedState().stagedActionIds.length).toBe(4); + + fixture.cleanup(); + }); + + it('should auto-commit actions after hot reload fixes error', () => { + spyOn(console, 'error'); + const fixture = createStore(counterWithBug, { maxAge: 3 }); + + fixture.store.dispatch({ type: 'DECREMENT' }); + fixture.store.dispatch({ type: 'DECREMENT' }); + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'DECREMENT' }); + fixture.store.dispatch({ type: 'DECREMENT' }); + fixture.store.dispatch({ type: 'DECREMENT' }); + expect(fixture.getLiftedState().stagedActionIds.length).toBe(7); + + // Auto-commit 2 actions by "fixing" reducer bug, but introducing another. + fixture.replaceReducer(counterWithAnotherBug); + expect(fixture.getLiftedState().stagedActionIds.length).toBe(5); + + // Auto-commit 2 more actions by "fixing" other reducer bug. + fixture.replaceReducer(counter); + expect(fixture.getLiftedState().stagedActionIds.length).toBe(3); + + fixture.cleanup(); + }); + + it('should update currentStateIndex when auto-committing', () => { + const fixture = createStore(counter, { maxAge: 3 }); + + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'INCREMENT' }); + + expect(fixture.getLiftedState().currentStateIndex).toBe(2); + + // currentStateIndex should stay at 2 as actions are committed. + fixture.store.dispatch({ type: 'INCREMENT' }); + const liftedStoreState = fixture.getLiftedState(); + const currentComputedState = + liftedStoreState.computedStates[liftedStoreState.currentStateIndex]; + + expect(liftedStoreState.currentStateIndex).toBe(2); + expect(currentComputedState.state).toEqual({ state: 3 }); + + fixture.cleanup(); + }); + + it('should continue to increment currentStateIndex while error blocks commit', () => { + spyOn(console, 'error'); + const fixture = createStore(counterWithBug, { maxAge: 3 }); + + fixture.store.dispatch({ type: 'DECREMENT' }); + fixture.store.dispatch({ type: 'DECREMENT' }); + fixture.store.dispatch({ type: 'DECREMENT' }); + fixture.store.dispatch({ type: 'DECREMENT' }); + + let liftedStoreState = fixture.getLiftedState(); + let currentComputedState = + liftedStoreState.computedStates[liftedStoreState.currentStateIndex]; + expect(liftedStoreState.currentStateIndex).toBe(4); + expect(currentComputedState.state).toEqual({ state: 0 }); + expect(currentComputedState.error).toBeDefined(); + + fixture.cleanup(); + }); + + it('should adjust currentStateIndex correctly when multiple actions are committed', () => { + spyOn(console, 'error'); + const fixture = createStore(counterWithBug, { maxAge: 3 }); + + fixture.store.dispatch({ type: 'DECREMENT' }); + fixture.store.dispatch({ type: 'DECREMENT' }); + fixture.store.dispatch({ type: 'DECREMENT' }); + fixture.store.dispatch({ type: 'DECREMENT' }); + + // Auto-commit 2 actions by "fixing" reducer bug. + fixture.replaceReducer(counter); + let liftedStoreState = fixture.getLiftedState(); + let currentComputedState = + liftedStoreState.computedStates[liftedStoreState.currentStateIndex]; + expect(liftedStoreState.currentStateIndex).toBe(2); + expect(currentComputedState.state).toEqual({ state: -4 }); + + fixture.cleanup(); + }); + + it('should not allow currentStateIndex to drop below 0', () => { + spyOn(console, 'error'); + const fixture = createStore(counterWithBug, { maxAge: 3 }); + + fixture.store.dispatch({ type: 'DECREMENT' }); + fixture.store.dispatch({ type: 'DECREMENT' }); + fixture.store.dispatch({ type: 'DECREMENT' }); + fixture.store.dispatch({ type: 'DECREMENT' }); + fixture.devtools.jumpToState(1); + + // Auto-commit 2 actions by "fixing" reducer bug. + fixture.replaceReducer(counter); + let liftedStoreState = fixture.getLiftedState(); + let currentComputedState = + liftedStoreState.computedStates[liftedStoreState.currentStateIndex]; + expect(liftedStoreState.currentStateIndex).toBe(0); + expect(currentComputedState.state).toEqual({ state: -2 }); + + fixture.cleanup(); + }); + + it('should throw error when maxAge < 2', () => { + expect(() => { + createStore(counter, { maxAge: 1 }); + }).toThrowError(/cannot be less than/); + }); + + it('should support a function to return devtools options', () => { + expect(() => { + createStore(counter, function() { + return { maxAge: 1 }; + }); + }).toThrowError(/cannot be less than/); + }); + }); + + describe('Import State', () => { + let fixture: Fixture; + let exportedState: LiftedState; + + beforeEach(() => { + fixture = createStore(counter); + + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'INCREMENT' }); + + exportedState = fixture.getLiftedState(); + }); + + afterEach(() => { + fixture.cleanup(); + }); + + it('should replay all the steps when a state is imported', () => { + fixture.devtools.importState(exportedState); + expect(fixture.getLiftedState()).toEqual(exportedState); + }); + + it('should replace the existing action log with the one imported', () => { + fixture.store.dispatch({ type: 'DECREMENT' }); + fixture.store.dispatch({ type: 'DECREMENT' }); + + fixture.devtools.importState(exportedState); + expect(fixture.getLiftedState()).toEqual(exportedState); + }); + }); +}); diff --git a/modules/store-devtools/src/devtools.ts b/modules/store-devtools/src/devtools.ts index 11332f2e01..b3f8f60324 100644 --- a/modules/store-devtools/src/devtools.ts +++ b/modules/store-devtools/src/devtools.ts @@ -32,6 +32,7 @@ import { StoreDevtoolsConfig, STORE_DEVTOOLS_CONFIG, StateSanitizer, + ActionSanitizer, } from './config'; @Injectable() @@ -80,10 +81,7 @@ export class StoreDevtools implements Observer { const reducedLiftedState = reducer(liftedState, action); // Extension should be sent the sanitized lifted state - extension.notify( - action, - this.getSanitizedState(reducedLiftedState, config.stateSanitizer) - ); + extension.notify(action, reducedLiftedState); return { state: reducedLiftedState, action }; }, @@ -110,26 +108,6 @@ export class StoreDevtools implements Observer { this.state = state$; } - /** - * Restructures the lifted state passed in to prepare for sending to the - * Redux Devtools Extension - */ - getSanitizedState(state: LiftedState, stateSanitizer?: StateSanitizer) { - const sanitizedComputedStates = stateSanitizer - ? state.computedStates.map((entry: ComputedState) => ({ - state: entry.sanitizedState, - error: entry.error, - })) - : state.computedStates; - - // Replace action and state logs with their sanitized versions - return { - ...state, - actionsById: state.sanitizedActionsById, - computedStates: sanitizedComputedStates, - }; - } - dispatch(action: Action) { this.dispatcher.next(action); } diff --git a/modules/store-devtools/src/extension.ts b/modules/store-devtools/src/extension.ts index 7d904e1b74..0a60992f7f 100644 --- a/modules/store-devtools/src/extension.ts +++ b/modules/store-devtools/src/extension.ts @@ -8,10 +8,26 @@ import { share } from 'rxjs/operator/share'; import { switchMap } from 'rxjs/operator/switchMap'; import { takeUntil } from 'rxjs/operator/takeUntil'; -import { STORE_DEVTOOLS_CONFIG, StoreDevtoolsConfig } from './config'; -import { LiftedState } from './reducer'; -import { PerformAction } from './actions'; -import { applyOperators, unliftState } from './utils'; +import { + STORE_DEVTOOLS_CONFIG, + StoreDevtoolsConfig, + StateSanitizer, +} from './config'; +import { + LiftedState, + LiftedActions, + ComputedState, + LiftedAction, +} from './reducer'; +import { PerformAction, PERFORM_ACTION } from './actions'; +import { + applyOperators, + unliftState, + sanitizeState, + sanitizeAction, + sanitizeActions, + sanitizeStates, +} from './utils'; export const ExtensionActionTypes = { START: 'START', @@ -36,7 +52,7 @@ export interface ReduxDevtoolsExtensionConfig { name: string | undefined; instanceId: string; maxAge?: number; - actionSanitizer?: (action: Action, id: number) => Action; + serialize?: boolean; } export interface ReduxDevtoolsExtension { @@ -46,7 +62,7 @@ export interface ReduxDevtoolsExtension { send( action: any, state: any, - options: StoreDevtoolsConfig, + options: ReduxDevtoolsExtensionConfig, instanceId?: string ): void; } @@ -68,7 +84,7 @@ export class DevtoolsExtension { this.createActionStreams(); } - notify(action: Action, state: LiftedState) { + notify(action: LiftedAction, state: LiftedState) { if (!this.devtoolsExtension) { return; } @@ -86,12 +102,40 @@ export class DevtoolsExtension { // c) the state has been recomputed due to time-traveling // d) any action that is not a PerformAction to err on the side of // caution. - if (action instanceof PerformAction) { + if (action.type === PERFORM_ACTION) { const currentState = unliftState(state); - this.extensionConnection.send(action.action, currentState); + const sanitizedState = this.config.stateSanitizer + ? sanitizeState( + this.config.stateSanitizer, + currentState, + state.currentStateIndex + ) + : currentState; + const sanitizedAction = this.config.actionSanitizer + ? sanitizeAction( + this.config.actionSanitizer, + action, + state.nextActionId + ) + : action; + this.extensionConnection.send(sanitizedAction, sanitizedState); } else { - // Requires full state update; - this.devtoolsExtension.send(null, state, this.config, this.instanceId); + // Requires full state update + const sanitizedLiftedState = { + ...state, + actionsById: this.config.actionSanitizer + ? sanitizeActions(this.config.actionSanitizer, state.actionsById) + : state.actionsById, + computedStates: this.config.stateSanitizer + ? sanitizeStates(this.config.stateSanitizer, state.computedStates) + : state.computedStates, + }; + this.devtoolsExtension.send( + null, + sanitizedLiftedState, + this.getExtensionConfig(this.instanceId, this.config), + this.instanceId + ); } } @@ -101,16 +145,9 @@ export class DevtoolsExtension { } return new Observable(subscriber => { - let extensionOptions: ReduxDevtoolsExtensionConfig = { - instanceId: this.instanceId, - name: this.config.name, - features: this.config.features, - actionSanitizer: this.config.actionSanitizer, - }; - if (this.config.maxAge !== false /* support === 0 */) { - extensionOptions.maxAge = this.config.maxAge; - } - const connection = this.devtoolsExtension.connect(extensionOptions); + const connection = this.devtoolsExtension.connect( + this.getExtensionConfig(this.instanceId, this.config) + ); this.extensionConnection = connection; connection.init(); @@ -158,4 +195,24 @@ export class DevtoolsExtension { private unwrapAction(action: Action) { return typeof action === 'string' ? eval(`(${action})`) : action; } + + private getExtensionConfig(instanceId: string, config: StoreDevtoolsConfig) { + const extensionOptions: ReduxDevtoolsExtensionConfig = { + instanceId: instanceId, + name: config.name, + features: config.features, + serialize: config.serialize, + // The action/state sanitizers are not added to the config + // because sanitation is done in this class already. + // It is done before sending it to the devtools extension for consistency: + // - If we call extensionConnection.send(...), + // the extension would call the sanitizers. + // - If we call devtoolsExtension.send(...) (aka full state update), + // the extension would NOT call the sanitizers, so we have to do it ourselves. + }; + if (config.maxAge !== false /* support === 0 */) { + extensionOptions.maxAge = config.maxAge; + } + return extensionOptions; + } } diff --git a/modules/store-devtools/src/reducer.ts b/modules/store-devtools/src/reducer.ts index 50a7f3b7d1..994d54838d 100644 --- a/modules/store-devtools/src/reducer.ts +++ b/modules/store-devtools/src/reducer.ts @@ -26,15 +26,22 @@ export const INIT_ACTION = { type: INIT }; export interface ComputedState { state: any; - sanitizedState?: any; error: any; } +export interface LiftedAction { + type: string; + action: Action; +} + +export interface LiftedActions { + [id: number]: LiftedAction; +} + export interface LiftedState { monitorState: any; nextActionId: number; - actionsById: { [id: number]: { action: Action } }; - sanitizedActionsById: { [id: number]: { action: Action } }; + actionsById: LiftedActions; stagedActionIds: number[]; skippedActionIds: number[]; committedState: any; @@ -81,10 +88,9 @@ function recomputeStates( minInvalidatedStateIndex: number, reducer: ActionReducer, committedState: any, - actionsById: { [id: number]: { action: Action } }, + actionsById: LiftedActions, stagedActionIds: number[], - skippedActionIds: number[], - stateSanitizer?: StateSanitizer + skippedActionIds: number[] ) { // Optimization: exit early and return the same reference // if we know nothing could have changed. @@ -109,15 +115,7 @@ function recomputeStates( ? previousEntry : computeNextEntry(reducer, action, previousState, previousError); - if (stateSanitizer) { - const sanitizedEntry = { - ...entry, - sanitizedState: stateSanitizer(entry.state, actionId), - }; - nextComputedStates.push(sanitizedEntry); - } else { - nextComputedStates.push(entry); - } + nextComputedStates.push(entry); } return nextComputedStates; @@ -131,7 +129,6 @@ export function liftInitialState( monitorState: monitorReducer(undefined, {}), nextActionId: 1, actionsById: { 0: liftAction(INIT_ACTION) }, - sanitizedActionsById: { 0: liftAction(INIT_ACTION) }, stagedActionIds: [0], skippedActionIds: [], committedState: initialCommittedState, @@ -158,7 +155,6 @@ export function liftReducerWith( let { monitorState, actionsById, - sanitizedActionsById, nextActionId, stagedActionIds, skippedActionIds, @@ -171,7 +167,6 @@ export function liftReducerWith( if (!liftedState) { // Prevent mutating initialLiftedState actionsById = Object.create(actionsById); - sanitizedActionsById = Object.create(sanitizedActionsById); } function commitExcessActions(n: number) { @@ -187,7 +182,6 @@ export function liftReducerWith( break; } else { delete actionsById[idsToDelete[i]]; - delete sanitizedActionsById[idsToDelete[i]]; } } @@ -210,7 +204,6 @@ export function liftReducerWith( case Actions.RESET: { // Get back to the state the store was created with. actionsById = { 0: liftAction(INIT_ACTION) }; - sanitizedActionsById = { 0: liftAction(INIT_ACTION) }; nextActionId = 1; stagedActionIds = [0]; skippedActionIds = []; @@ -223,7 +216,6 @@ export function liftReducerWith( // Consider the last committed state the new starting point. // Squash any staged actions into a single committed state. actionsById = { 0: liftAction(INIT_ACTION) }; - sanitizedActionsById = { 0: liftAction(INIT_ACTION) }; nextActionId = 1; stagedActionIds = [0]; skippedActionIds = []; @@ -236,7 +228,6 @@ export function liftReducerWith( // Forget about any staged actions. // Start again from the last committed state. actionsById = { 0: liftAction(INIT_ACTION) }; - sanitizedActionsById = { 0: liftAction(INIT_ACTION) }; nextActionId = 1; stagedActionIds = [0]; skippedActionIds = []; @@ -313,9 +304,6 @@ export function liftReducerWith( // Mutation! This is the hottest path, and we optimize on purpose. // It is safe because we set a new key in a cache dictionary. actionsById[actionId] = liftedAction; - sanitizedActionsById[actionId] = options.actionSanitizer - ? liftAction(options.actionSanitizer(liftedAction.action, actionId)) - : liftedAction; stagedActionIds = [...stagedActionIds, actionId]; // Optimization: we know that only the new action needs computing. @@ -327,7 +315,6 @@ export function liftReducerWith( ({ monitorState, actionsById, - sanitizedActionsById, nextActionId, stagedActionIds, skippedActionIds, @@ -350,8 +337,7 @@ export function liftReducerWith( committedState, actionsById, stagedActionIds, - skippedActionIds, - options.stateSanitizer + skippedActionIds ); commitExcessActions(stagedActionIds.length - options.maxAge); @@ -379,8 +365,7 @@ export function liftReducerWith( committedState, actionsById, stagedActionIds, - skippedActionIds, - options.stateSanitizer + skippedActionIds ); commitExcessActions(stagedActionIds.length - options.maxAge); @@ -396,7 +381,6 @@ export function liftReducerWith( // Add a new action to only recompute state const actionId = nextActionId++; actionsById[actionId] = new PerformAction(liftedAction); - sanitizedActionsById[actionId] = new PerformAction(liftedAction); stagedActionIds = [...stagedActionIds, actionId]; minInvalidatedStateIndex = stagedActionIds.length - 1; @@ -409,8 +393,7 @@ export function liftReducerWith( committedState, actionsById, stagedActionIds, - skippedActionIds, - options.stateSanitizer + skippedActionIds ); // Recompute state history with latest reducer and update action @@ -446,15 +429,13 @@ export function liftReducerWith( committedState, actionsById, stagedActionIds, - skippedActionIds, - options.stateSanitizer + skippedActionIds ); monitorState = monitorReducer(monitorState, liftedAction); return { monitorState, actionsById, - sanitizedActionsById, nextActionId, stagedActionIds, skippedActionIds, diff --git a/modules/store-devtools/src/utils.ts b/modules/store-devtools/src/utils.ts index 36e8573534..38157a08db 100644 --- a/modules/store-devtools/src/utils.ts +++ b/modules/store-devtools/src/utils.ts @@ -1,6 +1,12 @@ +import { ActionSanitizer, StateSanitizer } from './config'; import { Action } from '@ngrx/store'; import { Observable } from 'rxjs/Observable'; -import { LiftedState } from './reducer'; +import { + LiftedState, + LiftedAction, + LiftedActions, + ComputedState, +} from './reducer'; import * as Actions from './actions'; export function difference(first: any[], second: any[]) { @@ -17,7 +23,7 @@ export function unliftState(liftedState: LiftedState) { return state; } -export function unliftAction(liftedState: LiftedState) { +export function unliftAction(liftedState: LiftedState): LiftedAction { return liftedState.actionsById[liftedState.nextActionId - 1]; } @@ -36,3 +42,62 @@ export function applyOperators( return operator.apply(source$, args); }, input$); } + +/** + * Sanitizes given actions with given function. + */ +export function sanitizeActions( + actionSanitizer: ActionSanitizer, + actions: LiftedActions +): LiftedActions { + return Object.keys(actions).reduce( + (sanitizedActions, actionIdx) => { + const idx = Number(actionIdx); + sanitizedActions[idx] = sanitizeAction( + actionSanitizer, + actions[idx], + idx + ); + return sanitizedActions; + }, + {} + ); +} + +/** + * Sanitizes given action with given function. + */ +export function sanitizeAction( + actionSanitizer: ActionSanitizer, + action: LiftedAction, + actionIdx: number +): LiftedAction { + return { + ...action, + action: actionSanitizer(action.action, actionIdx), + }; +} + +/** + * Sanitizes given states with given function. + */ +export function sanitizeStates( + stateSanitizer: StateSanitizer, + states: ComputedState[] +): ComputedState[] { + return states.map((computedState, idx) => ({ + state: sanitizeState(stateSanitizer, computedState.state, idx), + error: computedState.error, + })); +} + +/** + * Sanitizes given state with given function. + */ +export function sanitizeState( + stateSanitizer: StateSanitizer, + state: any, + stateIdx: number +) { + return stateSanitizer(state, stateIdx); +}