Skip to content

Commit

Permalink
[Fixes angular-redux#413] allow tests to reset observables cached by …
Browse files Browse the repository at this point in the history
…decorators (angular-redux#414)

[Fixes angular-redux#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.
  • Loading branch information
SethDavenport authored Jun 1, 2017
1 parent 92fbda3 commit 9a9f77e
Show file tree
Hide file tree
Showing 12 changed files with 80 additions and 20 deletions.
1 change: 1 addition & 0 deletions .npmignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ circle.yml
npm-debug.log
.compiled
ISSUE_TEMPLATE.md
*.tgz
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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": {
Expand Down
4 changes: 2 additions & 2 deletions src/components/ng-redux.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<RootState> implements ObservableStore<RootState> {
/** @hidden */
Expand Down
4 changes: 1 addition & 3 deletions src/components/observable-store.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down
3 changes: 3 additions & 0 deletions src/components/selectors.ts
Original file line number Diff line number Diff line change
@@ -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<RootState, V> = (store$: Observable<RootState>) => Observable<V>
export type PropertySelector = string | number | symbol;
export type PathSelector = (string | number)[];
export type FunctionSelector<RootState, S> = ((s: RootState) => S);
Expand Down
3 changes: 2 additions & 1 deletion src/components/sub-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
2 changes: 2 additions & 0 deletions src/decorators/select.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() }

Expand All @@ -24,6 +25,7 @@ describe('Select decorators', () => {

beforeEach(() => {
targetObj = {};
selectionMap.reset();
ngRedux = new NgRedux(mockNgZone);
ngRedux.configureStore(rootReducer, defaultState);
});
Expand Down
24 changes: 12 additions & 12 deletions src/decorators/select.ts
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -20,7 +20,6 @@ export function select<T>(
comparator?: Comparator) {

return function decorate(target: any, key: string): void {
let result;
let bindingKey = selector;
if (!selector) {
bindingKey = (key.lastIndexOf('$') === key.length - 1) ?
Expand All @@ -29,10 +28,12 @@ export function select<T>(
}

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.
Expand All @@ -46,8 +47,6 @@ export function select<T>(
};
}

export type Transformer<RootState, V> = (store$: Observable<RootState>) => Observable<V>

/**
* 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
Expand All @@ -63,14 +62,15 @@ export function select$<T>(
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.
Expand Down
7 changes: 6 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -22,10 +25,12 @@ export {
PropertySelector,
FunctionSelector,
Comparator,
Transformer,
NgReduxModule,
DevToolsExtension,
select,
select$,
dispatch,
ObservableStore,
selectionMap,
};
44 changes: 44 additions & 0 deletions src/utils/selection-map.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { Observable } from 'rxjs/Observable';
import { Selector, Comparator, Transformer } from '../components/selectors';

const toKey = (val: Object | Array<any> | Function | String) =>
val ? val.toString() : '';

const computeKey = (
selector: Selector<any, any>,
transformer: Transformer<any, any>,
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<any> } = {};

set(
selector: Selector<any, any>,
transformer: Transformer<any, any>,
comparator: Comparator,
selection: Observable<any>): void {
const key = computeKey(selector, transformer, comparator);
this._map[key] = selection;
}

get(
selector: Selector<any, any>,
transformer: Transformer<any, any>,
comparator: Comparator): Observable<any> {
const key = computeKey(selector, transformer, comparator);
return this._map[key];
}

reset() {
this._map = {};
}
}

/** @hidden */
export const selectionMap = new SelectionMap();
2 changes: 2 additions & 0 deletions testing/ng-redux.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
Comparator,
ObservableStore,
PathSelector,
selectionMap,
} from '@angular-redux/store';
import { Reducer, Action } from 'redux';
import { Observable } from 'rxjs/Observable';
Expand Down Expand Up @@ -53,6 +54,7 @@ export class MockNgRedux<RootState> extends MockObservableStore<RootState> {
*/
static reset(): void {
MockNgRedux.mockInstance.reset();
selectionMap.reset();
}

/** @hidden */
Expand Down

0 comments on commit 9a9f77e

Please sign in to comment.