From bacc9d4024a5c22b6d8c10be2743e94f8ba7df24 Mon Sep 17 00:00:00 2001 From: Sam Jackson Date: Thu, 21 Jan 2016 17:02:17 -0800 Subject: [PATCH 01/14] adding basic lru logic --- src/reactor.js | 3 ++ src/reactor/fns.js | 35 +++++++++++++++++----- src/reactor/records.js | 4 ++- tests/reactor-tests.js | 67 +++++++++++++++++++++++++++++++++++++++++- 4 files changed, 100 insertions(+), 9 deletions(-) diff --git a/src/reactor.js b/src/reactor.js index ce46ecb..0288a8c 100644 --- a/src/reactor.js +++ b/src/reactor.js @@ -24,9 +24,12 @@ import { class Reactor { constructor(config = {}) { const debug = !!config.debug + const itemsToCache = Number(config.maxItemsToCache) + const maxItemsToCache = itemsToCache && itemsToCache > 1 ? itemsToCache : null const baseOptions = debug ? DEBUG_OPTIONS : PROD_OPTIONS const initialReactorState = new ReactorState({ debug: debug, + maxItemsToCache: maxItemsToCache, // merge config options with the defaults options: baseOptions.merge(config.options || {}), }) diff --git a/src/reactor/fns.js b/src/reactor/fns.js index 7f47527..2f0d404 100644 --- a/src/reactor/fns.js +++ b/src/reactor/fns.js @@ -10,6 +10,7 @@ import { each } from '../utils' * Immutable Types */ const EvaluateResult = Immutable.Record({ result: null, reactorState: null}) +export const CACHE_CLEAR_RATIO = .8; function evaluateResult(result, reactorState) { return new EvaluateResult({ @@ -335,14 +336,13 @@ export function evaluate(reactorState, keyPathOrGetter) { // Cache hit return evaluateResult( getCachedValue(reactorState, keyPathOrGetter), - reactorState + updateCacheRecency(reactorState, keyPathOrGetter) ) } // evaluate dependencies const args = getDeps(keyPathOrGetter).map(dep => evaluate(reactorState, dep).result) const evaluatedValue = getComputeFn(keyPathOrGetter).apply(null, args) - return evaluateResult( evaluatedValue, cacheValue(reactorState, keyPathOrGetter, evaluatedValue) @@ -434,11 +434,32 @@ function cacheValue(reactorState, getter, value) { }) }) - return reactorState.setIn(['cache', cacheKey], Immutable.Map({ - value: value, - storeStates: storeStates, - dispatchId: dispatchId, - })) + const maxItemsToCache = reactorState.get('maxItemsToCache') + const itemsToCache = maxItemsToCache * CACHE_CLEAR_RATIO + + return reactorState.withMutations(state => { + if (maxItemsToCache && maxItemsToCache <= state.get('cache').size) { + do { + let key = state.get('cacheRecency').first() + state.deleteIn(['cache', key]) + state.deleteIn(['cacheRecency', key]) + } while (itemsToCache < state.get('cache').size) + } + + state.setIn(['cacheRecency', cacheKey], cacheKey) + state.setIn(['cache', cacheKey], Immutable.Map({ + value: value, + storeStates: storeStates, + dispatchId: dispatchId, + })) + }) +} + +function updateCacheRecency(reactorState, cacheKey) { + return reactorState.withMutations(state => { + state.deleteIn(['cacheRecency', cacheKey]) + state.setIn(['cacheRecency', cacheKey], cacheKey) + }) } /** diff --git a/src/reactor/records.js b/src/reactor/records.js index a59314f..fe8028d 100644 --- a/src/reactor/records.js +++ b/src/reactor/records.js @@ -1,4 +1,4 @@ -import { Map, Set, Record } from 'immutable' +import { Map, Set, Record, OrderedMap } from 'immutable' export const PROD_OPTIONS = Map({ // logs information for each dispatch @@ -39,10 +39,12 @@ export const ReactorState = Record({ state: Map(), stores: Map(), cache: Map(), + cacheRecency: OrderedMap(), // maintains a mapping of storeId => state id (monotomically increasing integer whenever store state changes) storeStates: Map(), dirtyStores: Set(), debug: false, + maxItemsToCache: null, // production defaults options: PROD_OPTIONS, }) diff --git a/tests/reactor-tests.js b/tests/reactor-tests.js index bf095db..96a6ed5 100644 --- a/tests/reactor-tests.js +++ b/tests/reactor-tests.js @@ -1,6 +1,6 @@ import Immutable, { Map, List, is } from 'immutable' import { Reactor, Store } from '../src/main' -import { getOption } from '../src/reactor/fns' +import { getOption, CACHE_CLEAR_RATIO } from '../src/reactor/fns' import { toImmutable } from '../src/immutable-helpers' import { PROD_OPTIONS, DEBUG_OPTIONS } from '../src/reactor/records' import logging from '../src/logging' @@ -15,6 +15,7 @@ describe('Reactor', () => { it('should create a reactor with PROD_OPTIONS', () => { var reactor = new Reactor() expect(reactor.reactorState.get('debug')).toBe(false) + expect(reactor.reactorState.get('maxItemsToCache')).toBe(null) expect(is(reactor.reactorState.get('options'), PROD_OPTIONS)).toBe(true) }) it('should create a reactor with DEBUG_OPTIONS', () => { @@ -22,8 +23,23 @@ describe('Reactor', () => { debug: true, }) expect(reactor.reactorState.get('debug')).toBe(true) + expect(reactor.reactorState.get('maxItemsToCache')).toBe(null) expect(is(reactor.reactorState.get('options'), DEBUG_OPTIONS)).toBe(true) }) + it('should create a reactor that overrides maxItemsToCache', () => { + var reactor = new Reactor({ + maxItemsToCache: 20, + }) + expect(reactor.reactorState.get('maxItemsToCache')).toBe(20) + reactor = new Reactor({ + maxItemsToCache: -20, + }) + expect(reactor.reactorState.get('maxItemsToCache')).toBe(null) + reactor = new Reactor({ + maxItemsToCache: 'abc', + }) + expect(reactor.reactorState.get('maxItemsToCache')).toBe(null) + }) it('should override PROD options', () => { var reactor = new Reactor({ options: { @@ -595,6 +611,55 @@ describe('Reactor', () => { expect(taxPercentSpy.calls.count()).toEqual(2) expect(subtotalSpy.calls.count()).toEqual(1) }) + + + describe('recency caching', () => { + var getters = Array.from(new Array(100), () => { + var z = Math.random() + return [['price'], function() { + return z + }] + }) + var cacheReactor + const maxItemsToCache = 50 + + beforeEach(() => { + cacheReactor = new Reactor({ + maxItemsToCache: maxItemsToCache + }) + }) + + it('should add getters to the recency cache map', () => { + for (let x = 0; x < 50; x++) { + cacheReactor.evaluate(getters[x]) + } + expect(cacheReactor.reactorState.get('cacheRecency').size).toBe(50) + expect(cacheReactor.reactorState.get('cacheRecency').first()).toBe(getters[0]) + expect(cacheReactor.reactorState.get('cacheRecency').last()).toBe(getters[49]) + }) + + it('should prioritize the recency cache by when getters were added', () => { + for (let x = 0; x < 10; x++) { + cacheReactor.evaluate(getters[x]) + } + cacheReactor.evaluate(getters[0]) + expect(cacheReactor.reactorState.get('cacheRecency').size).toBe(10) + expect(cacheReactor.reactorState.get('cacheRecency').last()).toBe(getters[0]) + expect(cacheReactor.reactorState.get('cacheRecency').first()).toBe(getters[1]) + }) + + it('should clear the appropriate ratio of items from cache when the maxItemsToCache limit is exceeded', () => { + for (let x = 0; x <= maxItemsToCache; x++) { + cacheReactor.evaluate(getters[x]) + } + expect(cacheReactor.reactorState.get('cacheRecency').size).toBe(cacheReactor.reactorState.get('cache').size) + expect(cacheReactor.reactorState.get('cacheRecency').size).toBe(Math.floor(CACHE_CLEAR_RATIO * maxItemsToCache) + 1) + expect(cacheReactor.reactorState.getIn(['cacheRecency', getters[0]])).toBe(undefined) + expect(cacheReactor.reactorState.getIn(['cacheRecency', getters[50]])).toBe(getters[50]) + + + }) + }) }) describe('#observe', () => { From 836b4e62a3e472c29a93d95f5302ee58b416658f Mon Sep 17 00:00:00 2001 From: Sam Jackson Date: Thu, 21 Jan 2016 17:06:18 -0800 Subject: [PATCH 02/14] comments/formatting --- src/reactor/fns.js | 6 ++++++ tests/reactor-tests.js | 3 --- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/reactor/fns.js b/src/reactor/fns.js index 2f0d404..5b9a578 100644 --- a/src/reactor/fns.js +++ b/src/reactor/fns.js @@ -455,6 +455,12 @@ function cacheValue(reactorState, getter, value) { }) } +/** + * Readds the key for the item in cache to update recency + * @param {ReactorState} reactorState + * @param {cacheKey} cacheKey + * @return {ReactorState} + */ function updateCacheRecency(reactorState, cacheKey) { return reactorState.withMutations(state => { state.deleteIn(['cacheRecency', cacheKey]) diff --git a/tests/reactor-tests.js b/tests/reactor-tests.js index 96a6ed5..6ab808a 100644 --- a/tests/reactor-tests.js +++ b/tests/reactor-tests.js @@ -612,7 +612,6 @@ describe('Reactor', () => { expect(subtotalSpy.calls.count()).toEqual(1) }) - describe('recency caching', () => { var getters = Array.from(new Array(100), () => { var z = Math.random() @@ -656,8 +655,6 @@ describe('Reactor', () => { expect(cacheReactor.reactorState.get('cacheRecency').size).toBe(Math.floor(CACHE_CLEAR_RATIO * maxItemsToCache) + 1) expect(cacheReactor.reactorState.getIn(['cacheRecency', getters[0]])).toBe(undefined) expect(cacheReactor.reactorState.getIn(['cacheRecency', getters[50]])).toBe(getters[50]) - - }) }) }) From 3c940f565cfcf7f535d24c9bb694449cb8b68e23 Mon Sep 17 00:00:00 2001 From: Sam Jackson Date: Mon, 25 Jan 2016 12:16:42 -0800 Subject: [PATCH 03/14] addGetterOptions --- src/getter.js | 30 +++++++++++ src/main.js | 3 +- src/reactor.js | 10 ++++ src/reactor/fns.js | 13 +++++ src/reactor/records.js | 1 + tests/getter-tests.js | 35 ++++++++++++- tests/reactor-tests.js | 110 ++++++++++++++++++++++++++++++++++------- 7 files changed, 182 insertions(+), 20 deletions(-) diff --git a/src/getter.js b/src/getter.js index aece455..70687be 100644 --- a/src/getter.js +++ b/src/getter.js @@ -102,6 +102,35 @@ function getStoreDeps(getter) { return storeDeps } +/** + * Add override options to a getter + * @param {getter} getter + * @param {object} options + * @param {boolean} options.useCache + * @param {*} options.cacheKey + * @returns {getter} + */ +function addGetterOptions(getter, options) { + if (!isKeyPath(getter) && !isGetter(getter)) { + throw new Error('createGetter must be passed a keyPath or Getter') + } + + if (getter.hasOwnProperty('__options')) { + throw new Error('Cannot reassign options to getter') + } + + getter.__options = {} + + if (options.useCache !== undefined) { + getter.__options.useCache = !!options.useCache + } + + if (options.cacheKey !== undefined) { + getter.__options.cacheKey = options.cacheKey + } + return getter +} + export default { isGetter, getComputeFn, @@ -109,4 +138,5 @@ export default { getStoreDeps, getDeps, fromKeyPath, + addGetterOptions, } diff --git a/src/main.js b/src/main.js index 3e0ebc1..8f9e6fa 100644 --- a/src/main.js +++ b/src/main.js @@ -4,7 +4,7 @@ import Reactor from './reactor' import Immutable from 'immutable' import { toJS, toImmutable, isImmutable } from './immutable-helpers' import { isKeyPath } from './key-path' -import { isGetter } from './getter' +import { isGetter, addGetterOptions } from './getter' import createReactMixin from './create-react-mixin' export default { @@ -17,4 +17,5 @@ export default { toImmutable, isImmutable, createReactMixin, + addGetterOptions, } diff --git a/src/reactor.js b/src/reactor.js index 0288a8c..0018cc2 100644 --- a/src/reactor.js +++ b/src/reactor.js @@ -24,6 +24,7 @@ import { class Reactor { constructor(config = {}) { const debug = !!config.debug + const useCache = config.useCache === undefined ? true : !!config.useCache const itemsToCache = Number(config.maxItemsToCache) const maxItemsToCache = itemsToCache && itemsToCache > 1 ? itemsToCache : null const baseOptions = debug ? DEBUG_OPTIONS : PROD_OPTIONS @@ -32,6 +33,7 @@ class Reactor { maxItemsToCache: maxItemsToCache, // merge config options with the defaults options: baseOptions.merge(config.options || {}), + useCache: useCache }) this.prevReactorState = initialReactorState @@ -288,6 +290,14 @@ class Reactor { this.__isDispatching = false } } + + /** + * Retrieve cache values + * + */ + getCacheValues() { + return this.reactorState.get('cache') + } } export default toFactory(Reactor) diff --git a/src/reactor/fns.js b/src/reactor/fns.js index 5b9a578..66611d5 100644 --- a/src/reactor/fns.js +++ b/src/reactor/fns.js @@ -381,6 +381,9 @@ export function resetDirtyStores(reactorState) { * @return {Getter} */ function getCacheKey(getter) { + if (getter.__options && getter.__options.cacheKey !== undefined) { + return getter.__options.cacheKey + } return getter } @@ -424,6 +427,16 @@ function isCached(reactorState, keyPathOrGetter) { * @return {ReactorState} */ function cacheValue(reactorState, getter, value) { + const hasGetterUseCacheSpecified = getter.__options && getter.__options.useCache !== undefined + + if (hasGetterUseCacheSpecified) { + if (!getter.__options.useCache) { + return reactorState + } + } else if (!reactorState.get('useCache')) { + return reactorState + } + const cacheKey = getCacheKey(getter) const dispatchId = reactorState.get('dispatchId') const storeDeps = getStoreDeps(getter) diff --git a/src/reactor/records.js b/src/reactor/records.js index fe8028d..f0bc855 100644 --- a/src/reactor/records.js +++ b/src/reactor/records.js @@ -47,6 +47,7 @@ export const ReactorState = Record({ maxItemsToCache: null, // production defaults options: PROD_OPTIONS, + useCache: true, }) export const ObserverState = Record({ diff --git a/tests/getter-tests.js b/tests/getter-tests.js index 93d89f6..869cacf 100644 --- a/tests/getter-tests.js +++ b/tests/getter-tests.js @@ -1,4 +1,4 @@ -import { isGetter, getFlattenedDeps, fromKeyPath } from '../src/getter' +import { isGetter, getFlattenedDeps, fromKeyPath, addGetterOptions } from '../src/getter' import { Set, List, is } from 'immutable' describe('Getter', () => { @@ -32,6 +32,39 @@ describe('Getter', () => { }) }) + describe('#addGetterOptions', () => { + it('should throw an error if not passed a getter', () => { + expect(() => { addGetterOptions('test', {}) }).toThrow() + }) + + it('should throw an error if options are added more than once', () => { + let getter = ['test'] + getter = addGetterOptions(getter, {}) + expect(() => { addGetterOptions(getter, {}) }).toThrow() + }) + + it('should not add options if they are not explicitly supplied or are not valid options', () => { + let getter = ['test'] + getter = addGetterOptions(getter, {}) + expect(getter.__options).toEqual({}) + getter = ['test'] + getter = addGetterOptions(getter, { fakeOption: true }) + expect(getter.__options).toEqual({}) + }) + + it('should add the use cache option', () => { + let getter = ['test'] + getter = addGetterOptions(getter, { useCache: false }) + expect(getter.__options.useCache).toBe(false) + }) + + it('should add the cacheKey option', () => { + let getter = ['test'] + getter = addGetterOptions(getter, { cacheKey: 100 }) + expect(getter.__options.cacheKey).toBe(100) + }) + }) + describe('getFlattenedDeps', function() { describe('when passed the identity getter', () => { it('should return a set with only an empty list', () => { diff --git a/tests/reactor-tests.js b/tests/reactor-tests.js index 6ab808a..9658d50 100644 --- a/tests/reactor-tests.js +++ b/tests/reactor-tests.js @@ -4,6 +4,7 @@ import { getOption, CACHE_CLEAR_RATIO } from '../src/reactor/fns' import { toImmutable } from '../src/immutable-helpers' import { PROD_OPTIONS, DEBUG_OPTIONS } from '../src/reactor/records' import logging from '../src/logging' +import { addGetterOptions } from '../src/getter' describe('Reactor', () => { it('should construct without \'new\'', () => { @@ -11,7 +12,7 @@ describe('Reactor', () => { expect(reactor instanceof Reactor).toBe(true) }) - describe('debug and options flags', () => { + describe('debug, cache, and options flags', () => { it('should create a reactor with PROD_OPTIONS', () => { var reactor = new Reactor() expect(reactor.reactorState.get('debug')).toBe(false) @@ -26,20 +27,6 @@ describe('Reactor', () => { expect(reactor.reactorState.get('maxItemsToCache')).toBe(null) expect(is(reactor.reactorState.get('options'), DEBUG_OPTIONS)).toBe(true) }) - it('should create a reactor that overrides maxItemsToCache', () => { - var reactor = new Reactor({ - maxItemsToCache: 20, - }) - expect(reactor.reactorState.get('maxItemsToCache')).toBe(20) - reactor = new Reactor({ - maxItemsToCache: -20, - }) - expect(reactor.reactorState.get('maxItemsToCache')).toBe(null) - reactor = new Reactor({ - maxItemsToCache: 'abc', - }) - expect(reactor.reactorState.get('maxItemsToCache')).toBe(null) - }) it('should override PROD options', () => { var reactor = new Reactor({ options: { @@ -70,6 +57,28 @@ describe('Reactor', () => { expect(getOption(reactor.reactorState, 'throwOnNonImmutableStore')).toBe(true) expect(getOption(reactor.reactorState, 'throwOnDispatchInDispatch')).toBe(false) }) + it('should create a reactor that overrides maxItemsToCache', () => { + var reactor = new Reactor({ + maxItemsToCache: 20, + }) + expect(reactor.reactorState.get('maxItemsToCache')).toBe(20) + reactor = new Reactor({ + maxItemsToCache: -20, + }) + expect(reactor.reactorState.get('maxItemsToCache')).toBe(null) + reactor = new Reactor({ + maxItemsToCache: 'abc', + }) + expect(reactor.reactorState.get('maxItemsToCache')).toBe(null) + }) + it('should override caching by default', () => { + let reactor = new Reactor({}) + expect(reactor.reactorState.get('useCache')).toBe(true) + reactor = new Reactor({ + useCache: false, + }) + expect(reactor.reactorState.get('useCache')).toBe(false) + }) }) describe('options', () => { @@ -612,13 +621,53 @@ describe('Reactor', () => { expect(subtotalSpy.calls.count()).toEqual(1) }) + it('should not cache if useCache is set to false', () => { + let cacheReactor = new Reactor({ + useCache: true, + }) + cacheReactor.evaluate([['test'], () => 1]) + expect(cacheReactor.getCacheValues().size).toBe(1) + cacheReactor = new Reactor({ + useCache: false, + }) + cacheReactor.evaluate([['test'], () => 1]) + expect(cacheReactor.getCacheValues().size).toBe(0) + }) + + it('should respect individual getter cache settings over global settings', () => { + let cacheReactor = new Reactor({ + useCache: true, + }) + + let getter = addGetterOptions([['test2'], () => 1], { useCache: false }) + cacheReactor.evaluate(getter) + expect(cacheReactor.getCacheValues().size).toBe(0) + + cacheReactor = new Reactor({ + useCache: false, + }) + getter = addGetterOptions([['test2'], () => 1], { useCache: true }) + cacheReactor.evaluate(getter) + expect(cacheReactor.getCacheValues().size).toBe(1) + }) + + it('should use cache key supplied by getter if it is present in options', () => { + let cacheReactor = new Reactor({ + useCache: true, + }) + + let getter = addGetterOptions([['test'], () => 1], { cacheKey: 'test' }) + cacheReactor.evaluate(getter) + expect(Object.keys(cacheReactor.getCacheValues().toJS())).toEqual(['test']) + }) + describe('recency caching', () => { - var getters = Array.from(new Array(100), () => { - var z = Math.random() + var getters = Array.apply(null, new Array(100)).map(() => { return [['price'], function() { - return z + return 1; }] }) + var cacheReactor const maxItemsToCache = 50 @@ -1916,4 +1965,29 @@ describe('Reactor', () => { expect(reactor.evaluate(['counter2'])).toBe(21) }) }) + + describe('#getCacheValues', () => { + let reactor + let store1 + beforeEach(() => { + reactor = new Reactor() + store1 = new Store({ + getInitialState: () => 1, + initialize() { + this.on('increment1', (state) => state + 1) + } + }) + + reactor.registerStores({ + store1: store1, + }) + }) + + it('should return all cached values', () => { + expect(reactor.getCacheValues().toJS()).toEqual({}) + let getter = [['test'], () => 1] + reactor.evaluate(getter) + expect(reactor.getCacheValues().get(getter).get('value')).toEqual(1) + }) + }) }) From 5351733a266b7699a0ccb68f9d61f45f97f64e67 Mon Sep 17 00:00:00 2001 From: Sam Jackson Date: Mon, 25 Jan 2016 12:21:07 -0800 Subject: [PATCH 04/14] add return type to comment --- src/reactor.js | 2 +- src/reactor/fns.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/reactor.js b/src/reactor.js index 0018cc2..c924c78 100644 --- a/src/reactor.js +++ b/src/reactor.js @@ -293,7 +293,7 @@ class Reactor { /** * Retrieve cache values - * + * @returns {Immutable.Map} */ getCacheValues() { return this.reactorState.get('cache') diff --git a/src/reactor/fns.js b/src/reactor/fns.js index 66611d5..9dc8e59 100644 --- a/src/reactor/fns.js +++ b/src/reactor/fns.js @@ -429,6 +429,7 @@ function isCached(reactorState, keyPathOrGetter) { function cacheValue(reactorState, getter, value) { const hasGetterUseCacheSpecified = getter.__options && getter.__options.useCache !== undefined + // Prefer getter settings over reactor settings if (hasGetterUseCacheSpecified) { if (!getter.__options.useCache) { return reactorState From da98531ee4c3f0d24851ef9d630e7f755881f232 Mon Sep 17 00:00:00 2001 From: Sam Jackson Date: Mon, 25 Jan 2016 12:28:46 -0800 Subject: [PATCH 05/14] clear cache values --- src/reactor.js | 7 +++++++ tests/reactor-tests.js | 27 +++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/src/reactor.js b/src/reactor.js index c924c78..8602686 100644 --- a/src/reactor.js +++ b/src/reactor.js @@ -298,6 +298,13 @@ class Reactor { getCacheValues() { return this.reactorState.get('cache') } + + /** + * Clear all cached values + */ + clearCacheValues() { + this.reactorState = this.reactorState.set('cache', Immutable.Map()) + } } export default toFactory(Reactor) diff --git a/tests/reactor-tests.js b/tests/reactor-tests.js index 9658d50..e5b562a 100644 --- a/tests/reactor-tests.js +++ b/tests/reactor-tests.js @@ -1990,4 +1990,31 @@ describe('Reactor', () => { expect(reactor.getCacheValues().get(getter).get('value')).toEqual(1) }) }) + + + describe('#clearCacheValues', () => { + let reactor + let store1 + beforeEach(() => { + reactor = new Reactor() + store1 = new Store({ + getInitialState: () => 1, + initialize() { + this.on('increment1', (state) => state + 1) + } + }) + + reactor.registerStores({ + store1: store1, + }) + }) + + it('should return all cached values', () => { + let getter = [['test'], () => 1] + reactor.evaluate(getter) + expect(reactor.getCacheValues().size).toEqual(1) + reactor.clearCacheValues() + expect(reactor.getCacheValues().toJS()).toEqual({}) + }) + }) }) From 5025e2c7c274d88c6e6b5dca3a8282c31f877438 Mon Sep 17 00:00:00 2001 From: Sam Jackson Date: Wed, 27 Jan 2016 16:26:27 -0800 Subject: [PATCH 06/14] Getter, delete from cache on observe --- src/getter.js | 70 +++++++++++++-------- src/main.js | 4 +- src/reactor.js | 27 +++------ src/reactor/fns.js | 121 +++++++++++++++++++++++-------------- tests/getter-tests.js | 81 ++++++++++++++++++------- tests/reactor-fns-tests.js | 43 ++++++++++--- tests/reactor-tests.js | 70 +++------------------ 7 files changed, 232 insertions(+), 184 deletions(-) diff --git a/src/getter.js b/src/getter.js index 70687be..deb4a2a 100644 --- a/src/getter.js +++ b/src/getter.js @@ -1,7 +1,9 @@ import Immutable, { List } from 'immutable' -import { isFunction, isArray } from './utils' +import { isFunction, isArray, toFactory } from './utils' import { isKeyPath } from './key-path' +const CACHE_OPTIONS = ['default', 'always', 'never'] + /** * Getter helper functions * A getter is an array with the form: @@ -9,6 +11,21 @@ import { isKeyPath } from './key-path' */ const identity = (x) => x +class GetterClass { + constructor(getter, config = {}) { + + if (!isKeyPath(getter) && !isGetter(getter)) { + throw new Error('Getter must be passed a keyPath or Getter') + } + + this.getter = getter + this.cache = CACHE_OPTIONS.indexOf(config.cache) > -1 ? config.cache : 'default' + this.cacheKey = config.cacheKey !== undefined ? config.cacheKey : null + } +} + +const Getter = toFactory(GetterClass) + /** * Checks if something is a getter literal, ex: ['dep1', 'dep2', function(dep1, dep2) {...}] * @param {*} toTest @@ -18,12 +35,22 @@ function isGetter(toTest) { return (isArray(toTest) && isFunction(toTest[toTest.length - 1])) } +/** + * Checks if something is a getter object, ie created with the Getter function + * @param {*} toTest + * @return {boolean} + */ +function isGetterObject(toTest) { + return toTest instanceof GetterClass +} + /** * Returns the compute function from a getter * @param {Getter} getter * @return {function} */ function getComputeFn(getter) { + getter = convertToGetterLiteral(getter) return getter[getter.length - 1] } @@ -33,6 +60,7 @@ function getComputeFn(getter) { * @return {function} */ function getDeps(getter) { + getter = convertToGetterLiteral(getter) return getter.slice(0, getter.length - 1) } @@ -43,6 +71,7 @@ function getDeps(getter) { * @return {Immutable.Set} */ function getFlattenedDeps(getter, existing) { + getter = convertToGetterLiteral(getter) if (!existing) { existing = Immutable.Set() } @@ -83,6 +112,7 @@ function fromKeyPath(keyPath) { * @param {Getter} */ function getStoreDeps(getter) { + getter = convertToGetterLiteral(getter) if (getter.hasOwnProperty('__storeDeps')) { return getter.__storeDeps } @@ -103,32 +133,18 @@ function getStoreDeps(getter) { } /** - * Add override options to a getter - * @param {getter} getter - * @param {object} options - * @param {boolean} options.useCache - * @param {*} options.cacheKey - * @returns {getter} + * If the function is an instance of GetterClass, return the getter property + * @param {*} getter + * returns {*} */ -function addGetterOptions(getter, options) { - if (!isKeyPath(getter) && !isGetter(getter)) { - throw new Error('createGetter must be passed a keyPath or Getter') - } - - if (getter.hasOwnProperty('__options')) { - throw new Error('Cannot reassign options to getter') - } - - getter.__options = {} - - if (options.useCache !== undefined) { - getter.__options.useCache = !!options.useCache - } - - if (options.cacheKey !== undefined) { - getter.__options.cacheKey = options.cacheKey +function convertToGetterLiteral(getter) { + if (isGetterObject(getter)) { + return getter.getter + } else if (isKeyPath(getter) || isGetter(getter)) { + return getter + } else { + throw new Error('Getter must be passed a keyPath or Getter') } - return getter } export default { @@ -138,5 +154,7 @@ export default { getStoreDeps, getDeps, fromKeyPath, - addGetterOptions, + isGetterObject, + Getter, + convertToGetterLiteral, } diff --git a/src/main.js b/src/main.js index 8f9e6fa..784f5f7 100644 --- a/src/main.js +++ b/src/main.js @@ -4,7 +4,7 @@ import Reactor from './reactor' import Immutable from 'immutable' import { toJS, toImmutable, isImmutable } from './immutable-helpers' import { isKeyPath } from './key-path' -import { isGetter, addGetterOptions } from './getter' +import { isGetter, Getter } from './getter' import createReactMixin from './create-react-mixin' export default { @@ -17,5 +17,5 @@ export default { toImmutable, isImmutable, createReactMixin, - addGetterOptions, + Getter, } diff --git a/src/reactor.js b/src/reactor.js index 8602686..09e1a85 100644 --- a/src/reactor.js +++ b/src/reactor.js @@ -2,7 +2,7 @@ import Immutable from 'immutable' import createReactMixin from './create-react-mixin' import * as fns from './reactor/fns' import { isKeyPath } from './key-path' -import { isGetter } from './getter' +import { isGetter, isGetterObject } from './getter' import { toJS } from './immutable-helpers' import { toFactory } from './utils' import { @@ -93,7 +93,9 @@ class Reactor { let { observerState, entry } = fns.addObserver(this.observerState, getter, handler) this.observerState = observerState return () => { - this.observerState = fns.removeObserverByEntry(this.observerState, entry) + let [ observerState, reactorState ] = fns.removeObserverByEntry(this.observerState, this.reactorState, entry) + this.observerState = observerState + this.reactorState = reactorState } } @@ -101,11 +103,13 @@ class Reactor { if (arguments.length === 0) { throw new Error('Must call unobserve with a Getter') } - if (!isGetter(getter) && !isKeyPath(getter)) { + if (!isGetterObject(getter) && !isGetter(getter) && !isKeyPath(getter)) { throw new Error('Must call unobserve with a Getter') } - this.observerState = fns.removeObserver(this.observerState, getter, handler) + let [ observerState, reactorState ] = fns.removeObserver(this.observerState, this.reactorState, getter, handler) + this.observerState = observerState + this.reactorState = reactorState } /** @@ -290,21 +294,6 @@ class Reactor { this.__isDispatching = false } } - - /** - * Retrieve cache values - * @returns {Immutable.Map} - */ - getCacheValues() { - return this.reactorState.get('cache') - } - - /** - * Clear all cached values - */ - clearCacheValues() { - this.reactorState = this.reactorState.set('cache', Immutable.Map()) - } } export default toFactory(Reactor) diff --git a/src/reactor/fns.js b/src/reactor/fns.js index 9dc8e59..63b5a98 100644 --- a/src/reactor/fns.js +++ b/src/reactor/fns.js @@ -2,7 +2,7 @@ import Immutable from 'immutable' import logging from '../logging' import { isImmutableValue } from '../immutable-helpers' import { toImmutable } from '../immutable-helpers' -import { fromKeyPath, getStoreDeps, getComputeFn, getDeps, isGetter } from '../getter' +import { fromKeyPath, getStoreDeps, getComputeFn, getDeps, isGetter, isGetterObject, convertToGetterLiteral } from '../getter' import { isEqual, isKeyPath } from '../key-path' import { each } from '../utils' @@ -224,18 +224,19 @@ export function getOption(reactorState, option) { /** * Use cases - * removeObserver(observerState, []) - * removeObserver(observerState, [], handler) - * removeObserver(observerState, ['keyPath']) - * removeObserver(observerState, ['keyPath'], handler) - * removeObserver(observerState, getter) - * removeObserver(observerState, getter, handler) + * removeObserver(observerState, reactorState, []) + * removeObserver(observerState, reactorState, [], handler) + * removeObserver(observerState, reactorState, ['keyPath']) + * removeObserver(observerState, reactorState, ['keyPath'], handler) + * removeObserver(observerState, reactorState, getter) + * removeObserver(observerState, reactorState, getter, handler) * @param {ObserverState} observerState + * @param {ReactorState} reactorState * @param {KeyPath|Getter} getter * @param {Function} handler - * @return {ObserverState} + * @return {Object} */ -export function removeObserver(observerState, getter, handler) { +export function removeObserver(observerState, reactorState, getter, handler) { const entriesToRemove = observerState.get('observersMap').filter(entry => { // use the getterKey in the case of a keyPath is transformed to a getter in addObserver let entryGetter = entry.get('getterKey') @@ -251,38 +252,49 @@ export function removeObserver(observerState, getter, handler) { return (getter === entryGetter) }) - return observerState.withMutations(map => { - entriesToRemove.forEach(entry => removeObserverByEntry(map, entry)) + + observerState = observerState.withMutations(oState => { + reactorState = reactorState.withMutations(rState => { + entriesToRemove.forEach(entry => { removeObserverByEntry(oState, rState, entry) }) + }) }) + return [ + observerState, + reactorState, + ] } /** * Removes an observer entry by id from the observerState * @param {ObserverState} observerState + * @param {ReactorState} reactorState * @param {Immutable.Map} entry - * @return {ObserverState} + * @return {Object} */ -export function removeObserverByEntry(observerState, entry) { - return observerState.withMutations(map => { - const id = entry.get('id') - const storeDeps = entry.get('storeDeps') - - if (storeDeps.size === 0) { - map.update('any', anyObsevers => anyObsevers.remove(id)) - } else { - storeDeps.forEach(storeId => { - map.updateIn(['stores', storeId], observers => { - if (observers) { - // check for observers being present because reactor.reset() can be called before an unwatch fn - return observers.remove(id) - } - return observers +export function removeObserverByEntry(observerState, reactorState, entry) { + return [ + observerState.withMutations(map => { + const id = entry.get('id') + const storeDeps = entry.get('storeDeps') + + if (storeDeps.size === 0) { + map.update('any', anyObsevers => anyObsevers.remove(id)) + } else { + storeDeps.forEach(storeId => { + map.updateIn(['stores', storeId], observers => { + if (observers) { + // check for observers being present because reactor.reset() can be called before an unwatch fn + return observers.remove(id) + } + return observers + }) }) - }) - } + } - map.removeIn(['observersMap', id]) - }) + map.removeIn(['observersMap', id]) + }), + removeCacheValue(reactorState, entry.get('getterKey')), + ] } /** @@ -309,6 +321,8 @@ export function reset(reactorState) { reactorState.update('storeStates', storeStates => incrementStoreStates(storeStates, storeIds)) resetDirtyStores(reactorState) + reactorState.set('cache', Immutable.Map()) + reactorState.set('cacheRecency', Immutable.OrderedMap()) }) } @@ -326,7 +340,7 @@ export function evaluate(reactorState, keyPathOrGetter) { state.getIn(keyPathOrGetter), reactorState ) - } else if (!isGetter(keyPathOrGetter)) { + } else if (!isGetter(keyPathOrGetter) && !isGetterObject(keyPathOrGetter)) { throw new Error('evaluate must be passed a keyPath or Getter') } @@ -340,9 +354,11 @@ export function evaluate(reactorState, keyPathOrGetter) { ) } + const getter = convertToGetterLiteral(keyPathOrGetter) + // evaluate dependencies - const args = getDeps(keyPathOrGetter).map(dep => evaluate(reactorState, dep).result) - const evaluatedValue = getComputeFn(keyPathOrGetter).apply(null, args) + const args = getDeps(getter).map(dep => evaluate(reactorState, dep).result) + const evaluatedValue = getComputeFn(getter).apply(null, args) return evaluateResult( evaluatedValue, cacheValue(reactorState, keyPathOrGetter, evaluatedValue) @@ -381,8 +397,8 @@ export function resetDirtyStores(reactorState) { * @return {Getter} */ function getCacheKey(getter) { - if (getter.__options && getter.__options.cacheKey !== undefined) { - return getter.__options.cacheKey + if (isGetterObject(getter) && getter.cacheKey !== null) { + return getter.cacheKey } return getter } @@ -427,14 +443,17 @@ function isCached(reactorState, keyPathOrGetter) { * @return {ReactorState} */ function cacheValue(reactorState, getter, value) { - const hasGetterUseCacheSpecified = getter.__options && getter.__options.useCache !== undefined - // Prefer getter settings over reactor settings - if (hasGetterUseCacheSpecified) { - if (!getter.__options.useCache) { - return reactorState - } - } else if (!reactorState.get('useCache')) { + // Check global cache settings + const globalCacheEnabled = !!reactorState.get('useCache') + let useCache = globalCacheEnabled + + // Check cache settings on a getter basis + if (isGetterObject(getter) && getter.cache !== 'default') { + useCache = getter.cache === 'always' ? true : false + } + + if (!useCache) { return reactorState } @@ -469,13 +488,25 @@ function cacheValue(reactorState, getter, value) { }) } +export function removeCacheValue(reactorState, getter) { + if (!isGetterObject(getter) && !isGetter(getter)) { + return reactorState + } + const cacheKey = getCacheKey(getter) + return reactorState.withMutations(rState => { + rState.deleteIn(['cache', cacheKey]) + rState.deleteIn(['cacheRecency', cacheKey]) + }) +} + /** * Readds the key for the item in cache to update recency * @param {ReactorState} reactorState - * @param {cacheKey} cacheKey + * @param {getter} getter * @return {ReactorState} */ -function updateCacheRecency(reactorState, cacheKey) { +function updateCacheRecency(reactorState, getter) { + const cacheKey = getCacheKey(getter) return reactorState.withMutations(state => { state.deleteIn(['cacheRecency', cacheKey]) state.setIn(['cacheRecency', cacheKey], cacheKey) diff --git a/tests/getter-tests.js b/tests/getter-tests.js index 869cacf..d507dd3 100644 --- a/tests/getter-tests.js +++ b/tests/getter-tests.js @@ -1,4 +1,4 @@ -import { isGetter, getFlattenedDeps, fromKeyPath, addGetterOptions } from '../src/getter' +import { isGetter, getFlattenedDeps, fromKeyPath, isGetterObject, Getter, convertToGetterLiteral } from '../src/getter' import { Set, List, is } from 'immutable' describe('Getter', () => { @@ -23,6 +23,31 @@ describe('Getter', () => { }) }) + describe('#isGetterObject', () => { + it('should return false for getter literal and primitives', () => { + expect(isGetterObject(1)).toBe(false) + expect(isGetterObject('foo')).toBe(false) + expect(isGetterObject(false)).toBe(false) + expect(isGetterObject(null)).toBe(false) + expect(isGetterObject([['foo'], (x) => x])).toBe(false) + expect(isGetterObject([['foo', 'bar'], (x) => x])).toBe(false) + }) + + it('should return true for Getter instances', () => { + const getter = Getter(['test']) + expect(isGetterObject(getter)).toBe(true) + }) + }) + + describe('#convertToGetterLiteral', () => { + it('should return supplied getter if not a getter object', () => { + const getter = ['test'] + const getterObject = Getter(getter) + expect(convertToGetterLiteral(getter)).toBe(getter) + expect(convertToGetterLiteral(getterObject)).toBe(getter) + }) + }) + describe('fromKeyPath', () => { it('should throw an Error for a nonvalid KeyPath', () => { var invalidKeypath = 'foo.bar' @@ -32,36 +57,48 @@ describe('Getter', () => { }) }) - describe('#addGetterOptions', () => { + describe('#Getter', () => { it('should throw an error if not passed a getter', () => { - expect(() => { addGetterOptions('test', {}) }).toThrow() + expect(() => { Getter(false) }).toThrow() + }) + + it('should accept a keyPath as a getter argument', () => { + const keyPath = ['test'] + expect(Getter(keyPath).getter).toBe(keyPath) }) - it('should throw an error if options are added more than once', () => { - let getter = ['test'] - getter = addGetterOptions(getter, {}) - expect(() => { addGetterOptions(getter, {}) }).toThrow() + it('should accept a getter as a getter argument', () => { + const getter = ['test', () => 1] + expect(Getter(getter).getter).toBe(getter) + }) + + + it('should use "default" as the default cache option', () => { + const getter = ['test'] + const getterObject = Getter(getter, {}) + expect(getterObject.cache).toBe('default') + const getterObject1 = Getter(getter, { cache: 'fakeOption' }) + expect(getterObject1.cache).toBe('default') }) - it('should not add options if they are not explicitly supplied or are not valid options', () => { - let getter = ['test'] - getter = addGetterOptions(getter, {}) - expect(getter.__options).toEqual({}) - getter = ['test'] - getter = addGetterOptions(getter, { fakeOption: true }) - expect(getter.__options).toEqual({}) + it('should set "always" and "never" as cache options', () => { + const getter = ['test'] + const getterObject = Getter(getter, { cache: 'never' }) + expect(getterObject.cache).toBe('never') + const getterObject1 = Getter(getter, { cache: 'always' }) + expect(getterObject1.cache).toBe('always') }) - it('should add the use cache option', () => { - let getter = ['test'] - getter = addGetterOptions(getter, { useCache: false }) - expect(getter.__options.useCache).toBe(false) + it('should default cacheKey to null', () => { + const getter = ['test'] + const getterObject = Getter(getter, {}) + expect(getterObject.cacheKey).toBe(null) }) - it('should add the cacheKey option', () => { - let getter = ['test'] - getter = addGetterOptions(getter, { cacheKey: 100 }) - expect(getter.__options.cacheKey).toBe(100) + it('should set cacheKey to supplied value', () => { + const getter = ['test'] + const getterObject = Getter(getter, { cacheKey: getter }) + expect(getterObject.cacheKey).toBe(getter) }) }) diff --git a/tests/reactor-fns-tests.js b/tests/reactor-fns-tests.js index dfdd4c3..95c0589 100644 --- a/tests/reactor-fns-tests.js +++ b/tests/reactor-fns-tests.js @@ -1,5 +1,5 @@ /*eslint-disable one-var, comma-dangle*/ -import { Map, Set, is } from 'immutable' +import { Map, Set, is, OrderedMap } from 'immutable' import { Store } from '../src/main' import * as fns from '../src/reactor/fns' import { ReactorState, ObserverState, DEBUG_OPTIONS } from '../src/reactor/records' @@ -355,6 +355,13 @@ describe('reactor fns', () => { }) expect(is(expected, result)).toBe(true) }) + + it('should clear cache', () => { + const resultCache = nextReactorState.get('cache') + const resultCacheRecency = nextReactorState.get('cacheRecency') + expect(resultCache.toJS()).toEqual({}) + expect(resultCacheRecency.toJS()).toEqual({}) + }) }) describe('#addObserver', () => { @@ -471,7 +478,8 @@ describe('reactor fns', () => { }) describe('#removeObserver', () => { - let initialObserverState, nextObserverState, getter1, getter2, handler1, handler2, handler3 + let initialObserverState, initialReactorState, nextObserverState, nextReactorState, + getter1, getter2, handler1, handler2, handler3 beforeEach(() => { handler1 = () => 1 @@ -485,6 +493,14 @@ describe('reactor fns', () => { ] getter2 = [[], x => x] + initialReactorState = new ReactorState() + + // Add some dummy cache values + initialReactorState = initialReactorState.setIn(['cache', getter1], 'test1') + initialReactorState = initialReactorState.setIn(['cacheRecency', getter1], 'test1') + initialReactorState = initialReactorState.setIn(['cache', getter2], 'test2') + initialReactorState = initialReactorState.setIn(['cacheRecency', getter2], 'test2') + const initialObserverState1 = new ObserverState() const result1 = fns.addObserver(initialObserverState1, getter1, handler1) const initialObserverState2 = result1.observerState @@ -496,8 +512,11 @@ describe('reactor fns', () => { describe('when removing by getter', () => { it('should return a new ObserverState with all entries containing the getter removed', () => { - nextObserverState = fns.removeObserver(initialObserverState, getter1) - const expected = Map({ + expect(initialReactorState.getIn(['cache', getter1])).toBe('test1') + expect(initialReactorState.getIn(['cacheRecency', getter1])).toBe('test1') + + let [ observerState, reactorState ] = fns.removeObserver(initialObserverState, initialReactorState, getter1) + const expectedObserverState = Map({ any: Set.of(3), stores: Map({ store1: Set(), @@ -514,14 +533,17 @@ describe('reactor fns', () => { })] ]) }) - const result = nextObserverState - expect(is(expected, result)).toBe(true) + expect(is(expectedObserverState, observerState)).toBe(true) + expect(reactorState.getIn(['cache', getter1])).toBe(undefined) + expect(reactorState.getIn(['cacheRecency', getter1])).toBe(undefined) + expect(reactorState.getIn(['cache', getter2])).toBe('test2') + expect(reactorState.getIn(['cacheRecency', getter2])).toBe('test2') }) }) describe('when removing by getter / handler', () => { it('should return a new ObserverState with all entries containing the getter removed', () => { - nextObserverState = fns.removeObserver(initialObserverState, getter2, handler3) + let [ observerState, reactorState ] = fns.removeObserver(initialObserverState, initialReactorState, getter2, handler3) const expected = Map({ any: Set(), stores: Map({ @@ -546,8 +568,11 @@ describe('reactor fns', () => { })] ]) }) - const result = nextObserverState - expect(is(expected, result)).toBe(true) + expect(is(expected, observerState)).toBe(true) + expect(reactorState.getIn(['cache', getter2])).toBe(undefined) + expect(reactorState.getIn(['cacheRecency', getter2])).toBe(undefined) + expect(reactorState.getIn(['cache', getter1])).toBe('test1') + expect(reactorState.getIn(['cacheRecency', getter1])).toBe('test1') }) }) }) diff --git a/tests/reactor-tests.js b/tests/reactor-tests.js index e5b562a..251635e 100644 --- a/tests/reactor-tests.js +++ b/tests/reactor-tests.js @@ -4,7 +4,7 @@ import { getOption, CACHE_CLEAR_RATIO } from '../src/reactor/fns' import { toImmutable } from '../src/immutable-helpers' import { PROD_OPTIONS, DEBUG_OPTIONS } from '../src/reactor/records' import logging from '../src/logging' -import { addGetterOptions } from '../src/getter' +import { Getter } from '../src/getter' describe('Reactor', () => { it('should construct without \'new\'', () => { @@ -626,12 +626,12 @@ describe('Reactor', () => { useCache: true, }) cacheReactor.evaluate([['test'], () => 1]) - expect(cacheReactor.getCacheValues().size).toBe(1) + expect(cacheReactor.reactorState.get('cache').size).toBe(1) cacheReactor = new Reactor({ useCache: false, }) cacheReactor.evaluate([['test'], () => 1]) - expect(cacheReactor.getCacheValues().size).toBe(0) + expect(cacheReactor.reactorState.get('cache').size).toBe(0) }) it('should respect individual getter cache settings over global settings', () => { @@ -639,16 +639,16 @@ describe('Reactor', () => { useCache: true, }) - let getter = addGetterOptions([['test2'], () => 1], { useCache: false }) + let getter = Getter([['test2'], () => 1], { cache: 'never' }) cacheReactor.evaluate(getter) - expect(cacheReactor.getCacheValues().size).toBe(0) + expect(cacheReactor.reactorState.get('cache').size).toBe(0) cacheReactor = new Reactor({ useCache: false, }) - getter = addGetterOptions([['test2'], () => 1], { useCache: true }) + getter = Getter([['test2'], () => 1], { cache: 'always' }) cacheReactor.evaluate(getter) - expect(cacheReactor.getCacheValues().size).toBe(1) + expect(cacheReactor.reactorState.get('cache').size).toBe(1) }) it('should use cache key supplied by getter if it is present in options', () => { @@ -656,9 +656,9 @@ describe('Reactor', () => { useCache: true, }) - let getter = addGetterOptions([['test'], () => 1], { cacheKey: 'test' }) + let getter = Getter([['test'], () => 1], { cacheKey: 'test' }) cacheReactor.evaluate(getter) - expect(Object.keys(cacheReactor.getCacheValues().toJS())).toEqual(['test']) + expect(Object.keys(cacheReactor.reactorState.get('cache').toJS())).toEqual(['test']) }) describe('recency caching', () => { @@ -1965,56 +1965,4 @@ describe('Reactor', () => { expect(reactor.evaluate(['counter2'])).toBe(21) }) }) - - describe('#getCacheValues', () => { - let reactor - let store1 - beforeEach(() => { - reactor = new Reactor() - store1 = new Store({ - getInitialState: () => 1, - initialize() { - this.on('increment1', (state) => state + 1) - } - }) - - reactor.registerStores({ - store1: store1, - }) - }) - - it('should return all cached values', () => { - expect(reactor.getCacheValues().toJS()).toEqual({}) - let getter = [['test'], () => 1] - reactor.evaluate(getter) - expect(reactor.getCacheValues().get(getter).get('value')).toEqual(1) - }) - }) - - - describe('#clearCacheValues', () => { - let reactor - let store1 - beforeEach(() => { - reactor = new Reactor() - store1 = new Store({ - getInitialState: () => 1, - initialize() { - this.on('increment1', (state) => state + 1) - } - }) - - reactor.registerStores({ - store1: store1, - }) - }) - - it('should return all cached values', () => { - let getter = [['test'], () => 1] - reactor.evaluate(getter) - expect(reactor.getCacheValues().size).toEqual(1) - reactor.clearCacheValues() - expect(reactor.getCacheValues().toJS()).toEqual({}) - }) - }) }) From 28cc5472428b19fd2f968fcb8654ee689e1c2ccd Mon Sep 17 00:00:00 2001 From: Sam Jackson Date: Wed, 27 Jan 2016 16:42:18 -0800 Subject: [PATCH 07/14] comments --- src/reactor/fns.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/reactor/fns.js b/src/reactor/fns.js index 63b5a98..96faa41 100644 --- a/src/reactor/fns.js +++ b/src/reactor/fns.js @@ -252,7 +252,7 @@ export function removeObserver(observerState, reactorState, getter, handler) { return (getter === entryGetter) }) - + // Update both observer and reactor state observerState = observerState.withMutations(oState => { reactorState = reactorState.withMutations(rState => { entriesToRemove.forEach(entry => { removeObserverByEntry(oState, rState, entry) }) @@ -293,6 +293,7 @@ export function removeObserverByEntry(observerState, reactorState, entry) { map.removeIn(['observersMap', id]) }), + // remove cache values for getter removeCacheValue(reactorState, entry.get('getterKey')), ] } @@ -488,7 +489,13 @@ function cacheValue(reactorState, getter, value) { }) } -export function removeCacheValue(reactorState, getter) { +/** + * Remove getter cache value from cache and recency cache + * @param {ReactorState} reactorState + * @param {getter} getter + * @return {ReactorState} + */ +function removeCacheValue(reactorState, getter) { if (!isGetterObject(getter) && !isGetter(getter)) { return reactorState } @@ -529,7 +536,6 @@ function incrementId(reactorState) { return reactorState.update('dispatchId', id => id + 1) } - /** * @param {Immutable.Map} storeStates * @param {Array} storeIds From 372f8b7110128db6a0b9062aef4f749e043bf363 Mon Sep 17 00:00:00 2001 From: Sam Jackson Date: Thu, 28 Jan 2016 10:10:27 -0800 Subject: [PATCH 08/14] more tests, fixes --- src/reactor/fns.js | 21 +++++++++++---------- tests/getter-tests.js | 2 ++ tests/reactor-fns-tests.js | 5 +++-- tests/reactor-tests.js | 24 ++++++++++++++++++++++++ 4 files changed, 40 insertions(+), 12 deletions(-) diff --git a/src/reactor/fns.js b/src/reactor/fns.js index 96faa41..da2bc0e 100644 --- a/src/reactor/fns.js +++ b/src/reactor/fns.js @@ -169,6 +169,8 @@ export function loadState(reactorState, state) { export function addObserver(observerState, getter, handler) { // use the passed in getter as the key so we can rely on a byreference call for unobserve const getterKey = getter + + getter = convertToGetterLiteral(getter) if (isKeyPath(getter)) { getter = fromKeyPath(getter) } @@ -234,19 +236,20 @@ export function getOption(reactorState, option) { * @param {ReactorState} reactorState * @param {KeyPath|Getter} getter * @param {Function} handler - * @return {Object} + * @return {Array} */ export function removeObserver(observerState, reactorState, getter, handler) { const entriesToRemove = observerState.get('observersMap').filter(entry => { // use the getterKey in the case of a keyPath is transformed to a getter in addObserver - let entryGetter = entry.get('getterKey') + const entryGetter = entry.get('getterKey') + const entryGetterLiteral = convertToGetterLiteral(entryGetter) let handlersMatch = (!handler || entry.get('handler') === handler) if (!handlersMatch) { return false } // check for a by-value equality of keypaths - if (isKeyPath(getter) && isKeyPath(entryGetter)) { - return isEqual(getter, entryGetter) + if (isKeyPath(getter) && isKeyPath(entryGetterLiteral)) { + return isEqual(getter, entryGetterLiteral) } // we are comparing two getters do it by reference return (getter === entryGetter) @@ -269,7 +272,7 @@ export function removeObserver(observerState, reactorState, getter, handler) { * @param {ObserverState} observerState * @param {ReactorState} reactorState * @param {Immutable.Map} entry - * @return {Object} + * @return {Array} */ export function removeObserverByEntry(observerState, reactorState, entry) { return [ @@ -334,18 +337,18 @@ export function reset(reactorState) { */ export function evaluate(reactorState, keyPathOrGetter) { const state = reactorState.get('state') + const getter = convertToGetterLiteral(keyPathOrGetter) - if (isKeyPath(keyPathOrGetter)) { + if (isKeyPath(getter)) { // if its a keyPath simply return return evaluateResult( - state.getIn(keyPathOrGetter), + state.getIn(getter), reactorState ) } else if (!isGetter(keyPathOrGetter) && !isGetterObject(keyPathOrGetter)) { throw new Error('evaluate must be passed a keyPath or Getter') } - // Must be a Getter // if the value is cached for this dispatch cycle, return the cached value if (isCached(reactorState, keyPathOrGetter)) { // Cache hit @@ -355,8 +358,6 @@ export function evaluate(reactorState, keyPathOrGetter) { ) } - const getter = convertToGetterLiteral(keyPathOrGetter) - // evaluate dependencies const args = getDeps(getter).map(dep => evaluate(reactorState, dep).result) const evaluatedValue = getComputeFn(getter).apply(null, args) diff --git a/tests/getter-tests.js b/tests/getter-tests.js index d507dd3..8a8abb5 100644 --- a/tests/getter-tests.js +++ b/tests/getter-tests.js @@ -42,8 +42,10 @@ describe('Getter', () => { describe('#convertToGetterLiteral', () => { it('should return supplied getter if not a getter object', () => { const getter = ['test'] + const getter2 = [['test'], x => x] const getterObject = Getter(getter) expect(convertToGetterLiteral(getter)).toBe(getter) + expect(convertToGetterLiteral(getter2)).toBe(getter2) expect(convertToGetterLiteral(getterObject)).toBe(getter) }) }) diff --git a/tests/reactor-fns-tests.js b/tests/reactor-fns-tests.js index 95c0589..08c5be7 100644 --- a/tests/reactor-fns-tests.js +++ b/tests/reactor-fns-tests.js @@ -4,6 +4,7 @@ import { Store } from '../src/main' import * as fns from '../src/reactor/fns' import { ReactorState, ObserverState, DEBUG_OPTIONS } from '../src/reactor/records' import { toImmutable } from '../src/immutable-helpers' +import { Getter, convertToGetterLiteral } from '../src/getter' describe('reactor fns', () => { describe('#registerStores', () => { @@ -491,7 +492,7 @@ describe('reactor fns', () => { ['store2'], (a, b) => a + b ] - getter2 = [[], x => x] + getter2 = Getter([[], x => x]) initialReactorState = new ReactorState() @@ -528,7 +529,7 @@ describe('reactor fns', () => { id: 3, storeDeps: Set(), getterKey: getter2, - getter: getter2, + getter: convertToGetterLiteral(getter2), handler: handler3, })] ]) diff --git a/tests/reactor-tests.js b/tests/reactor-tests.js index 251635e..2416dc6 100644 --- a/tests/reactor-tests.js +++ b/tests/reactor-tests.js @@ -661,6 +661,21 @@ describe('Reactor', () => { expect(Object.keys(cacheReactor.reactorState.get('cache').toJS())).toEqual(['test']) }) + it('should not cache keyPaths', () => { + let cacheReactor = new Reactor({ + useCache: true, + }) + + let getter = ['test'] + cacheReactor.evaluate(getter) + expect(Object.keys(cacheReactor.reactorState.get('cache').toJS())).toEqual([]) + + getter = Getter(['test']) + cacheReactor.evaluate(getter) + expect(Object.keys(cacheReactor.reactorState.get('cache').toJS())).toEqual([]) + + }) + describe('recency caching', () => { var getters = Array.apply(null, new Array(100)).map(() => { return [['price'], function() { @@ -718,6 +733,15 @@ describe('Reactor', () => { expect(mockFn.calls.count()).toEqual(1) expect(mockFn.calls.argsFor(0)).toEqual([5]) }) + it('should observe a Getter object', () => { + var mockFn = jasmine.createSpy() + reactor.observe(Getter(['taxPercent']), mockFn) + + checkoutActions.setTaxPercent(5) + + expect(mockFn.calls.count()).toEqual(1) + expect(mockFn.calls.argsFor(0)).toEqual([5]) + }) it('should not invoke a change handler if another keyPath changes', () => { var mockFn = jasmine.createSpy() reactor.observe(['taxPercent'], mockFn) From fe630e94a0883315d49dec89fb4ff59fe9784a37 Mon Sep 17 00:00:00 2001 From: Sam Jackson Date: Thu, 28 Jan 2016 10:23:54 -0800 Subject: [PATCH 09/14] formatting --- src/getter.js | 4 +--- src/reactor/fns.js | 3 --- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/getter.js b/src/getter.js index deb4a2a..61cf495 100644 --- a/src/getter.js +++ b/src/getter.js @@ -13,11 +13,9 @@ const identity = (x) => x class GetterClass { constructor(getter, config = {}) { - if (!isKeyPath(getter) && !isGetter(getter)) { throw new Error('Getter must be passed a keyPath or Getter') } - this.getter = getter this.cache = CACHE_OPTIONS.indexOf(config.cache) > -1 ? config.cache : 'default' this.cacheKey = config.cacheKey !== undefined ? config.cacheKey : null @@ -143,7 +141,7 @@ function convertToGetterLiteral(getter) { } else if (isKeyPath(getter) || isGetter(getter)) { return getter } else { - throw new Error('Getter must be passed a keyPath or Getter') + throw new Error('convertToGetterLiteral must be passed a keyPath or Getter') } } diff --git a/src/reactor/fns.js b/src/reactor/fns.js index da2bc0e..a93a5d9 100644 --- a/src/reactor/fns.js +++ b/src/reactor/fns.js @@ -497,9 +497,6 @@ function cacheValue(reactorState, getter, value) { * @return {ReactorState} */ function removeCacheValue(reactorState, getter) { - if (!isGetterObject(getter) && !isGetter(getter)) { - return reactorState - } const cacheKey = getCacheKey(getter) return reactorState.withMutations(rState => { rState.deleteIn(['cache', cacheKey]) From 69e6904c5436d55e83fb76ef668acbdcfb437f68 Mon Sep 17 00:00:00 2001 From: Sam Jackson Date: Thu, 28 Jan 2016 10:34:29 -0800 Subject: [PATCH 10/14] adding default max --- src/reactor.js | 14 ++++++++++++-- tests/reactor-tests.js | 5 +++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/reactor.js b/src/reactor.js index 09e1a85..cd136b7 100644 --- a/src/reactor.js +++ b/src/reactor.js @@ -12,6 +12,8 @@ import { PROD_OPTIONS, } from './reactor/records' +export const DEFAULT_MAX_ITEMS_TO_CACHE = 1000 + /** * State is stored in NuclearJS Reactors. Reactors * contain a 'state' object which is an Immutable.Map @@ -25,8 +27,16 @@ class Reactor { constructor(config = {}) { const debug = !!config.debug const useCache = config.useCache === undefined ? true : !!config.useCache - const itemsToCache = Number(config.maxItemsToCache) - const maxItemsToCache = itemsToCache && itemsToCache > 1 ? itemsToCache : null + + // use default # of items in cache + let maxItemsToCache = DEFAULT_MAX_ITEMS_TO_CACHE + + // if user has defined maxItemsToCache, use the number provide or set to null (ie no max) + if (config.maxItemsToCache !== undefined) { + maxItemsToCache = Number(config.maxItemsToCache) + maxItemsToCache = maxItemsToCache && maxItemsToCache > 0 ? maxItemsToCache : null + } + const baseOptions = debug ? DEBUG_OPTIONS : PROD_OPTIONS const initialReactorState = new ReactorState({ debug: debug, diff --git a/tests/reactor-tests.js b/tests/reactor-tests.js index 2416dc6..97edfd9 100644 --- a/tests/reactor-tests.js +++ b/tests/reactor-tests.js @@ -1,5 +1,6 @@ import Immutable, { Map, List, is } from 'immutable' import { Reactor, Store } from '../src/main' +import { DEFAULT_MAX_ITEMS_TO_CACHE } from '../src/reactor' import { getOption, CACHE_CLEAR_RATIO } from '../src/reactor/fns' import { toImmutable } from '../src/immutable-helpers' import { PROD_OPTIONS, DEBUG_OPTIONS } from '../src/reactor/records' @@ -16,7 +17,7 @@ describe('Reactor', () => { it('should create a reactor with PROD_OPTIONS', () => { var reactor = new Reactor() expect(reactor.reactorState.get('debug')).toBe(false) - expect(reactor.reactorState.get('maxItemsToCache')).toBe(null) + expect(reactor.reactorState.get('maxItemsToCache')).toBe(DEFAULT_MAX_ITEMS_TO_CACHE) expect(is(reactor.reactorState.get('options'), PROD_OPTIONS)).toBe(true) }) it('should create a reactor with DEBUG_OPTIONS', () => { @@ -24,7 +25,7 @@ describe('Reactor', () => { debug: true, }) expect(reactor.reactorState.get('debug')).toBe(true) - expect(reactor.reactorState.get('maxItemsToCache')).toBe(null) + expect(reactor.reactorState.get('maxItemsToCache')).toBe(DEFAULT_MAX_ITEMS_TO_CACHE) expect(is(reactor.reactorState.get('options'), DEBUG_OPTIONS)).toBe(true) }) it('should override PROD options', () => { From ce555edcde917b60f95e62eee79eb6b83e07ec96 Mon Sep 17 00:00:00 2001 From: Sam Jackson Date: Thu, 28 Jan 2016 11:23:47 -0800 Subject: [PATCH 11/14] removing duplicate packages --- docs/package.json | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/package.json b/docs/package.json index de8b5a8..8da5885 100644 --- a/docs/package.json +++ b/docs/package.json @@ -23,8 +23,6 @@ "nuclear-js": "^1.0.5", "webpack": "^1.9.11", "webpack-dev-server": "^1.9.0", - "grunt-concurrent": "^1.0.0", - "grunt-contrib-connect": "^0.10.1", "remarkable": "^1.6.0", "front-matter": "^1.0.0", "glob": "^5.0.10", From 2ea41bb010a06e8c3be79a424f2ccf6ca3d15c08 Mon Sep 17 00:00:00 2001 From: Sam Jackson Date: Thu, 28 Jan 2016 11:27:28 -0800 Subject: [PATCH 12/14] fixing lint errors --- src/console-polyfill.js | 16 ++++++++-------- src/create-react-mixin.js | 2 +- src/getter.js | 6 +++--- src/reactor.js | 6 +++--- src/reactor/fns.js | 4 ++-- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/console-polyfill.js b/src/console-polyfill.js index 124dae8..33c531b 100644 --- a/src/console-polyfill.js +++ b/src/console-polyfill.js @@ -1,11 +1,11 @@ try { - if (!(window.console && console.log)) { + if (!(window.console && console.log)) { // eslint-disable-line console = { - log: function(){}, - debug: function(){}, - info: function(){}, - warn: function(){}, - error: function(){} - }; + log: function() {}, + debug: function() {}, + info: function() {}, + warn: function() {}, + error: function() {}, + } } -} catch(e) {} +} catch(e) {} // eslint-disable-line diff --git a/src/create-react-mixin.js b/src/create-react-mixin.js index 9195f72..3965451 100644 --- a/src/create-react-mixin.js +++ b/src/create-react-mixin.js @@ -26,7 +26,7 @@ export default function(reactor) { each(this.getDataBindings(), (getter, key) => { const unwatchFn = reactor.observe(getter, (val) => { this.setState({ - [key]: val + [key]: val, }) }) diff --git a/src/getter.js b/src/getter.js index 61cf495..76a2a4f 100644 --- a/src/getter.js +++ b/src/getter.js @@ -138,11 +138,11 @@ function getStoreDeps(getter) { function convertToGetterLiteral(getter) { if (isGetterObject(getter)) { return getter.getter - } else if (isKeyPath(getter) || isGetter(getter)) { + } + if (isKeyPath(getter) || isGetter(getter)) { return getter - } else { - throw new Error('convertToGetterLiteral must be passed a keyPath or Getter') } + throw new Error('convertToGetterLiteral must be passed a keyPath or Getter') } export default { diff --git a/src/reactor.js b/src/reactor.js index cd136b7..03061af 100644 --- a/src/reactor.js +++ b/src/reactor.js @@ -33,8 +33,8 @@ class Reactor { // if user has defined maxItemsToCache, use the number provide or set to null (ie no max) if (config.maxItemsToCache !== undefined) { - maxItemsToCache = Number(config.maxItemsToCache) - maxItemsToCache = maxItemsToCache && maxItemsToCache > 0 ? maxItemsToCache : null + maxItemsToCache = Number(config.maxItemsToCache) + maxItemsToCache = maxItemsToCache && maxItemsToCache > 0 ? maxItemsToCache : null } const baseOptions = debug ? DEBUG_OPTIONS : PROD_OPTIONS @@ -43,7 +43,7 @@ class Reactor { maxItemsToCache: maxItemsToCache, // merge config options with the defaults options: baseOptions.merge(config.options || {}), - useCache: useCache + useCache: useCache, }) this.prevReactorState = initialReactorState diff --git a/src/reactor/fns.js b/src/reactor/fns.js index a93a5d9..6b5bba4 100644 --- a/src/reactor/fns.js +++ b/src/reactor/fns.js @@ -10,7 +10,7 @@ import { each } from '../utils' * Immutable Types */ const EvaluateResult = Immutable.Record({ result: null, reactorState: null}) -export const CACHE_CLEAR_RATIO = .8; +export const CACHE_CLEAR_RATIO = 0.8 function evaluateResult(result, reactorState) { return new EvaluateResult({ @@ -75,7 +75,7 @@ export function replaceStores(reactorState, stores) { */ export function dispatch(reactorState, actionType, payload) { if (actionType === undefined && getOption(reactorState, 'throwOnUndefinedActionType')) { - throw new Error('`dispatch` cannot be called with an `undefined` action type.'); + throw new Error('`dispatch` cannot be called with an `undefined` action type.') } const currState = reactorState.get('state') From 90b35c677597c25143d99ba7f07c414e43663c81 Mon Sep 17 00:00:00 2001 From: Sam Jackson Date: Mon, 1 Feb 2016 10:04:13 -0800 Subject: [PATCH 13/14] removing Getter object --- src/getter.js | 73 +++++++++++++++++--------------------- src/reactor.js | 4 +-- src/reactor/fns.js | 32 ++++++++--------- tests/getter-tests.js | 70 +++++++++++++----------------------- tests/reactor-fns-tests.js | 4 +-- 5 files changed, 77 insertions(+), 106 deletions(-) diff --git a/src/getter.js b/src/getter.js index 76a2a4f..4123675 100644 --- a/src/getter.js +++ b/src/getter.js @@ -1,5 +1,5 @@ import Immutable, { List } from 'immutable' -import { isFunction, isArray, toFactory } from './utils' +import { isFunction, isArray } from './utils' import { isKeyPath } from './key-path' const CACHE_OPTIONS = ['default', 'always', 'never'] @@ -11,35 +11,48 @@ const CACHE_OPTIONS = ['default', 'always', 'never'] */ const identity = (x) => x -class GetterClass { - constructor(getter, config = {}) { - if (!isKeyPath(getter) && !isGetter(getter)) { - throw new Error('Getter must be passed a keyPath or Getter') - } - this.getter = getter - this.cache = CACHE_OPTIONS.indexOf(config.cache) > -1 ? config.cache : 'default' - this.cacheKey = config.cacheKey !== undefined ? config.cacheKey : null +/** + * Add override options to a getter + * @param {getter} getter + * @param {object} options + * @param {boolean} options.cache + * @param {*} options.cacheKey + * @returns {getter} + */ +function Getter(getter, options={}) { + if (!isKeyPath(getter) && !isGetter(getter)) { + throw new Error('createGetter must be passed a keyPath or Getter') } -} -const Getter = toFactory(GetterClass) + if (getter.hasOwnProperty('__options')) { + throw new Error('Cannot reassign options to getter') + } + + getter.__options = {} + getter.__options.cache = CACHE_OPTIONS.indexOf(options.cache) > -1 ? options.cache : 'default' + getter.__options.cacheKey = options.cacheKey !== undefined ? options.cacheKey : null + return getter +} /** - * Checks if something is a getter literal, ex: ['dep1', 'dep2', function(dep1, dep2) {...}] - * @param {*} toTest - * @return {boolean} + * Retrieve an option from getter options + * @param {getter} getter + * @param {string} Name of option to retrieve + * @returns {*} */ -function isGetter(toTest) { - return (isArray(toTest) && isFunction(toTest[toTest.length - 1])) +function getGetterOption(getter, option) { + if (getter.__options) { + return getter.__options[option] + } } /** - * Checks if something is a getter object, ie created with the Getter function + * Checks if something is a getter literal, ex: ['dep1', 'dep2', function(dep1, dep2) {...}] * @param {*} toTest * @return {boolean} */ -function isGetterObject(toTest) { - return toTest instanceof GetterClass +function isGetter(toTest) { + return (isArray(toTest) && isFunction(toTest[toTest.length - 1])) } /** @@ -48,7 +61,6 @@ function isGetterObject(toTest) { * @return {function} */ function getComputeFn(getter) { - getter = convertToGetterLiteral(getter) return getter[getter.length - 1] } @@ -58,7 +70,6 @@ function getComputeFn(getter) { * @return {function} */ function getDeps(getter) { - getter = convertToGetterLiteral(getter) return getter.slice(0, getter.length - 1) } @@ -69,7 +80,6 @@ function getDeps(getter) { * @return {Immutable.Set} */ function getFlattenedDeps(getter, existing) { - getter = convertToGetterLiteral(getter) if (!existing) { existing = Immutable.Set() } @@ -110,7 +120,6 @@ function fromKeyPath(keyPath) { * @param {Getter} */ function getStoreDeps(getter) { - getter = convertToGetterLiteral(getter) if (getter.hasOwnProperty('__storeDeps')) { return getter.__storeDeps } @@ -130,29 +139,13 @@ function getStoreDeps(getter) { return storeDeps } -/** - * If the function is an instance of GetterClass, return the getter property - * @param {*} getter - * returns {*} - */ -function convertToGetterLiteral(getter) { - if (isGetterObject(getter)) { - return getter.getter - } - if (isKeyPath(getter) || isGetter(getter)) { - return getter - } - throw new Error('convertToGetterLiteral must be passed a keyPath or Getter') -} - export default { isGetter, getComputeFn, getFlattenedDeps, + getGetterOption, getStoreDeps, getDeps, fromKeyPath, - isGetterObject, Getter, - convertToGetterLiteral, } diff --git a/src/reactor.js b/src/reactor.js index 03061af..311fadb 100644 --- a/src/reactor.js +++ b/src/reactor.js @@ -2,7 +2,7 @@ import Immutable from 'immutable' import createReactMixin from './create-react-mixin' import * as fns from './reactor/fns' import { isKeyPath } from './key-path' -import { isGetter, isGetterObject } from './getter' +import { isGetter } from './getter' import { toJS } from './immutable-helpers' import { toFactory } from './utils' import { @@ -113,7 +113,7 @@ class Reactor { if (arguments.length === 0) { throw new Error('Must call unobserve with a Getter') } - if (!isGetterObject(getter) && !isGetter(getter) && !isKeyPath(getter)) { + if (!isGetter(getter) && !isKeyPath(getter)) { throw new Error('Must call unobserve with a Getter') } diff --git a/src/reactor/fns.js b/src/reactor/fns.js index 6b5bba4..76af76a 100644 --- a/src/reactor/fns.js +++ b/src/reactor/fns.js @@ -2,7 +2,7 @@ import Immutable from 'immutable' import logging from '../logging' import { isImmutableValue } from '../immutable-helpers' import { toImmutable } from '../immutable-helpers' -import { fromKeyPath, getStoreDeps, getComputeFn, getDeps, isGetter, isGetterObject, convertToGetterLiteral } from '../getter' +import { fromKeyPath, getStoreDeps, getComputeFn, getDeps, isGetter, getGetterOption } from '../getter' import { isEqual, isKeyPath } from '../key-path' import { each } from '../utils' @@ -170,7 +170,6 @@ export function addObserver(observerState, getter, handler) { // use the passed in getter as the key so we can rely on a byreference call for unobserve const getterKey = getter - getter = convertToGetterLiteral(getter) if (isKeyPath(getter)) { getter = fromKeyPath(getter) } @@ -242,14 +241,13 @@ export function removeObserver(observerState, reactorState, getter, handler) { const entriesToRemove = observerState.get('observersMap').filter(entry => { // use the getterKey in the case of a keyPath is transformed to a getter in addObserver const entryGetter = entry.get('getterKey') - const entryGetterLiteral = convertToGetterLiteral(entryGetter) let handlersMatch = (!handler || entry.get('handler') === handler) if (!handlersMatch) { return false } // check for a by-value equality of keypaths - if (isKeyPath(getter) && isKeyPath(entryGetterLiteral)) { - return isEqual(getter, entryGetterLiteral) + if (isKeyPath(getter) && isKeyPath(entryGetter)) { + return isEqual(getter, entryGetter) } // we are comparing two getters do it by reference return (getter === entryGetter) @@ -337,15 +335,14 @@ export function reset(reactorState) { */ export function evaluate(reactorState, keyPathOrGetter) { const state = reactorState.get('state') - const getter = convertToGetterLiteral(keyPathOrGetter) - if (isKeyPath(getter)) { + if (isKeyPath(keyPathOrGetter)) { // if its a keyPath simply return return evaluateResult( - state.getIn(getter), + state.getIn(keyPathOrGetter), reactorState ) - } else if (!isGetter(keyPathOrGetter) && !isGetterObject(keyPathOrGetter)) { + } else if (!isGetter(keyPathOrGetter)) { throw new Error('evaluate must be passed a keyPath or Getter') } @@ -359,8 +356,8 @@ export function evaluate(reactorState, keyPathOrGetter) { } // evaluate dependencies - const args = getDeps(getter).map(dep => evaluate(reactorState, dep).result) - const evaluatedValue = getComputeFn(getter).apply(null, args) + const args = getDeps(keyPathOrGetter).map(dep => evaluate(reactorState, dep).result) + const evaluatedValue = getComputeFn(keyPathOrGetter).apply(null, args) return evaluateResult( evaluatedValue, cacheValue(reactorState, keyPathOrGetter, evaluatedValue) @@ -399,10 +396,8 @@ export function resetDirtyStores(reactorState) { * @return {Getter} */ function getCacheKey(getter) { - if (isGetterObject(getter) && getter.cacheKey !== null) { - return getter.cacheKey - } - return getter + const getterOption = getGetterOption(getter, 'cacheKey') + return getterOption ? getterOption : getter } /** @@ -451,8 +446,11 @@ function cacheValue(reactorState, getter, value) { let useCache = globalCacheEnabled // Check cache settings on a getter basis - if (isGetterObject(getter) && getter.cache !== 'default') { - useCache = getter.cache === 'always' ? true : false + const getterCacheOption = getGetterOption(getter, 'cache') + if (getterCacheOption === 'always') { + useCache = true + } else if (getterCacheOption === 'never') { + useCache = false } if (!useCache) { diff --git a/tests/getter-tests.js b/tests/getter-tests.js index 8a8abb5..baa4289 100644 --- a/tests/getter-tests.js +++ b/tests/getter-tests.js @@ -1,4 +1,4 @@ -import { isGetter, getFlattenedDeps, fromKeyPath, isGetterObject, Getter, convertToGetterLiteral } from '../src/getter' +import { isGetter, getFlattenedDeps, fromKeyPath, getGetterOption, Getter } from '../src/getter' import { Set, List, is } from 'immutable' describe('Getter', () => { @@ -23,33 +23,6 @@ describe('Getter', () => { }) }) - describe('#isGetterObject', () => { - it('should return false for getter literal and primitives', () => { - expect(isGetterObject(1)).toBe(false) - expect(isGetterObject('foo')).toBe(false) - expect(isGetterObject(false)).toBe(false) - expect(isGetterObject(null)).toBe(false) - expect(isGetterObject([['foo'], (x) => x])).toBe(false) - expect(isGetterObject([['foo', 'bar'], (x) => x])).toBe(false) - }) - - it('should return true for Getter instances', () => { - const getter = Getter(['test']) - expect(isGetterObject(getter)).toBe(true) - }) - }) - - describe('#convertToGetterLiteral', () => { - it('should return supplied getter if not a getter object', () => { - const getter = ['test'] - const getter2 = [['test'], x => x] - const getterObject = Getter(getter) - expect(convertToGetterLiteral(getter)).toBe(getter) - expect(convertToGetterLiteral(getter2)).toBe(getter2) - expect(convertToGetterLiteral(getterObject)).toBe(getter) - }) - }) - describe('fromKeyPath', () => { it('should throw an Error for a nonvalid KeyPath', () => { var invalidKeypath = 'foo.bar' @@ -59,6 +32,16 @@ describe('Getter', () => { }) }) + describe('#getGetterOption', () => { + it('should return undefined if options are not set, or an unrecognized option is requested', () => { + expect(getGetterOption(['test'], 'cache')).toBe(undefined) + expect(getGetterOption(Getter(['test'], { cache: 'always'}), 'fakeOption').toBe(undefined) + }) + it('should return the value of the requested option', () => { + expect(getGetterOption(Getter(['test'], { cache: 'always'}), 'cache').toBe('always') + }) + }) + describe('#Getter', () => { it('should throw an error if not passed a getter', () => { expect(() => { Getter(false) }).toThrow() @@ -66,41 +49,38 @@ describe('Getter', () => { it('should accept a keyPath as a getter argument', () => { const keyPath = ['test'] - expect(Getter(keyPath).getter).toBe(keyPath) + expect(Getter(keyPath)).toBe(keyPath) }) it('should accept a getter as a getter argument', () => { const getter = ['test', () => 1] - expect(Getter(getter).getter).toBe(getter) + expect(Getter(getter)).toBe(getter) }) it('should use "default" as the default cache option', () => { - const getter = ['test'] - const getterObject = Getter(getter, {}) - expect(getterObject.cache).toBe('default') - const getterObject1 = Getter(getter, { cache: 'fakeOption' }) - expect(getterObject1.cache).toBe('default') + const getterObject = Getter(['test'], {}) + expect(getGetterOption(getterObject, 'cache')).toBe('default') + const getterObject1 = Getter(['test'], { cache: 'fakeOption' }) + expect(getGetterOption(getterObject, 'cache')).toBe('default') }) it('should set "always" and "never" as cache options', () => { - const getter = ['test'] - const getterObject = Getter(getter, { cache: 'never' }) - expect(getterObject.cache).toBe('never') - const getterObject1 = Getter(getter, { cache: 'always' }) - expect(getterObject1.cache).toBe('always') + const getterObject = Getter(['test'], { cache: 'never' }) + expect(getGetterOption(getterObject, 'cache')).toBe('never') + const getterObject1 = Getter(['test'], { cache: 'always' }) + expect(getGetterOption(getterObject1, 'cache')).toBe('always') }) it('should default cacheKey to null', () => { - const getter = ['test'] - const getterObject = Getter(getter, {}) - expect(getterObject.cacheKey).toBe(null) + const getterObject = Getter(['test'], {}) + expect(getGetterOption(getterObject, 'cacheKey')).toBe(null) }) it('should set cacheKey to supplied value', () => { const getter = ['test'] - const getterObject = Getter(getter, { cacheKey: getter }) - expect(getterObject.cacheKey).toBe(getter) + const getterObject = Getter(getter, { cacheKey: 'test' }) + expect(getGetterOption(getterObject, 'cacheKey')).toBe('test') }) }) diff --git a/tests/reactor-fns-tests.js b/tests/reactor-fns-tests.js index 08c5be7..b0d9c21 100644 --- a/tests/reactor-fns-tests.js +++ b/tests/reactor-fns-tests.js @@ -4,7 +4,7 @@ import { Store } from '../src/main' import * as fns from '../src/reactor/fns' import { ReactorState, ObserverState, DEBUG_OPTIONS } from '../src/reactor/records' import { toImmutable } from '../src/immutable-helpers' -import { Getter, convertToGetterLiteral } from '../src/getter' +import { Getter } from '../src/getter' describe('reactor fns', () => { describe('#registerStores', () => { @@ -529,7 +529,7 @@ describe('reactor fns', () => { id: 3, storeDeps: Set(), getterKey: getter2, - getter: convertToGetterLiteral(getter2), + getter: getter2, handler: handler3, })] ]) From 5aef5eb9ba8853e072a84a9f45f5bbd56dc2dc5c Mon Sep 17 00:00:00 2001 From: Sam Jackson Date: Mon, 1 Feb 2016 11:55:17 -0800 Subject: [PATCH 14/14] fixing lint errors --- src/reactor.js | 6 +-- src/reactor/fns.js | 107 +++++++++++++++++++++++-------------- tests/getter-tests.js | 6 +-- tests/reactor-fns-tests.js | 70 +++++++++++++++++++++--- tests/reactor-tests.js | 65 +++++++++++----------- 5 files changed, 165 insertions(+), 89 deletions(-) diff --git a/src/reactor.js b/src/reactor.js index 311fadb..4066bb4 100644 --- a/src/reactor.js +++ b/src/reactor.js @@ -34,7 +34,7 @@ class Reactor { // if user has defined maxItemsToCache, use the number provide or set to null (ie no max) if (config.maxItemsToCache !== undefined) { maxItemsToCache = Number(config.maxItemsToCache) - maxItemsToCache = maxItemsToCache && maxItemsToCache > 0 ? maxItemsToCache : null + maxItemsToCache = maxItemsToCache > 0 ? maxItemsToCache : null } const baseOptions = debug ? DEBUG_OPTIONS : PROD_OPTIONS @@ -103,7 +103,7 @@ class Reactor { let { observerState, entry } = fns.addObserver(this.observerState, getter, handler) this.observerState = observerState return () => { - let [ observerState, reactorState ] = fns.removeObserverByEntry(this.observerState, this.reactorState, entry) + let { observerState, reactorState } = fns.removeObserverByEntry(this.observerState, this.reactorState, entry) this.observerState = observerState this.reactorState = reactorState } @@ -117,7 +117,7 @@ class Reactor { throw new Error('Must call unobserve with a Getter') } - let [ observerState, reactorState ] = fns.removeObserver(this.observerState, this.reactorState, getter, handler) + const { observerState, reactorState } = fns.removeObserver(this.observerState, this.reactorState, getter, handler) this.observerState = observerState this.reactorState = reactorState } diff --git a/src/reactor/fns.js b/src/reactor/fns.js index 76af76a..985ef81 100644 --- a/src/reactor/fns.js +++ b/src/reactor/fns.js @@ -238,13 +238,44 @@ export function getOption(reactorState, option) { * @return {Array} */ export function removeObserver(observerState, reactorState, getter, handler) { - const entriesToRemove = observerState.get('observersMap').filter(entry => { + let entriesToRemove = getAllObserversForGetter(observerState, getter) + const originalEntriesToRemoveCount = entriesToRemove.size + if (handler) { + entriesToRemove = entriesToRemove.filter(entry => { + return entry.get('handler') === handler + }) + } + + // If a handler was specified, only clear cache if ALL entries for a getter have that handler + const shouldClearCache = originalEntriesToRemoveCount === entriesToRemove.size + + // Update both observer and reactor state + observerState = observerState.withMutations(oState => { + reactorState = reactorState.withMutations(rState => { + + entriesToRemove.forEach(entry => { removeObserverByEntry(oState, rState, entry, false) }) + if (shouldClearCache) { + removeCacheValue(rState, getter) + } + }) + }) + return { + observerState, + reactorState, + } +} + +/** + * Retrieve all observer entries that match a given getter + * @param {ObserverState} observerState + * @param {Getter} getter + * @returns {Immutable.Map} + */ +export function getAllObserversForGetter(observerState, getter) { + return observerState.get('observersMap').filter(entry => { // use the getterKey in the case of a keyPath is transformed to a getter in addObserver const entryGetter = entry.get('getterKey') - let handlersMatch = (!handler || entry.get('handler') === handler) - if (!handlersMatch) { - return false - } + // check for a by-value equality of keypaths if (isKeyPath(getter) && isKeyPath(entryGetter)) { return isEqual(getter, entryGetter) @@ -252,17 +283,6 @@ export function removeObserver(observerState, reactorState, getter, handler) { // we are comparing two getters do it by reference return (getter === entryGetter) }) - - // Update both observer and reactor state - observerState = observerState.withMutations(oState => { - reactorState = reactorState.withMutations(rState => { - entriesToRemove.forEach(entry => { removeObserverByEntry(oState, rState, entry) }) - }) - }) - return [ - observerState, - reactorState, - ] } /** @@ -272,31 +292,38 @@ export function removeObserver(observerState, reactorState, getter, handler) { * @param {Immutable.Map} entry * @return {Array} */ -export function removeObserverByEntry(observerState, reactorState, entry) { - return [ - observerState.withMutations(map => { - const id = entry.get('id') - const storeDeps = entry.get('storeDeps') - - if (storeDeps.size === 0) { - map.update('any', anyObsevers => anyObsevers.remove(id)) - } else { - storeDeps.forEach(storeId => { - map.updateIn(['stores', storeId], observers => { - if (observers) { - // check for observers being present because reactor.reset() can be called before an unwatch fn - return observers.remove(id) - } - return observers - }) +export function removeObserverByEntry(observerState, reactorState, entry, attemptToClearCache=true) { + + // Only clear cache values for a getter if there is only one entry for it + if (attemptToClearCache && getAllObserversForGetter(observerState, entry.get('getterKey')).size === 1) { + reactorState = removeCacheValue(reactorState, entry.get('getterKey')) + } + + observerState = observerState.withMutations(map => { + const id = entry.get('id') + const storeDeps = entry.get('storeDeps') + + if (storeDeps.size === 0) { + map.update('any', anyObsevers => anyObsevers.remove(id)) + } else { + storeDeps.forEach(storeId => { + map.updateIn(['stores', storeId], observers => { + if (observers) { + // check for observers being present because reactor.reset() can be called before an unwatch fn + return observers.remove(id) + } + return observers }) - } + }) + } - map.removeIn(['observersMap', id]) - }), - // remove cache values for getter - removeCacheValue(reactorState, entry.get('getterKey')), - ] + map.removeIn(['observersMap', id]) + }) + + return { + observerState, + reactorState, + } } /** @@ -471,7 +498,7 @@ function cacheValue(reactorState, getter, value) { const itemsToCache = maxItemsToCache * CACHE_CLEAR_RATIO return reactorState.withMutations(state => { - if (maxItemsToCache && maxItemsToCache <= state.get('cache').size) { + if (maxItemsToCache <= state.get('cache').size) { do { let key = state.get('cacheRecency').first() state.deleteIn(['cache', key]) diff --git a/tests/getter-tests.js b/tests/getter-tests.js index baa4289..aedd8b7 100644 --- a/tests/getter-tests.js +++ b/tests/getter-tests.js @@ -35,10 +35,10 @@ describe('Getter', () => { describe('#getGetterOption', () => { it('should return undefined if options are not set, or an unrecognized option is requested', () => { expect(getGetterOption(['test'], 'cache')).toBe(undefined) - expect(getGetterOption(Getter(['test'], { cache: 'always'}), 'fakeOption').toBe(undefined) + expect(getGetterOption(Getter(['test'], { cache: 'always'}), 'fakeOption')).toBe(undefined) }) it('should return the value of the requested option', () => { - expect(getGetterOption(Getter(['test'], { cache: 'always'}), 'cache').toBe('always') + expect(getGetterOption(Getter(['test'], { cache: 'always'}), 'cache')).toBe('always') }) }) @@ -62,7 +62,7 @@ describe('Getter', () => { const getterObject = Getter(['test'], {}) expect(getGetterOption(getterObject, 'cache')).toBe('default') const getterObject1 = Getter(['test'], { cache: 'fakeOption' }) - expect(getGetterOption(getterObject, 'cache')).toBe('default') + expect(getGetterOption(getterObject1, 'cache')).toBe('default') }) it('should set "always" and "never" as cache options', () => { diff --git a/tests/reactor-fns-tests.js b/tests/reactor-fns-tests.js index b0d9c21..3a52d5f 100644 --- a/tests/reactor-fns-tests.js +++ b/tests/reactor-fns-tests.js @@ -1,8 +1,8 @@ /*eslint-disable one-var, comma-dangle*/ -import { Map, Set, is, OrderedMap } from 'immutable' +import { Map, Set, is } from 'immutable' import { Store } from '../src/main' import * as fns from '../src/reactor/fns' -import { ReactorState, ObserverState, DEBUG_OPTIONS } from '../src/reactor/records' +import { ReactorState, ObserverState } from '../src/reactor/records' import { toImmutable } from '../src/immutable-helpers' import { Getter } from '../src/getter' @@ -479,8 +479,7 @@ describe('reactor fns', () => { }) describe('#removeObserver', () => { - let initialObserverState, initialReactorState, nextObserverState, nextReactorState, - getter1, getter2, handler1, handler2, handler3 + let initialObserverState, initialReactorState, getter1, getter2, handler1, handler2, handler3 beforeEach(() => { handler1 = () => 1 @@ -515,8 +514,7 @@ describe('reactor fns', () => { it('should return a new ObserverState with all entries containing the getter removed', () => { expect(initialReactorState.getIn(['cache', getter1])).toBe('test1') expect(initialReactorState.getIn(['cacheRecency', getter1])).toBe('test1') - - let [ observerState, reactorState ] = fns.removeObserver(initialObserverState, initialReactorState, getter1) + let { observerState, reactorState } = fns.removeObserver(initialObserverState, initialReactorState, getter1) const expectedObserverState = Map({ any: Set.of(3), stores: Map({ @@ -544,7 +542,7 @@ describe('reactor fns', () => { describe('when removing by getter / handler', () => { it('should return a new ObserverState with all entries containing the getter removed', () => { - let [ observerState, reactorState ] = fns.removeObserver(initialObserverState, initialReactorState, getter2, handler3) + let { observerState, reactorState } = fns.removeObserver(initialObserverState, initialReactorState, getter2, handler3) const expected = Map({ any: Set(), stores: Map({ @@ -575,8 +573,64 @@ describe('reactor fns', () => { expect(reactorState.getIn(['cache', getter1])).toBe('test1') expect(reactorState.getIn(['cacheRecency', getter1])).toBe('test1') }) + it('should not clear cache if more there are multiple handlers for the same getter', () => { + let { reactorState } = fns.removeObserver(initialObserverState, initialReactorState, getter1, handler2) + expect(reactorState.getIn(['cache', getter2])).toBe('test2') + expect(reactorState.getIn(['cacheRecency', getter2])).toBe('test2') + expect(reactorState.getIn(['cache', getter1])).toBe('test1') + expect(reactorState.getIn(['cacheRecency', getter1])).toBe('test1') + + }) }) }) + + + describe('#removeObserverByEntry', () => { + let initialObserverState, initialReactorState, getter1, handler1, handler2, entry1, entry2 + + beforeEach(() => { + handler1 = () => 1 + handler2 = () => 2 + + getter1 = [ + ['store1'], + ['store2'], + (a, b) => a + b + ] + + initialReactorState = new ReactorState() + + // Add some dummy cache values + initialReactorState = initialReactorState.setIn(['cache', getter1], 'test1') + initialReactorState = initialReactorState.setIn(['cacheRecency', getter1], 'test1') + + const initialObserverState1 = new ObserverState() + const result1 = fns.addObserver(initialObserverState1, getter1, handler1) + const initialObserverState2 = result1.observerState + const result2 = fns.addObserver(initialObserverState2, getter1, handler2) + initialObserverState = result2.observerState + entry1 = result1.entry + entry2 = result2.entry + }) + + it('should should not clear cache if there is only one entry associated with a getter', () => { + expect(initialReactorState.getIn(['cache', getter1])).toBe('test1') + expect(initialReactorState.getIn(['cacheRecency', getter1])).toBe('test1') + let { reactorState } = fns.removeObserverByEntry(initialObserverState, initialReactorState, entry1, true) + expect(reactorState.getIn(['cache', getter1])).toBe('test1') + expect(reactorState.getIn(['cacheRecency', getter1])).toBe('test1') + }) + + it('should should clear cache if there is more than one entry associated with a getter', () => { + expect(initialReactorState.getIn(['cache', getter1])).toBe('test1') + expect(initialReactorState.getIn(['cacheRecency', getter1])).toBe('test1') + let { observerState, reactorState } = fns.removeObserverByEntry(initialObserverState, initialReactorState, entry1, true) + let { reactorState: reactorState2 } = fns.removeObserverByEntry(observerState, reactorState, entry2, true) + expect(reactorState2.getIn(['cache', getter1])).toBe(undefined) + expect(reactorState2.getIn(['cacheRecency', getter1])).toBe(undefined) + }) + }) + describe('#getDebugOption', () => { it('should parse the option value in a reactorState', () => { const reactorState = new ReactorState({ @@ -597,7 +651,7 @@ describe('reactor fns', () => { }) expect(function() { - const result = fns.getOption(reactorState, 'unknownOption') + fns.getOption(reactorState, 'unknownOption') }).toThrow() }) }) diff --git a/tests/reactor-tests.js b/tests/reactor-tests.js index 97edfd9..48913d6 100644 --- a/tests/reactor-tests.js +++ b/tests/reactor-tests.js @@ -32,7 +32,7 @@ describe('Reactor', () => { var reactor = new Reactor({ options: { logDispatches: true, - } + }, }) expect(getOption(reactor.reactorState, 'logDispatches')).toBe(true) expect(getOption(reactor.reactorState, 'logAppState')).toBe(false) @@ -48,7 +48,7 @@ describe('Reactor', () => { options: { logDispatches: false, throwOnDispatchInDispatch: false, - } + }, }) expect(getOption(reactor.reactorState, 'logDispatches')).toBe(false) expect(getOption(reactor.reactorState, 'logAppState')).toBe(true) @@ -126,7 +126,7 @@ describe('Reactor', () => { initialize() { this.on('action', () => undefined) }, - }) + }), }) reactor.dispatch('action') reactor.reset() @@ -149,7 +149,7 @@ describe('Reactor', () => { initialize() { this.on('action', () => undefined) }, - }) + }), }) }).toThrow() }) @@ -170,7 +170,7 @@ describe('Reactor', () => { initialize() { this.on('action', () => undefined) }, - }) + }), }) }).toThrow() }) @@ -190,8 +190,8 @@ describe('Reactor', () => { }, handleReset() { return undefined - } - }) + }, + }), }) reactor.reset() }).toThrow() @@ -215,7 +215,7 @@ describe('Reactor', () => { handleReset() { return { foo: 'baz' } }, - }) + }), }) reactor.reset() }).not.toThrow() @@ -234,7 +234,7 @@ describe('Reactor', () => { getInitialState() { return { foo: 'bar' } }, - }) + }), }) }).toThrow() }) @@ -255,7 +255,7 @@ describe('Reactor', () => { handleReset() { return { foo: 'baz' } }, - }) + }), }) reactor.reset() }).toThrow() @@ -278,8 +278,8 @@ describe('Reactor', () => { }, initialize() { this.on('increment', curr => curr + 1) - } - }) + }, + }), }) reactor.observe(['count'], (val) => { @@ -307,8 +307,8 @@ describe('Reactor', () => { }, initialize() { this.on('increment', curr => curr + 1) - } - }) + }, + }), }) reactor.observe(['count'], (val) => { @@ -679,9 +679,7 @@ describe('Reactor', () => { describe('recency caching', () => { var getters = Array.apply(null, new Array(100)).map(() => { - return [['price'], function() { - return 1; - }] + return [['price'], () => 1] }) var cacheReactor @@ -689,7 +687,7 @@ describe('Reactor', () => { beforeEach(() => { cacheReactor = new Reactor({ - maxItemsToCache: maxItemsToCache + maxItemsToCache: maxItemsToCache, }) }) @@ -831,8 +829,8 @@ describe('Reactor', () => { test: Store({ getInitialState() { return 1 - } - }) + }, + }), }) expect(mockFn.calls.count()).toEqual(1) @@ -853,24 +851,24 @@ describe('Reactor', () => { }, initialize() { this.on('increment', (state) => state + 1) - } - }) + }, + }), }) // it should call the observer after the store has been registered expect(mockFn.calls.count()).toEqual(1) var observedValue = mockFn.calls.argsFor(0)[0] var expectedHandlerValue = Map({ - test: 1 + test: 1, }) expect(is(observedValue, expectedHandlerValue)).toBe(true) // it should call the observer again when the store handles an action reactor.dispatch('increment') expect(mockFn.calls.count()).toEqual(2) - var observedValue = mockFn.calls.argsFor(1)[0] - var expectedHandlerValue = Map({ - test: 2 + observedValue = mockFn.calls.argsFor(1)[0] + expectedHandlerValue = Map({ + test: 2, }) expect(is(observedValue, expectedHandlerValue)).toBe(true) }) @@ -964,14 +962,12 @@ describe('Reactor', () => { var mockFn1 = jasmine.createSpy() var mockFn2 = jasmine.createSpy() var unwatchFn2 - var unwatchFn1 = reactor.observe(subtotalGetter, (val) => { - console.log('hanlder1', unwatchFn2) + reactor.observe(subtotalGetter, (val) => { unwatchFn2() mockFn1(val) }) unwatchFn2 = reactor.observe(subtotalGetter, (val) => { - console.log('handler2') mockFn2(val) }) @@ -1913,7 +1909,6 @@ describe('Reactor', () => { describe('#replaceStores', () => { let counter1Store let counter2Store - let counter1StoreReplaced let reactor beforeEach(() => { @@ -1922,13 +1917,13 @@ describe('Reactor', () => { getInitialState: () => 1, initialize() { this.on('increment1', (state) => state + 1) - } + }, }) counter2Store = new Store({ getInitialState: () => 1, initialize() { this.on('increment2', (state) => state + 1) - } + }, }) reactor.registerStores({ @@ -1942,7 +1937,7 @@ describe('Reactor', () => { getInitialState: () => 1, initialize() { this.on('increment1', (state) => state + 10) - } + }, }) expect(reactor.evaluate(['counter1'])).toBe(1) @@ -1964,13 +1959,13 @@ describe('Reactor', () => { getInitialState: () => 1, initialize() { this.on('increment1', (state) => state + 10) - } + }, }) let newStore2 = new Store({ getInitialState: () => 1, initialize() { this.on('increment2', (state) => state + 20) - } + }, }) expect(reactor.evaluate(['counter1'])).toBe(1)