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", 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 aece455..4123675 100644 --- a/src/getter.js +++ b/src/getter.js @@ -2,6 +2,8 @@ import Immutable, { List } from 'immutable' import { isFunction, isArray } 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,41 @@ import { isKeyPath } from './key-path' */ const identity = (x) => x +/** + * 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') + } + + 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 +} + +/** + * Retrieve an option from getter options + * @param {getter} getter + * @param {string} Name of option to retrieve + * @returns {*} + */ +function getGetterOption(getter, option) { + if (getter.__options) { + return getter.__options[option] + } +} + /** * Checks if something is a getter literal, ex: ['dep1', 'dep2', function(dep1, dep2) {...}] * @param {*} toTest @@ -106,7 +143,9 @@ export default { isGetter, getComputeFn, getFlattenedDeps, + getGetterOption, getStoreDeps, getDeps, fromKeyPath, + Getter, } diff --git a/src/main.js b/src/main.js index 3e0ebc1..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 } from './getter' +import { isGetter, Getter } from './getter' import createReactMixin from './create-react-mixin' export default { @@ -17,4 +17,5 @@ export default { toImmutable, isImmutable, createReactMixin, + Getter, } diff --git a/src/reactor.js b/src/reactor.js index ce46ecb..4066bb4 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 @@ -24,11 +26,24 @@ import { class Reactor { constructor(config = {}) { const debug = !!config.debug + const useCache = config.useCache === undefined ? true : !!config.useCache + + // 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 > 0 ? maxItemsToCache : 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 || {}), + useCache: useCache, }) this.prevReactorState = initialReactorState @@ -88,7 +103,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 } } @@ -100,7 +117,9 @@ class Reactor { throw new Error('Must call unobserve with a Getter') } - this.observerState = fns.removeObserver(this.observerState, 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 7f47527..985ef81 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, getGetterOption } from '../getter' import { isEqual, isKeyPath } from '../key-path' import { each } from '../utils' @@ -10,6 +10,7 @@ import { each } from '../utils' * Immutable Types */ const EvaluateResult = Immutable.Record({ result: null, reactorState: null}) +export const CACHE_CLEAR_RATIO = 0.8 function evaluateResult(result, reactorState) { return new EvaluateResult({ @@ -74,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') @@ -168,6 +169,7 @@ 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 + if (isKeyPath(getter)) { getter = fromKeyPath(getter) } @@ -223,25 +225,57 @@ 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 {Array} */ -export function removeObserver(observerState, getter, handler) { - const entriesToRemove = observerState.get('observersMap').filter(entry => { +export function removeObserver(observerState, reactorState, getter, handler) { + 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 - let entryGetter = entry.get('getterKey') - let handlersMatch = (!handler || entry.get('handler') === handler) - if (!handlersMatch) { - return false - } + const entryGetter = entry.get('getterKey') + // check for a by-value equality of keypaths if (isKeyPath(getter) && isKeyPath(entryGetter)) { return isEqual(getter, entryGetter) @@ -249,20 +283,23 @@ export function removeObserver(observerState, getter, handler) { // we are comparing two getters do it by reference return (getter === entryGetter) }) - - return observerState.withMutations(map => { - entriesToRemove.forEach(entry => removeObserverByEntry(map, entry)) - }) } /** * Removes an observer entry by id from the observerState * @param {ObserverState} observerState + * @param {ReactorState} reactorState * @param {Immutable.Map} entry - * @return {ObserverState} + * @return {Array} */ -export function removeObserverByEntry(observerState, entry) { - return observerState.withMutations(map => { +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') @@ -282,6 +319,11 @@ export function removeObserverByEntry(observerState, entry) { map.removeIn(['observersMap', id]) }) + + return { + observerState, + reactorState, + } } /** @@ -308,6 +350,8 @@ export function reset(reactorState) { reactorState.update('storeStates', storeStates => incrementStoreStates(storeStates, storeIds)) resetDirtyStores(reactorState) + reactorState.set('cache', Immutable.Map()) + reactorState.set('cacheRecency', Immutable.OrderedMap()) }) } @@ -329,20 +373,18 @@ export function evaluate(reactorState, 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 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) @@ -381,7 +423,8 @@ export function resetDirtyStores(reactorState) { * @return {Getter} */ function getCacheKey(getter) { - return getter + const getterOption = getGetterOption(getter, 'cacheKey') + return getterOption ? getterOption : getter } /** @@ -424,6 +467,23 @@ function isCached(reactorState, keyPathOrGetter) { * @return {ReactorState} */ function cacheValue(reactorState, getter, value) { + + // Check global cache settings + const globalCacheEnabled = !!reactorState.get('useCache') + let useCache = globalCacheEnabled + + // Check cache settings on a getter basis + const getterCacheOption = getGetterOption(getter, 'cache') + if (getterCacheOption === 'always') { + useCache = true + } else if (getterCacheOption === 'never') { + useCache = false + } + + if (!useCache) { + return reactorState + } + const cacheKey = getCacheKey(getter) const dispatchId = reactorState.get('dispatchId') const storeDeps = getStoreDeps(getter) @@ -434,11 +494,53 @@ 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 <= 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, + })) + }) +} + +/** + * Remove getter cache value from cache and recency cache + * @param {ReactorState} reactorState + * @param {getter} getter + * @return {ReactorState} + */ +function removeCacheValue(reactorState, getter) { + 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 {getter} getter + * @return {ReactorState} + */ +function updateCacheRecency(reactorState, getter) { + const cacheKey = getCacheKey(getter) + return reactorState.withMutations(state => { + state.deleteIn(['cacheRecency', cacheKey]) + state.setIn(['cacheRecency', cacheKey], cacheKey) + }) } /** @@ -457,7 +559,6 @@ function incrementId(reactorState) { return reactorState.update('dispatchId', id => id + 1) } - /** * @param {Immutable.Map} storeStates * @param {Array} storeIds diff --git a/src/reactor/records.js b/src/reactor/records.js index a59314f..f0bc855 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,12 +39,15 @@ 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, + useCache: true, }) export const ObserverState = Record({ diff --git a/tests/getter-tests.js b/tests/getter-tests.js index 93d89f6..aedd8b7 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, getGetterOption, Getter } from '../src/getter' import { Set, List, is } from 'immutable' describe('Getter', () => { @@ -32,6 +32,58 @@ 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() + }) + + it('should accept a keyPath as a getter argument', () => { + const keyPath = ['test'] + expect(Getter(keyPath)).toBe(keyPath) + }) + + it('should accept a getter as a getter argument', () => { + const getter = ['test', () => 1] + expect(Getter(getter)).toBe(getter) + }) + + + it('should use "default" as the default cache option', () => { + const getterObject = Getter(['test'], {}) + expect(getGetterOption(getterObject, 'cache')).toBe('default') + const getterObject1 = Getter(['test'], { cache: 'fakeOption' }) + expect(getGetterOption(getterObject1, 'cache')).toBe('default') + }) + + it('should set "always" and "never" as cache options', () => { + 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 getterObject = Getter(['test'], {}) + expect(getGetterOption(getterObject, 'cacheKey')).toBe(null) + }) + + it('should set cacheKey to supplied value', () => { + const getter = ['test'] + const getterObject = Getter(getter, { cacheKey: 'test' }) + expect(getGetterOption(getterObject, 'cacheKey')).toBe('test') + }) + }) + describe('getFlattenedDeps', function() { describe('when passed the identity getter', () => { it('should return a set with only an empty list', () => { diff --git a/tests/reactor-fns-tests.js b/tests/reactor-fns-tests.js index dfdd4c3..3a52d5f 100644 --- a/tests/reactor-fns-tests.js +++ b/tests/reactor-fns-tests.js @@ -2,8 +2,9 @@ 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' describe('reactor fns', () => { describe('#registerStores', () => { @@ -355,6 +356,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 +479,7 @@ describe('reactor fns', () => { }) describe('#removeObserver', () => { - let initialObserverState, nextObserverState, getter1, getter2, handler1, handler2, handler3 + let initialObserverState, initialReactorState, getter1, getter2, handler1, handler2, handler3 beforeEach(() => { handler1 = () => 1 @@ -483,7 +491,15 @@ describe('reactor fns', () => { ['store2'], (a, b) => a + b ] - getter2 = [[], x => x] + getter2 = Getter([[], 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) @@ -496,8 +512,10 @@ 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 +532,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,11 +567,70 @@ 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') + }) + 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({ @@ -571,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 bf095db..48913d6 100644 --- a/tests/reactor-tests.js +++ b/tests/reactor-tests.js @@ -1,9 +1,11 @@ import Immutable, { Map, List, is } from 'immutable' import { Reactor, Store } from '../src/main' -import { getOption } from '../src/reactor/fns' +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' import logging from '../src/logging' +import { Getter } from '../src/getter' describe('Reactor', () => { it('should construct without \'new\'', () => { @@ -11,10 +13,11 @@ 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) + 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', () => { @@ -22,13 +25,14 @@ describe('Reactor', () => { debug: true, }) expect(reactor.reactorState.get('debug')).toBe(true) + 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', () => { var reactor = new Reactor({ options: { logDispatches: true, - } + }, }) expect(getOption(reactor.reactorState, 'logDispatches')).toBe(true) expect(getOption(reactor.reactorState, 'logAppState')).toBe(false) @@ -44,7 +48,7 @@ describe('Reactor', () => { options: { logDispatches: false, throwOnDispatchInDispatch: false, - } + }, }) expect(getOption(reactor.reactorState, 'logDispatches')).toBe(false) expect(getOption(reactor.reactorState, 'logAppState')).toBe(true) @@ -54,6 +58,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', () => { @@ -100,7 +126,7 @@ describe('Reactor', () => { initialize() { this.on('action', () => undefined) }, - }) + }), }) reactor.dispatch('action') reactor.reset() @@ -123,7 +149,7 @@ describe('Reactor', () => { initialize() { this.on('action', () => undefined) }, - }) + }), }) }).toThrow() }) @@ -144,7 +170,7 @@ describe('Reactor', () => { initialize() { this.on('action', () => undefined) }, - }) + }), }) }).toThrow() }) @@ -164,8 +190,8 @@ describe('Reactor', () => { }, handleReset() { return undefined - } - }) + }, + }), }) reactor.reset() }).toThrow() @@ -189,7 +215,7 @@ describe('Reactor', () => { handleReset() { return { foo: 'baz' } }, - }) + }), }) reactor.reset() }).not.toThrow() @@ -208,7 +234,7 @@ describe('Reactor', () => { getInitialState() { return { foo: 'bar' } }, - }) + }), }) }).toThrow() }) @@ -229,7 +255,7 @@ describe('Reactor', () => { handleReset() { return { foo: 'baz' } }, - }) + }), }) reactor.reset() }).toThrow() @@ -252,8 +278,8 @@ describe('Reactor', () => { }, initialize() { this.on('increment', curr => curr + 1) - } - }) + }, + }), }) reactor.observe(['count'], (val) => { @@ -281,8 +307,8 @@ describe('Reactor', () => { }, initialize() { this.on('increment', curr => curr + 1) - } - }) + }, + }), }) reactor.observe(['count'], (val) => { @@ -595,6 +621,105 @@ describe('Reactor', () => { expect(taxPercentSpy.calls.count()).toEqual(2) 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.reactorState.get('cache').size).toBe(1) + cacheReactor = new Reactor({ + useCache: false, + }) + cacheReactor.evaluate([['test'], () => 1]) + expect(cacheReactor.reactorState.get('cache').size).toBe(0) + }) + + it('should respect individual getter cache settings over global settings', () => { + let cacheReactor = new Reactor({ + useCache: true, + }) + + let getter = Getter([['test2'], () => 1], { cache: 'never' }) + cacheReactor.evaluate(getter) + expect(cacheReactor.reactorState.get('cache').size).toBe(0) + + cacheReactor = new Reactor({ + useCache: false, + }) + getter = Getter([['test2'], () => 1], { cache: 'always' }) + cacheReactor.evaluate(getter) + expect(cacheReactor.reactorState.get('cache').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 = Getter([['test'], () => 1], { cacheKey: 'test' }) + cacheReactor.evaluate(getter) + 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'], () => 1] + }) + + 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', () => { @@ -607,6 +732,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) @@ -695,8 +829,8 @@ describe('Reactor', () => { test: Store({ getInitialState() { return 1 - } - }) + }, + }), }) expect(mockFn.calls.count()).toEqual(1) @@ -717,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) }) @@ -828,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) }) @@ -1777,7 +1909,6 @@ describe('Reactor', () => { describe('#replaceStores', () => { let counter1Store let counter2Store - let counter1StoreReplaced let reactor beforeEach(() => { @@ -1786,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({ @@ -1806,7 +1937,7 @@ describe('Reactor', () => { getInitialState: () => 1, initialize() { this.on('increment1', (state) => state + 10) - } + }, }) expect(reactor.evaluate(['counter1'])).toBe(1) @@ -1828,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)