From f087a36d9cff446c36a93fa4ce4cb7a10fa45c14 Mon Sep 17 00:00:00 2001 From: Tim Deschryver Date: Thu, 28 Jun 2018 23:08:44 +0200 Subject: [PATCH 1/9] add features in batch --- modules/store/src/reducer_manager.ts | 191 ++++++++++++---------- modules/store/src/store_module.ts | 22 +-- modules/store/src/utils.ts | 234 +++++++++++++-------------- 3 files changed, 230 insertions(+), 217 deletions(-) diff --git a/modules/store/src/reducer_manager.ts b/modules/store/src/reducer_manager.ts index 89e3433f5b..4aab0d17d5 100644 --- a/modules/store/src/reducer_manager.ts +++ b/modules/store/src/reducer_manager.ts @@ -1,89 +1,102 @@ -import { Inject, Injectable, OnDestroy, Provider } from '@angular/core'; -import { BehaviorSubject, Observable } from 'rxjs'; - -import { ActionsSubject } from './actions_subject'; -import { - Action, - ActionReducer, - ActionReducerFactory, - ActionReducerMap, - StoreFeature, -} from './models'; -import { INITIAL_REDUCERS, INITIAL_STATE, REDUCER_FACTORY } from './tokens'; -import { - createFeatureReducerFactory, - createReducerFactory, - omit, -} from './utils'; - -export abstract class ReducerObservable extends Observable< - ActionReducer -> {} -export abstract class ReducerManagerDispatcher extends ActionsSubject {} -export const UPDATE = '@ngrx/store/update-reducers' as '@ngrx/store/update-reducers'; - -@Injectable() -export class ReducerManager extends BehaviorSubject> - implements OnDestroy { - constructor( - private dispatcher: ReducerManagerDispatcher, - @Inject(INITIAL_STATE) private initialState: any, - @Inject(INITIAL_REDUCERS) private reducers: ActionReducerMap, - @Inject(REDUCER_FACTORY) - private reducerFactory: ActionReducerFactory - ) { - super(reducerFactory(reducers, initialState)); - } - - addFeature({ - reducers, - reducerFactory, - metaReducers, - initialState, - key, - }: StoreFeature) { - const reducer = - typeof reducers === 'function' - ? createFeatureReducerFactory(metaReducers)(reducers, initialState) - : createReducerFactory(reducerFactory, metaReducers)( - reducers, - initialState - ); - - this.addReducer(key, reducer); - } - - removeFeature({ key }: StoreFeature) { - this.removeReducer(key); - } - - addReducer(key: string, reducer: ActionReducer) { - this.reducers = { ...this.reducers, [key]: reducer }; - - this.updateReducers(key); - } - - removeReducer(key: string) { - this.reducers = omit(this.reducers, key) /*TODO(#823)*/ as any; - - this.updateReducers(key); - } - - private updateReducers(key: string) { - this.next(this.reducerFactory(this.reducers, this.initialState)); - this.dispatcher.next({ - type: UPDATE, - feature: key, - }); - } - - ngOnDestroy() { - this.complete(); - } -} - -export const REDUCER_MANAGER_PROVIDERS: Provider[] = [ - ReducerManager, - { provide: ReducerObservable, useExisting: ReducerManager }, - { provide: ReducerManagerDispatcher, useExisting: ActionsSubject }, -]; +import { Inject, Injectable, OnDestroy, Provider } from '@angular/core'; +import { BehaviorSubject, Observable } from 'rxjs'; + +import { ActionsSubject } from './actions_subject'; +import { + Action, + ActionReducer, + ActionReducerFactory, + ActionReducerMap, + StoreFeature, +} from './models'; +import { INITIAL_REDUCERS, INITIAL_STATE, REDUCER_FACTORY } from './tokens'; +import { + createFeatureReducerFactory, + createReducerFactory, + omit, +} from './utils'; + +export abstract class ReducerObservable extends Observable< + ActionReducer +> {} +export abstract class ReducerManagerDispatcher extends ActionsSubject {} +export const UPDATE = '@ngrx/store/update-reducers' as '@ngrx/store/update-reducers'; + +@Injectable() +export class ReducerManager extends BehaviorSubject> + implements OnDestroy { + constructor( + private dispatcher: ReducerManagerDispatcher, + @Inject(INITIAL_STATE) private initialState: any, + @Inject(INITIAL_REDUCERS) private reducers: ActionReducerMap, + @Inject(REDUCER_FACTORY) + private reducerFactory: ActionReducerFactory + ) { + super(reducerFactory(reducers, initialState)); + } + + addFeature(feature: StoreFeature) { + this.addFeatures([feature]); + } + + addFeatures(features: StoreFeature[]) { + const reducers = features.reduce( + ( + reducerDict, + { reducers, reducerFactory, metaReducers, initialState, key } + ) => { + const reducer = + typeof reducers === 'function' + ? createFeatureReducerFactory(metaReducers)(reducers, initialState) + : createReducerFactory(reducerFactory, metaReducers)( + reducers, + initialState + ); + + reducerDict[key] = reducer; + return reducerDict; + }, + {} as { [key: string]: ActionReducer } + ); + + this.addReducers(reducers); + } + + removeFeature({ key }: StoreFeature) { + this.removeReducer(key); + } + + addReducer(key: string, reducer: ActionReducer) { + this.reducers = { ...this.reducers, [key]: reducer }; + this.updateReducers(key); + } + + addReducers(reducers: { [key: string]: ActionReducer }) { + this.reducers = { ...this.reducers, ...reducers }; + // TODO: find a better "name" + this.updateReducers('BATCH'); + } + + removeReducer(key: string) { + this.reducers = omit(this.reducers, key) /*TODO(#823)*/ as any; + this.updateReducers(key); + } + + private updateReducers(key: string) { + this.next(this.reducerFactory(this.reducers, this.initialState)); + this.dispatcher.next({ + type: UPDATE, + feature: key, + }); + } + + ngOnDestroy() { + this.complete(); + } +} + +export const REDUCER_MANAGER_PROVIDERS: Provider[] = [ + ReducerManager, + { provide: ReducerObservable, useExisting: ReducerManager }, + { provide: ReducerManagerDispatcher, useExisting: ActionsSubject }, +]; diff --git a/modules/store/src/store_module.ts b/modules/store/src/store_module.ts index 9275dd5955..25be35fefb 100644 --- a/modules/store/src/store_module.ts +++ b/modules/store/src/store_module.ts @@ -61,18 +61,18 @@ export class StoreFeatureModule implements OnDestroy { private reducerManager: ReducerManager, root: StoreRootModule ) { - features - .map((feature, index) => { - const featureReducerCollection = featureReducers.shift(); - const reducers = featureReducerCollection /*TODO(#823)*/![index]; + const feats = features.map((feature, index) => { + const featureReducerCollection = featureReducers.shift(); + const reducers = featureReducerCollection /*TODO(#823)*/![index]; - return { - ...feature, - reducers, - initialState: _initialStateFactory(feature.initialState), - }; - }) - .forEach(feature => reducerManager.addFeature(feature)); + return { + ...feature, + reducers, + initialState: _initialStateFactory(feature.initialState), + }; + }); + + reducerManager.addFeatures(feats); } ngOnDestroy() { diff --git a/modules/store/src/utils.ts b/modules/store/src/utils.ts index 7ded8a3810..f8df50006a 100644 --- a/modules/store/src/utils.ts +++ b/modules/store/src/utils.ts @@ -1,117 +1,117 @@ -import { - Action, - ActionReducer, - ActionReducerFactory, - ActionReducerMap, - MetaReducer, -} from './models'; - -export function combineReducers( - reducers: ActionReducerMap, - initialState?: Partial -): ActionReducer; -export function combineReducers( - reducers: any, - initialState: any = {} -): ActionReducer { - const reducerKeys = Object.keys(reducers); - const finalReducers: any = {}; - - for (let i = 0; i < reducerKeys.length; i++) { - const key = reducerKeys[i]; - if (typeof reducers[key] === 'function') { - finalReducers[key] = reducers[key]; - } - } - - const finalReducerKeys = Object.keys(finalReducers); - - return function combination(state, action) { - state = state === undefined ? initialState : state; - let hasChanged = false; - const nextState: any = {}; - for (let i = 0; i < finalReducerKeys.length; i++) { - const key = finalReducerKeys[i]; - const reducer: any = finalReducers[key]; - const previousStateForKey = state[key]; - const nextStateForKey = reducer(previousStateForKey, action); - - nextState[key] = nextStateForKey; - hasChanged = hasChanged || nextStateForKey !== previousStateForKey; - } - return hasChanged ? nextState : state; - }; -} - -export function omit( - object: T, - keyToRemove: keyof T -): Partial { - return Object.keys(object) - .filter(key => key !== keyToRemove) - .reduce((result, key) => Object.assign(result, { [key]: object[key] }), {}); -} - -export function compose(): (i: A) => A; -export function compose(b: (i: A) => B): (i: A) => B; -export function compose(c: (i: B) => C, b: (i: A) => B): (i: A) => C; -export function compose( - d: (i: C) => D, - c: (i: B) => C, - b: (i: A) => B -): (i: A) => D; -export function compose( - e: (i: D) => E, - d: (i: C) => D, - c: (i: B) => C, - b: (i: A) => B -): (i: A) => E; -export function compose( - f: (i: E) => F, - e: (i: D) => E, - d: (i: C) => D, - c: (i: B) => C, - b: (i: A) => B -): (i: A) => F; -export function compose(...functions: any[]): (i: A) => F; -export function compose(...functions: any[]) { - return function(arg: any) { - if (functions.length === 0) { - return arg; - } - - const last = functions[functions.length - 1]; - const rest = functions.slice(0, -1); - - return rest.reduceRight((composed, fn) => fn(composed), last(arg)); - }; -} - -export function createReducerFactory( - reducerFactory: ActionReducerFactory, - metaReducers?: MetaReducer[] -): ActionReducerFactory { - if (Array.isArray(metaReducers) && metaReducers.length > 0) { - return compose.apply(null, [...metaReducers, reducerFactory]); - } - - return reducerFactory; -} - -export function createFeatureReducerFactory( - metaReducers?: MetaReducer[] -): (reducer: ActionReducer, initialState?: T) => ActionReducer { - const reducerFactory = - Array.isArray(metaReducers) && metaReducers.length > 0 - ? compose>(...metaReducers) - : (r: ActionReducer) => r; - - return (reducer: ActionReducer, initialState?: T) => { - reducer = reducerFactory(reducer); - - return (state: T | undefined, action: V) => { - state = state === undefined ? initialState : state; - return reducer(state, action); - }; - }; -} +import { + Action, + ActionReducer, + ActionReducerFactory, + ActionReducerMap, + MetaReducer, +} from './models'; + +export function combineReducers( + reducers: ActionReducerMap, + initialState?: Partial +): ActionReducer; +export function combineReducers( + reducers: any, + initialState: any = {} +): ActionReducer { + const reducerKeys = Object.keys(reducers); + const finalReducers: any = {}; + // TODO: take a look at this + for (let i = 0; i < reducerKeys.length; i++) { + const key = reducerKeys[i]; + if (typeof reducers[key] === 'function') { + finalReducers[key] = reducers[key]; + } + } + + const finalReducerKeys = Object.keys(finalReducers); + + return function combination(state, action) { + state = state === undefined ? initialState : state; + let hasChanged = false; + const nextState: any = {}; + for (let i = 0; i < finalReducerKeys.length; i++) { + const key = finalReducerKeys[i]; + const reducer: any = finalReducers[key]; + const previousStateForKey = state[key]; + const nextStateForKey = reducer(previousStateForKey, action); + + nextState[key] = nextStateForKey; + hasChanged = hasChanged || nextStateForKey !== previousStateForKey; + } + return hasChanged ? nextState : state; + }; +} + +export function omit( + object: T, + keyToRemove: keyof T +): Partial { + return Object.keys(object) + .filter(key => key !== keyToRemove) + .reduce((result, key) => Object.assign(result, { [key]: object[key] }), {}); +} + +export function compose(): (i: A) => A; +export function compose(b: (i: A) => B): (i: A) => B; +export function compose(c: (i: B) => C, b: (i: A) => B): (i: A) => C; +export function compose( + d: (i: C) => D, + c: (i: B) => C, + b: (i: A) => B +): (i: A) => D; +export function compose( + e: (i: D) => E, + d: (i: C) => D, + c: (i: B) => C, + b: (i: A) => B +): (i: A) => E; +export function compose( + f: (i: E) => F, + e: (i: D) => E, + d: (i: C) => D, + c: (i: B) => C, + b: (i: A) => B +): (i: A) => F; +export function compose(...functions: any[]): (i: A) => F; +export function compose(...functions: any[]) { + return function(arg: any) { + if (functions.length === 0) { + return arg; + } + + const last = functions[functions.length - 1]; + const rest = functions.slice(0, -1); + + return rest.reduceRight((composed, fn) => fn(composed), last(arg)); + }; +} + +export function createReducerFactory( + reducerFactory: ActionReducerFactory, + metaReducers?: MetaReducer[] +): ActionReducerFactory { + if (Array.isArray(metaReducers) && metaReducers.length > 0) { + return compose.apply(null, [...metaReducers, reducerFactory]); + } + + return reducerFactory; +} + +export function createFeatureReducerFactory( + metaReducers?: MetaReducer[] +): (reducer: ActionReducer, initialState?: T) => ActionReducer { + const reducerFactory = + Array.isArray(metaReducers) && metaReducers.length > 0 + ? compose>(...metaReducers) + : (r: ActionReducer) => r; + + return (reducer: ActionReducer, initialState?: T) => { + reducer = reducerFactory(reducer); + + return (state: T | undefined, action: V) => { + state = state === undefined ? initialState : state; + return reducer(state, action); + }; + }; +} From 4f4283f5389b6e735cf467adb8bd0f23d3112d4d Mon Sep 17 00:00:00 2001 From: Tim Deschryver Date: Fri, 29 Jun 2018 09:35:15 +0200 Subject: [PATCH 2/9] test: add partial initial state test --- modules/store/spec/integration.spec.ts | 67 ++++++++++++++++++++------ 1 file changed, 51 insertions(+), 16 deletions(-) diff --git a/modules/store/spec/integration.spec.ts b/modules/store/spec/integration.spec.ts index 9267e860ce..e124788913 100644 --- a/modules/store/spec/integration.spec.ts +++ b/modules/store/spec/integration.spec.ts @@ -176,25 +176,25 @@ describe('ngRx Integration spec', () => { }); describe('feature state', () => { - const initialState = { - todos: [ - { - id: 1, - text: 'do things', - completed: false, - }, - ], - visibilityFilter: VisibilityFilters.SHOW_ALL, - }; + it('should initialize properly', () => { + const initialState = { + todos: [ + { + id: 1, + text: 'do things', + completed: false, + }, + ], + visibilityFilter: VisibilityFilters.SHOW_ALL, + }; - const reducers: ActionReducerMap = { - todos: todos, - visibilityFilter: visibilityFilter, - }; + const reducers: ActionReducerMap = { + todos: todos, + visibilityFilter: visibilityFilter, + }; - const featureInitialState = [{ id: 1, completed: false, text: 'Item' }]; + const featureInitialState = [{ id: 1, completed: false, text: 'Item' }]; - it('should initialize properly', () => { TestBed.configureTestingModule({ imports: [ StoreModule.forRoot(reducers, { initialState }), @@ -218,5 +218,40 @@ describe('ngRx Integration spec', () => { expect(state).toEqual(expected.shift()); }); }); + + it('should initialize properly with a partial state', () => { + const initialState = { + items: [{ id: 1, completed: false, text: 'Item' }], + }; + + const reducers: ActionReducerMap = { + todos: todos, + visibilityFilter: visibilityFilter, + }; + + TestBed.configureTestingModule({ + imports: [ + StoreModule.forRoot({} as any, { + initialState, + }), + StoreModule.forFeature('todos', reducers), + StoreModule.forFeature('items', todos), + ], + }); + + const store: Store = TestBed.get(Store); + + const expected = { + todos: { + todos: [], + visibilityFilter: VisibilityFilters.SHOW_ALL, + }, + items: [{ id: 1, completed: false, text: 'Item' }], + }; + + store.pipe(select(state => state)).subscribe(state => { + expect(state).toEqual(expected); + }); + }); }); }); From fcabb4abff44e464b833eeb68220e03ba506f888 Mon Sep 17 00:00:00 2001 From: Tim Deschryver Date: Fri, 29 Jun 2018 21:18:22 +0200 Subject: [PATCH 3/9] remove features in batch --- modules/store/src/reducer_manager.ts | 28 +++++++++++++++++++++------- modules/store/src/store_module.ts | 4 +--- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/modules/store/src/reducer_manager.ts b/modules/store/src/reducer_manager.ts index 4aab0d17d5..d96ef09b3a 100644 --- a/modules/store/src/reducer_manager.ts +++ b/modules/store/src/reducer_manager.ts @@ -62,8 +62,12 @@ export class ReducerManager extends BehaviorSubject> this.addReducers(reducers); } - removeFeature({ key }: StoreFeature) { - this.removeReducer(key); + removeFeature(feature: StoreFeature) { + this.removeFeatures([feature]); + } + + removeFeatures(features: StoreFeature[]) { + this.removeReducers(features.map(p => p.key)); } addReducer(key: string, reducer: ActionReducer) { @@ -73,13 +77,19 @@ export class ReducerManager extends BehaviorSubject> addReducers(reducers: { [key: string]: ActionReducer }) { this.reducers = { ...this.reducers, ...reducers }; - // TODO: find a better "name" - this.updateReducers('BATCH'); + this.updateReducers(this.concatFeatureKeys(Object.keys(reducers))); } - removeReducer(key: string) { - this.reducers = omit(this.reducers, key) /*TODO(#823)*/ as any; - this.updateReducers(key); + removeReducer(featureKey: string) { + this.reducers = omit(this.reducers, featureKey) /*TODO(#823)*/ as any; + this.updateReducers(featureKey); + } + + removeReducers(featureKeys: string[]) { + featureKeys.forEach(key => { + this.reducers = omit(this.reducers, key) /*TODO(#823)*/ as any; + }); + this.updateReducers(this.concatFeatureKeys(featureKeys)); } private updateReducers(key: string) { @@ -90,6 +100,10 @@ export class ReducerManager extends BehaviorSubject> }); } + private concatFeatureKeys(keys: string[]) { + return keys.join('___'); + } + ngOnDestroy() { this.complete(); } diff --git a/modules/store/src/store_module.ts b/modules/store/src/store_module.ts index 25be35fefb..32aa2b0616 100644 --- a/modules/store/src/store_module.ts +++ b/modules/store/src/store_module.ts @@ -76,9 +76,7 @@ export class StoreFeatureModule implements OnDestroy { } ngOnDestroy() { - this.features.forEach(feature => - this.reducerManager.removeFeature(feature) - ); + this.reducerManager.removeFeatures(this.features); } } From 7c8649853d32aaa709d0b1248fff3fbcec5525b4 Mon Sep 17 00:00:00 2001 From: Tim Deschryver Date: Sat, 30 Jun 2018 12:22:49 +0200 Subject: [PATCH 4/9] test: add update-reducer tests --- modules/store/spec/store.spec.ts | 21 +++++++++++++++++++++ modules/store/src/reducer_manager.ts | 6 ++---- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/modules/store/spec/store.spec.ts b/modules/store/spec/store.spec.ts index b0375ac9ca..8477e55ae8 100644 --- a/modules/store/spec/store.spec.ts +++ b/modules/store/spec/store.spec.ts @@ -7,6 +7,8 @@ import { Store, StoreModule, select, + ReducerManagerDispatcher, + UPDATE, } from '../'; import { counterReducer, @@ -267,16 +269,19 @@ describe('ngRx Store', () => { describe(`add/remove reducers`, () => { let addReducerSpy: Spy; let removeReducerSpy: Spy; + let reducerManagerDispatcherSpy: Spy; const key = 'counter4'; beforeEach(() => { setup(); const reducerManager = TestBed.get(ReducerManager); + const dispatcher = TestBed.get(ReducerManagerDispatcher); addReducerSpy = spyOn(reducerManager, 'addReducer').and.callThrough(); removeReducerSpy = spyOn( reducerManager, 'removeReducer' ).and.callThrough(); + reducerManagerDispatcherSpy = spyOn(dispatcher, 'next').and.callThrough(); }); it(`should delegate add/remove to ReducerManager`, () => { @@ -299,5 +304,21 @@ describe('ngRx Store', () => { expect(val.counter4).toBeUndefined(); }); }); + + it('should dispatch an update reducers action when a reducer is added', () => { + store.addReducer(key, counterReducer); + expect(reducerManagerDispatcherSpy).toHaveBeenCalledWith({ + type: UPDATE, + feature: key, + }); + }); + + it('should dispatch an update reducers action when a reducer is removed', () => { + store.removeReducer(key); + expect(reducerManagerDispatcherSpy).toHaveBeenCalledWith({ + type: UPDATE, + feature: key, + }); + }); }); }); diff --git a/modules/store/src/reducer_manager.ts b/modules/store/src/reducer_manager.ts index d96ef09b3a..7981820406 100644 --- a/modules/store/src/reducer_manager.ts +++ b/modules/store/src/reducer_manager.ts @@ -71,8 +71,7 @@ export class ReducerManager extends BehaviorSubject> } addReducer(key: string, reducer: ActionReducer) { - this.reducers = { ...this.reducers, [key]: reducer }; - this.updateReducers(key); + this.addReducers({ [key]: reducer }); } addReducers(reducers: { [key: string]: ActionReducer }) { @@ -81,8 +80,7 @@ export class ReducerManager extends BehaviorSubject> } removeReducer(featureKey: string) { - this.reducers = omit(this.reducers, featureKey) /*TODO(#823)*/ as any; - this.updateReducers(featureKey); + this.removeReducers([featureKey]); } removeReducers(featureKeys: string[]) { From a2aa5682229e6c58c5a8483bf99b4c3e433dcf59 Mon Sep 17 00:00:00 2001 From: Tim Deschryver Date: Sat, 30 Jun 2018 12:37:04 +0200 Subject: [PATCH 5/9] dispatch update-reducers with features as an array --- modules/store/src/reducer_manager.ts | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/modules/store/src/reducer_manager.ts b/modules/store/src/reducer_manager.ts index 7981820406..58e79fb31c 100644 --- a/modules/store/src/reducer_manager.ts +++ b/modules/store/src/reducer_manager.ts @@ -76,7 +76,7 @@ export class ReducerManager extends BehaviorSubject> addReducers(reducers: { [key: string]: ActionReducer }) { this.reducers = { ...this.reducers, ...reducers }; - this.updateReducers(this.concatFeatureKeys(Object.keys(reducers))); + this.updateReducers(Object.keys(reducers)); } removeReducer(featureKey: string) { @@ -87,19 +87,23 @@ export class ReducerManager extends BehaviorSubject> featureKeys.forEach(key => { this.reducers = omit(this.reducers, key) /*TODO(#823)*/ as any; }); - this.updateReducers(this.concatFeatureKeys(featureKeys)); + this.updateReducers(featureKeys); } - private updateReducers(key: string) { + private updateReducers(keys: string[]) { this.next(this.reducerFactory(this.reducers, this.initialState)); - this.dispatcher.next({ - type: UPDATE, - feature: key, - }); - } - private concatFeatureKeys(keys: string[]) { - return keys.join('___'); + if (keys.length === 1) { + this.dispatcher.next({ + type: UPDATE, + feature: keys[0], + }); + } else { + this.dispatcher.next({ + type: UPDATE, + features: keys, + }); + } } ngOnDestroy() { From 43652378f8f586d5ac68174dcf8c6c80c493d297 Mon Sep 17 00:00:00 2001 From: Tim Deschryver Date: Sat, 30 Jun 2018 14:01:42 +0200 Subject: [PATCH 6/9] test: add tests for partially initialized state --- modules/store/spec/store.spec.ts | 193 ++++++++++++++++++++++++------- 1 file changed, 151 insertions(+), 42 deletions(-) diff --git a/modules/store/spec/store.spec.ts b/modules/store/spec/store.spec.ts index 8477e55ae8..b7d0db53dd 100644 --- a/modules/store/spec/store.spec.ts +++ b/modules/store/spec/store.spec.ts @@ -9,6 +9,7 @@ import { select, ReducerManagerDispatcher, UPDATE, + REDUCER_FACTORY, } from '../'; import { counterReducer, @@ -49,56 +50,42 @@ describe('ngRx Store', () => { describe('initial state', () => { it('should handle an initial state object', (done: any) => { setup(); - - store.pipe(take(1)).subscribe({ - next(val) { - expect(val).toEqual({ counter1: 0, counter2: 1, counter3: 0 }); - }, - error: done, - complete: done, - }); + testStoreValue({ counter1: 0, counter2: 1, counter3: 0 }, done); }); it('should handle an initial state function', (done: any) => { setup(() => ({ counter1: 0, counter2: 5 })); - - store.pipe(take(1)).subscribe({ - next(val) { - expect(val).toEqual({ counter1: 0, counter2: 5, counter3: 0 }); - }, - error: done, - complete: done, - }); + testStoreValue({ counter1: 0, counter2: 5, counter3: 0 }, done); }); - function testInitialState(feature?: string) { - store = TestBed.get(Store); - dispatcher = TestBed.get(ActionsSubject); - - const actionSequence = '--a--b--c--d--e--f--g'; - const stateSequence = 'i-w-----x-----y--z---'; - const actionValues = { - a: { type: INCREMENT }, - b: { type: 'OTHER' }, - c: { type: RESET }, - d: { type: 'OTHER' }, // reproduces https://github.com/ngrx/platform/issues/880 because state is falsey - e: { type: INCREMENT }, - f: { type: INCREMENT }, - g: { type: 'OTHER' }, - }; - const counterSteps = hot(actionSequence, actionValues); - counterSteps.subscribe(action => store.dispatch(action)); - - const counterStateWithString = feature - ? (store as any).select(feature, 'counter1') - : store.select('counter1'); - - const counter1Values = { i: 1, w: 2, x: 0, y: 1, z: 2 }; + it('should keep initial state values when state is partially initialized', (done: DoneFn) => { + TestBed.configureTestingModule({ + imports: [ + StoreModule.forRoot({} as any, { + initialState: { + feature1: { + counter1: 1, + }, + feature3: { + counter3: 3, + }, + }, + }), + StoreModule.forFeature('feature1', { counter1: counterReducer }), + StoreModule.forFeature('feature2', { counter2: counterReducer }), + StoreModule.forFeature('feature3', { counter3: counterReducer }), + ], + }); - expect(counterStateWithString).toBeObservable( - hot(stateSequence, counter1Values) + testStoreValue( + { + feature1: { counter1: 1 }, + feature2: { counter2: 0 }, + feature3: { counter3: 3 }, + }, + done ); - } + }); it('should reset to initial state when undefined (root ActionReducerMap)', () => { TestBed.configureTestingModule({ @@ -140,6 +127,47 @@ describe('ngRx Store', () => { testInitialState('feature1'); }); + + function testInitialState(feature?: string) { + store = TestBed.get(Store); + dispatcher = TestBed.get(ActionsSubject); + + const actionSequence = '--a--b--c--d--e--f--g'; + const stateSequence = 'i-w-----x-----y--z---'; + const actionValues = { + a: { type: INCREMENT }, + b: { type: 'OTHER' }, + c: { type: RESET }, + d: { type: 'OTHER' }, // reproduces https://github.com/ngrx/platform/issues/880 because state is falsey + e: { type: INCREMENT }, + f: { type: INCREMENT }, + g: { type: 'OTHER' }, + }; + const counterSteps = hot(actionSequence, actionValues); + counterSteps.subscribe(action => store.dispatch(action)); + + const counterStateWithString = feature + ? (store as any).select(feature, 'counter1') + : store.select('counter1'); + + const counter1Values = { i: 1, w: 2, x: 0, y: 1, z: 2 }; + + expect(counterStateWithString).toBeObservable( + hot(stateSequence, counter1Values) + ); + } + + function testStoreValue(expected: any, done: DoneFn) { + store = TestBed.get(Store); + + store.pipe(take(1)).subscribe({ + next(val) { + expect(val).toEqual(expected); + }, + error: done, + complete: done, + }); + } }); describe('basic store actions', () => { @@ -321,4 +349,85 @@ describe('ngRx Store', () => { }); }); }); + + describe('add/remove features', () => { + let reducerManager: ReducerManager; + let reducerManagerDispatcherSpy: Spy; + + beforeEach(() => { + TestBed.configureTestingModule({ + imports: [StoreModule.forRoot({})], + }); + + reducerManager = TestBed.get(ReducerManager); + const dispatcher = TestBed.get(ReducerManagerDispatcher); + reducerManagerDispatcherSpy = spyOn(dispatcher, 'next').and.callThrough(); + }); + + it('should dispatch an update reducers action when a feature is added', () => { + reducerManager.addFeature( + createFeature({ + key: 'feature1', + }) + ); + + expect(reducerManagerDispatcherSpy).toHaveBeenCalledWith({ + type: UPDATE, + feature: 'feature1', + }); + }); + + it('should dispatch an update reducers action when multiple features are added', () => { + reducerManager.addFeatures([ + createFeature({ + key: 'feature1', + }), + createFeature({ + key: 'feature2', + }), + ]); + + expect(reducerManagerDispatcherSpy).toHaveBeenCalledWith({ + type: UPDATE, + features: ['feature1', 'feature2'], + }); + }); + + it('should dispatch an update reducers action when a feature is removed', () => { + reducerManager.removeFeature( + createFeature({ + key: 'feature1', + }) + ); + + expect(reducerManagerDispatcherSpy).toHaveBeenCalledWith({ + type: UPDATE, + feature: 'feature1', + }); + }); + + it('should dispatch an update reducers action when multiple features are removed', () => { + reducerManager.removeFeatures([ + createFeature({ + key: 'feature1', + }), + createFeature({ + key: 'feature2', + }), + ]); + + expect(reducerManagerDispatcherSpy).toHaveBeenCalledWith({ + type: UPDATE, + features: ['feature1', 'feature2'], + }); + }); + + function createFeature({ key }: { key: string }) { + return { + key, + reducers: {}, + reducerFactory: jasmine.createSpy(`reducerFactory_${key}`), + }; + } + }); }); From f967d2218420b3005a106dd1e2414e3d99caeb3b Mon Sep 17 00:00:00 2001 From: Tim Deschryver Date: Sat, 30 Jun 2018 14:16:36 +0200 Subject: [PATCH 7/9] cleanup todo --- modules/store/src/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/store/src/utils.ts b/modules/store/src/utils.ts index f8df50006a..f4862cc6a7 100644 --- a/modules/store/src/utils.ts +++ b/modules/store/src/utils.ts @@ -16,7 +16,7 @@ export function combineReducers( ): ActionReducer { const reducerKeys = Object.keys(reducers); const finalReducers: any = {}; - // TODO: take a look at this + for (let i = 0; i < reducerKeys.length; i++) { const key = reducerKeys[i]; if (typeof reducers[key] === 'function') { From a4d57cfd6cc4cabfd87c8c7c3367fe6843a26fb4 Mon Sep 17 00:00:00 2001 From: Tim Deschryver Date: Thu, 5 Jul 2018 16:03:37 +0200 Subject: [PATCH 8/9] revert utils --- modules/store/src/utils.ts | 234 ++++++++++++++++++------------------- 1 file changed, 117 insertions(+), 117 deletions(-) diff --git a/modules/store/src/utils.ts b/modules/store/src/utils.ts index f4862cc6a7..7ded8a3810 100644 --- a/modules/store/src/utils.ts +++ b/modules/store/src/utils.ts @@ -1,117 +1,117 @@ -import { - Action, - ActionReducer, - ActionReducerFactory, - ActionReducerMap, - MetaReducer, -} from './models'; - -export function combineReducers( - reducers: ActionReducerMap, - initialState?: Partial -): ActionReducer; -export function combineReducers( - reducers: any, - initialState: any = {} -): ActionReducer { - const reducerKeys = Object.keys(reducers); - const finalReducers: any = {}; - - for (let i = 0; i < reducerKeys.length; i++) { - const key = reducerKeys[i]; - if (typeof reducers[key] === 'function') { - finalReducers[key] = reducers[key]; - } - } - - const finalReducerKeys = Object.keys(finalReducers); - - return function combination(state, action) { - state = state === undefined ? initialState : state; - let hasChanged = false; - const nextState: any = {}; - for (let i = 0; i < finalReducerKeys.length; i++) { - const key = finalReducerKeys[i]; - const reducer: any = finalReducers[key]; - const previousStateForKey = state[key]; - const nextStateForKey = reducer(previousStateForKey, action); - - nextState[key] = nextStateForKey; - hasChanged = hasChanged || nextStateForKey !== previousStateForKey; - } - return hasChanged ? nextState : state; - }; -} - -export function omit( - object: T, - keyToRemove: keyof T -): Partial { - return Object.keys(object) - .filter(key => key !== keyToRemove) - .reduce((result, key) => Object.assign(result, { [key]: object[key] }), {}); -} - -export function compose(): (i: A) => A; -export function compose(b: (i: A) => B): (i: A) => B; -export function compose(c: (i: B) => C, b: (i: A) => B): (i: A) => C; -export function compose( - d: (i: C) => D, - c: (i: B) => C, - b: (i: A) => B -): (i: A) => D; -export function compose( - e: (i: D) => E, - d: (i: C) => D, - c: (i: B) => C, - b: (i: A) => B -): (i: A) => E; -export function compose( - f: (i: E) => F, - e: (i: D) => E, - d: (i: C) => D, - c: (i: B) => C, - b: (i: A) => B -): (i: A) => F; -export function compose(...functions: any[]): (i: A) => F; -export function compose(...functions: any[]) { - return function(arg: any) { - if (functions.length === 0) { - return arg; - } - - const last = functions[functions.length - 1]; - const rest = functions.slice(0, -1); - - return rest.reduceRight((composed, fn) => fn(composed), last(arg)); - }; -} - -export function createReducerFactory( - reducerFactory: ActionReducerFactory, - metaReducers?: MetaReducer[] -): ActionReducerFactory { - if (Array.isArray(metaReducers) && metaReducers.length > 0) { - return compose.apply(null, [...metaReducers, reducerFactory]); - } - - return reducerFactory; -} - -export function createFeatureReducerFactory( - metaReducers?: MetaReducer[] -): (reducer: ActionReducer, initialState?: T) => ActionReducer { - const reducerFactory = - Array.isArray(metaReducers) && metaReducers.length > 0 - ? compose>(...metaReducers) - : (r: ActionReducer) => r; - - return (reducer: ActionReducer, initialState?: T) => { - reducer = reducerFactory(reducer); - - return (state: T | undefined, action: V) => { - state = state === undefined ? initialState : state; - return reducer(state, action); - }; - }; -} +import { + Action, + ActionReducer, + ActionReducerFactory, + ActionReducerMap, + MetaReducer, +} from './models'; + +export function combineReducers( + reducers: ActionReducerMap, + initialState?: Partial +): ActionReducer; +export function combineReducers( + reducers: any, + initialState: any = {} +): ActionReducer { + const reducerKeys = Object.keys(reducers); + const finalReducers: any = {}; + + for (let i = 0; i < reducerKeys.length; i++) { + const key = reducerKeys[i]; + if (typeof reducers[key] === 'function') { + finalReducers[key] = reducers[key]; + } + } + + const finalReducerKeys = Object.keys(finalReducers); + + return function combination(state, action) { + state = state === undefined ? initialState : state; + let hasChanged = false; + const nextState: any = {}; + for (let i = 0; i < finalReducerKeys.length; i++) { + const key = finalReducerKeys[i]; + const reducer: any = finalReducers[key]; + const previousStateForKey = state[key]; + const nextStateForKey = reducer(previousStateForKey, action); + + nextState[key] = nextStateForKey; + hasChanged = hasChanged || nextStateForKey !== previousStateForKey; + } + return hasChanged ? nextState : state; + }; +} + +export function omit( + object: T, + keyToRemove: keyof T +): Partial { + return Object.keys(object) + .filter(key => key !== keyToRemove) + .reduce((result, key) => Object.assign(result, { [key]: object[key] }), {}); +} + +export function compose(): (i: A) => A; +export function compose(b: (i: A) => B): (i: A) => B; +export function compose(c: (i: B) => C, b: (i: A) => B): (i: A) => C; +export function compose( + d: (i: C) => D, + c: (i: B) => C, + b: (i: A) => B +): (i: A) => D; +export function compose( + e: (i: D) => E, + d: (i: C) => D, + c: (i: B) => C, + b: (i: A) => B +): (i: A) => E; +export function compose( + f: (i: E) => F, + e: (i: D) => E, + d: (i: C) => D, + c: (i: B) => C, + b: (i: A) => B +): (i: A) => F; +export function compose(...functions: any[]): (i: A) => F; +export function compose(...functions: any[]) { + return function(arg: any) { + if (functions.length === 0) { + return arg; + } + + const last = functions[functions.length - 1]; + const rest = functions.slice(0, -1); + + return rest.reduceRight((composed, fn) => fn(composed), last(arg)); + }; +} + +export function createReducerFactory( + reducerFactory: ActionReducerFactory, + metaReducers?: MetaReducer[] +): ActionReducerFactory { + if (Array.isArray(metaReducers) && metaReducers.length > 0) { + return compose.apply(null, [...metaReducers, reducerFactory]); + } + + return reducerFactory; +} + +export function createFeatureReducerFactory( + metaReducers?: MetaReducer[] +): (reducer: ActionReducer, initialState?: T) => ActionReducer { + const reducerFactory = + Array.isArray(metaReducers) && metaReducers.length > 0 + ? compose>(...metaReducers) + : (r: ActionReducer) => r; + + return (reducer: ActionReducer, initialState?: T) => { + reducer = reducerFactory(reducer); + + return (state: T | undefined, action: V) => { + state = state === undefined ? initialState : state; + return reducer(state, action); + }; + }; +} From 126195d2f00cfa4ea7068949fe0ab8aa11be487c Mon Sep 17 00:00:00 2001 From: Tim Deschryver Date: Thu, 5 Jul 2018 18:58:49 +0200 Subject: [PATCH 9/9] dispatch every updated feature --- modules/store/spec/store.spec.ts | 28 ++++++++++++++++++++++------ modules/store/src/reducer_manager.ts | 13 ++++--------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/modules/store/spec/store.spec.ts b/modules/store/spec/store.spec.ts index b7d0db53dd..22562bcf48 100644 --- a/modules/store/spec/store.spec.ts +++ b/modules/store/spec/store.spec.ts @@ -377,7 +377,7 @@ describe('ngRx Store', () => { }); }); - it('should dispatch an update reducers action when multiple features are added', () => { + it('should dispatch an update reducers action for each feature that is added', () => { reducerManager.addFeatures([ createFeature({ key: 'feature1', @@ -387,9 +387,18 @@ describe('ngRx Store', () => { }), ]); - expect(reducerManagerDispatcherSpy).toHaveBeenCalledWith({ + expect(reducerManagerDispatcherSpy).toHaveBeenCalledTimes(2); + + // get the first argument for the first call + expect(reducerManagerDispatcherSpy.calls.argsFor(0)[0]).toEqual({ + type: UPDATE, + feature: 'feature1', + }); + + // get the first argument for the second call + expect(reducerManagerDispatcherSpy.calls.argsFor(1)[0]).toEqual({ type: UPDATE, - features: ['feature1', 'feature2'], + feature: 'feature2', }); }); @@ -406,7 +415,7 @@ describe('ngRx Store', () => { }); }); - it('should dispatch an update reducers action when multiple features are removed', () => { + it('should dispatch an update reducers action for each feature that is removed', () => { reducerManager.removeFeatures([ createFeature({ key: 'feature1', @@ -416,9 +425,16 @@ describe('ngRx Store', () => { }), ]); - expect(reducerManagerDispatcherSpy).toHaveBeenCalledWith({ + // get the first argument for the first call + expect(reducerManagerDispatcherSpy.calls.argsFor(0)[0]).toEqual({ + type: UPDATE, + feature: 'feature1', + }); + + // get the first argument for the second call + expect(reducerManagerDispatcherSpy.calls.argsFor(1)[0]).toEqual({ type: UPDATE, - features: ['feature1', 'feature2'], + feature: 'feature2', }); }); diff --git a/modules/store/src/reducer_manager.ts b/modules/store/src/reducer_manager.ts index 58e79fb31c..de6e733777 100644 --- a/modules/store/src/reducer_manager.ts +++ b/modules/store/src/reducer_manager.ts @@ -90,20 +90,15 @@ export class ReducerManager extends BehaviorSubject> this.updateReducers(featureKeys); } - private updateReducers(keys: string[]) { + private updateReducers(featureKeys: string[]) { this.next(this.reducerFactory(this.reducers, this.initialState)); - if (keys.length === 1) { + featureKeys.forEach(feature => { this.dispatcher.next({ type: UPDATE, - feature: keys[0], + feature, }); - } else { - this.dispatcher.next({ - type: UPDATE, - features: keys, - }); - } + }); } ngOnDestroy() {