From 48f5027591b8a6379bd157aee9bf7910067fc7b0 Mon Sep 17 00:00:00 2001 From: "byron.stange" Date: Wed, 2 Aug 2017 13:13:45 +0200 Subject: [PATCH 1/2] fix(createSelector): Memoize projector fn --- modules/store/spec/selector.spec.ts | 18 ++++++++++++++++++ modules/store/src/selector.ts | 13 +++++++++---- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/modules/store/spec/selector.spec.ts b/modules/store/spec/selector.spec.ts index d2db3ddb8c..0d721a9ac5 100644 --- a/modules/store/spec/selector.spec.ts +++ b/modules/store/spec/selector.spec.ts @@ -41,6 +41,24 @@ describe('Selectors', () => { expect(projectFn).toHaveBeenCalledWith(countOne, countTwo); }); + it('should call the projector function only when the value of a dependent selector change', () => { + const firstState = { first: 'state', unchanged: 'state' }; + const secondState = { second: 'state', unchanged: 'state' }; + const neverChangingSelector = jasmine.createSpy('unchangedSelector').and.callFake((state: any) => { + return state.unchanged; + }); + const projectFn = jasmine.createSpy('projectionFn'); + const selector = createSelector( + neverChangingSelector, + projectFn + ); + + selector(firstState); + selector(secondState); + + expect(projectFn).toHaveBeenCalledTimes(1); + }); + it('should memoize the function', () => { const firstState = { first: 'state' }; const secondState = { second: 'state' }; diff --git a/modules/store/src/selector.ts b/modules/store/src/selector.ts index 00c34f84c9..2a20df84ad 100644 --- a/modules/store/src/selector.ts +++ b/modules/store/src/selector.ts @@ -115,19 +115,24 @@ export function createSelector(...args: any[]): Selector { selector.release && typeof selector.release === 'function' ); - const { memoized, reset } = memoize(function(state: any) { + const memoizedProjector = memoize(function(...selectors: any[]) { + return projector.apply(null, selectors); + }); + + const memoizedState = memoize(function(state: any) { const args = selectors.map(fn => fn(state)); - return projector.apply(null, args); + return memoizedProjector.memoized.apply(null, args); }); function release() { - reset(); + memoizedState.reset(); + memoizedProjector.reset(); memoizedSelectors.forEach(selector => selector.release()); } - return Object.assign(memoized, { release }); + return Object.assign(memoizedState.memoized, { release }); } export function createFeatureSelector( From c91fe935f45a9d98039477820cabb54259854e55 Mon Sep 17 00:00:00 2001 From: "byron.stange" Date: Wed, 2 Aug 2017 13:18:53 +0200 Subject: [PATCH 2/2] fix(createSelector): Memoize projector fn --- modules/store/spec/selector.spec.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/modules/store/spec/selector.spec.ts b/modules/store/spec/selector.spec.ts index 0d721a9ac5..ac563fdabe 100644 --- a/modules/store/spec/selector.spec.ts +++ b/modules/store/spec/selector.spec.ts @@ -44,14 +44,13 @@ describe('Selectors', () => { it('should call the projector function only when the value of a dependent selector change', () => { const firstState = { first: 'state', unchanged: 'state' }; const secondState = { second: 'state', unchanged: 'state' }; - const neverChangingSelector = jasmine.createSpy('unchangedSelector').and.callFake((state: any) => { - return state.unchanged; - }); + const neverChangingSelector = jasmine + .createSpy('unchangedSelector') + .and.callFake((state: any) => { + return state.unchanged; + }); const projectFn = jasmine.createSpy('projectionFn'); - const selector = createSelector( - neverChangingSelector, - projectFn - ); + const selector = createSelector(neverChangingSelector, projectFn); selector(firstState); selector(secondState);