From 9a9f77e99786c0e21702a9de7cfdea5080f35ea1 Mon Sep 17 00:00:00 2001 From: Seth Davenport Date: Thu, 1 Jun 2017 11:42:05 -0400 Subject: [PATCH] [Fixes #413] allow tests to reset observables cached by decorators (#414) [Fixes #413] allow tests to reset observables cached by decorators `@select` and `@select$` cache the results of their selections, in order to avoid memory leaks. However we need to be able to reset these cached copies when MockNgRedux.reset() is called. The solution is to track the cached selections in a lookup table instead of as closure variables in the decorator itself. --- .npmignore | 1 + CHANGELOG.md | 4 +++ package.json | 2 +- src/components/ng-redux.ts | 4 +-- src/components/observable-store.ts | 4 +-- src/components/selectors.ts | 3 ++ src/components/sub-store.ts | 3 +- src/decorators/select.spec.ts | 2 ++ src/decorators/select.ts | 24 ++++++++-------- src/index.ts | 7 ++++- src/utils/selection-map.ts | 44 ++++++++++++++++++++++++++++++ testing/ng-redux.mock.ts | 2 ++ 12 files changed, 80 insertions(+), 20 deletions(-) create mode 100644 src/utils/selection-map.ts diff --git a/.npmignore b/.npmignore index 590ff67..2f8261c 100644 --- a/.npmignore +++ b/.npmignore @@ -13,3 +13,4 @@ circle.yml npm-debug.log .compiled ISSUE_TEMPLATE.md +*.tgz diff --git a/CHANGELOG.md b/CHANGELOG.md index bb57fe6..1b89139 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# 6.4.2 + +* Fixed some issues with MockNgRedux and the select dectorators. See https://github.com/angular-redux/store/issues/413 for details. + # 6.4.1 * Fixed a memory leak with `@select`, `@select$`. See https://github.com/angular-redux/example-app/issues/34 for details. diff --git a/package.json b/package.json index 10a596e..f3db402 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@angular-redux/store", - "version": "6.4.1", + "version": "6.4.2", "description": "Angular 2 bindings for Redux", "main": "./lib/src/index.js", "scripts": { diff --git a/src/components/ng-redux.ts b/src/components/ng-redux.ts index 8e7e8ec..79aecfa 100644 --- a/src/components/ng-redux.ts +++ b/src/components/ng-redux.ts @@ -18,11 +18,11 @@ import 'rxjs/add/operator/map'; import 'rxjs/add/operator/filter'; import 'rxjs/add/operator/distinctUntilChanged'; import 'rxjs/add/operator/switchMap'; -import { Selector, PathSelector, resolveToFunctionSelector } from './selectors'; +import { Selector, PathSelector, Comparator, resolveToFunctionSelector } from './selectors'; import { assert } from '../utils/assert'; import { SubStore } from './sub-store'; import { enableFractalReducers } from './fractal-reducer-map'; -import { ObservableStore, Comparator } from './observable-store'; +import { ObservableStore } from './observable-store'; export class NgRedux implements ObservableStore { /** @hidden */ diff --git a/src/components/observable-store.ts b/src/components/observable-store.ts index 193785b..394fa04 100644 --- a/src/components/observable-store.ts +++ b/src/components/observable-store.ts @@ -1,8 +1,6 @@ import { Store, Reducer } from 'redux'; import { Observable } from 'rxjs/Observable'; -import { Selector, PathSelector } from './selectors'; - -export type Comparator = (x: any, y: any) => boolean; +import { Selector, PathSelector, Comparator } from './selectors'; /** * This interface represents the glue that connects the diff --git a/src/components/selectors.ts b/src/components/selectors.ts index d198a20..249606c 100644 --- a/src/components/selectors.ts +++ b/src/components/selectors.ts @@ -1,5 +1,8 @@ import { getIn } from '../utils/get-in'; +import { Observable } from 'rxjs/Observable'; +export type Comparator = (x: any, y: any) => boolean; +export type Transformer = (store$: Observable) => Observable export type PropertySelector = string | number | symbol; export type PathSelector = (string | number)[]; export type FunctionSelector = ((s: RootState) => S); diff --git a/src/components/sub-store.ts b/src/components/sub-store.ts index c85b206..c0fbbae 100644 --- a/src/components/sub-store.ts +++ b/src/components/sub-store.ts @@ -7,10 +7,11 @@ import { Selector, PropertySelector, FunctionSelector, + Comparator, resolveToFunctionSelector, } from './selectors'; import { NgRedux } from './ng-redux'; -import { ObservableStore, Comparator } from './observable-store'; +import { ObservableStore, } from './observable-store'; import { registerFractalReducer, replaceLocalReducer } from './fractal-reducer-map'; /** @hidden */ diff --git a/src/decorators/select.spec.ts b/src/decorators/select.spec.ts index 251b70c..950409e 100644 --- a/src/decorators/select.spec.ts +++ b/src/decorators/select.spec.ts @@ -7,6 +7,7 @@ import 'rxjs/add/operator/take'; import { NgRedux } from '../components/ng-redux'; import { select, select$ } from './select'; +import { selectionMap } from '../utils/selection-map'; class MockNgZone { run = fn => fn() } @@ -24,6 +25,7 @@ describe('Select decorators', () => { beforeEach(() => { targetObj = {}; + selectionMap.reset(); ngRedux = new NgRedux(mockNgZone); ngRedux.configureStore(rootReducer, defaultState); }); diff --git a/src/decorators/select.ts b/src/decorators/select.ts index 6be0ca7..ad2a9e2 100644 --- a/src/decorators/select.ts +++ b/src/decorators/select.ts @@ -1,8 +1,8 @@ import { Observable } from 'rxjs/Observable'; import 'rxjs/add/operator/let' import { NgRedux } from '../components/ng-redux'; -import { Selector } from '../components/selectors'; -import { Comparator } from '../components/observable-store'; +import { Selector, Comparator, Transformer } from '../components/selectors'; +import { selectionMap } from '../utils/selection-map'; /** * Selects an observable from the store, and attaches it to the decorated @@ -20,7 +20,6 @@ export function select( comparator?: Comparator) { return function decorate(target: any, key: string): void { - let result; let bindingKey = selector; if (!selector) { bindingKey = (key.lastIndexOf('$') === key.length - 1) ? @@ -29,10 +28,12 @@ export function select( } function getter() { - if (NgRedux.instance && !result) { - result = NgRedux.instance.select(bindingKey, comparator); + let selection = selectionMap.get(bindingKey, null, comparator); + if (NgRedux.instance && !selection) { + selection = NgRedux.instance.select(bindingKey, comparator); + selectionMap.set(bindingKey, null, comparator, selection); } - return result; + return selection; } // Replace decorated property with a getter that returns the observable. @@ -46,8 +47,6 @@ export function select( }; } -export type Transformer = (store$: Observable) => Observable - /** * Selects an observable using the given path selector, and runs it through the given * transformer function. A transformer function takes the store observable as an input and @@ -63,14 +62,15 @@ export function select$( comparator?: Comparator) { return function decorate(target: any, key: string): void { - let result; function getter() { - if (NgRedux.instance && !result) { - result = NgRedux.instance.select(selector) + let selection = selectionMap.get(selector, transformer, comparator); + if (NgRedux.instance && !selection) { + selection = NgRedux.instance.select(selector) .let(transformer) .distinctUntilChanged(comparator); + selectionMap.set(selector, transformer, comparator, selection); } - return result; + return selection; } // Replace decorated property with a getter that returns the observable. diff --git a/src/index.ts b/src/index.ts index a4caac6..c778b26 100644 --- a/src/index.ts +++ b/src/index.ts @@ -4,12 +4,15 @@ import { PathSelector, PropertySelector, FunctionSelector, + Comparator, + Transformer, } from './components/selectors'; -import { ObservableStore, Comparator } from './components/observable-store'; +import { ObservableStore } from './components/observable-store'; import { DevToolsExtension } from './components/dev-tools'; import { select, select$ } from './decorators/select'; import { dispatch } from './decorators/dispatch'; import { NgReduxModule } from './ng-redux.module'; +import { selectionMap } from './utils/selection-map'; // Warning: don't do this: // export * from './foo' @@ -22,10 +25,12 @@ export { PropertySelector, FunctionSelector, Comparator, + Transformer, NgReduxModule, DevToolsExtension, select, select$, dispatch, ObservableStore, + selectionMap, }; diff --git a/src/utils/selection-map.ts b/src/utils/selection-map.ts new file mode 100644 index 0000000..319b6ff --- /dev/null +++ b/src/utils/selection-map.ts @@ -0,0 +1,44 @@ +import { Observable } from 'rxjs/Observable'; +import { Selector, Comparator, Transformer } from '../components/selectors'; + +const toKey = (val: Object | Array | Function | String) => + val ? val.toString() : ''; + +const computeKey = ( + selector: Selector, + transformer: Transformer, + comparator: Comparator) => + `s:${toKey(selector)}:t:${toKey(transformer)}:c:${toKey(comparator)}`; + +/** + * Used to pool Observables created by @select and @select$. This + * avoids memory leaks and improves efficiency. + * @hidden + */ +export class SelectionMap { + private _map: { [id: string]: Observable } = {}; + + set( + selector: Selector, + transformer: Transformer, + comparator: Comparator, + selection: Observable): void { + const key = computeKey(selector, transformer, comparator); + this._map[key] = selection; + } + + get( + selector: Selector, + transformer: Transformer, + comparator: Comparator): Observable { + const key = computeKey(selector, transformer, comparator); + return this._map[key]; + } + + reset() { + this._map = {}; + } +} + +/** @hidden */ +export const selectionMap = new SelectionMap(); diff --git a/testing/ng-redux.mock.ts b/testing/ng-redux.mock.ts index c53a4be..8b36abe 100644 --- a/testing/ng-redux.mock.ts +++ b/testing/ng-redux.mock.ts @@ -4,6 +4,7 @@ import { Comparator, ObservableStore, PathSelector, + selectionMap, } from '@angular-redux/store'; import { Reducer, Action } from 'redux'; import { Observable } from 'rxjs/Observable'; @@ -53,6 +54,7 @@ export class MockNgRedux extends MockObservableStore { */ static reset(): void { MockNgRedux.mockInstance.reset(); + selectionMap.reset(); } /** @hidden */