From 424db11bf2fb6fcc06124f89e4e5c7bf1b8b3fa2 Mon Sep 17 00:00:00 2001
From: Yun Lai <lyonlai1984@gmail.com>
Date: Sat, 31 Oct 2015 16:26:09 +1100
Subject: [PATCH 1/9] still something wrong
---
package.json | 2 +-
src/reactor.js | 19 +++----
src/reactor/fns.js | 125 +++++++++++++++++++++++++++--------------
src/reactor/records.js | 8 ++-
4 files changed, 98 insertions(+), 56 deletions(-)
diff --git a/package.json b/package.json
index 0cf02f5..123671e 100644
--- a/package.json
+++ b/package.json
@@ -2,7 +2,7 @@
"name": "nuclear-js",
"version": "1.1.2",
"description": "Immutable, reactive Flux architecture. UI Agnostic.",
- "main": "dist/nuclear.js",
+ "main": "src/main.js",
"scripts": {
"test": "grunt ci"
},
diff --git a/src/reactor.js b/src/reactor.js
index b31cb21..6e0cf49 100644
--- a/src/reactor.js
+++ b/src/reactor.js
@@ -195,7 +195,7 @@ class Reactor {
return
}
- let observerIdsToNotify = Immutable.Set().withMutations(set => {
+ let gettersToNotify = Immutable.Set().withMutations(set => {
// notify all observers
set.union(this.observerState.get('any'))
@@ -208,15 +208,8 @@ class Reactor {
})
})
- observerIdsToNotify.forEach((observerId) => {
- const entry = this.observerState.getIn(['observersMap', observerId])
- if (!entry) {
- // don't notify here in the case a handler called unobserve on another observer
- return
- }
-
- const getter = entry.get('getter')
- const handler = entry.get('handler')
+ gettersToNotify.forEach((getterId) => {
+ const getter = this.observerState.get('getters')[getterId];
const prevEvaluateResult = fns.evaluate(this.prevReactorState, getter)
const currEvaluateResult = fns.evaluate(this.reactorState, getter)
@@ -225,7 +218,11 @@ class Reactor {
const currValue = currEvaluateResult.result
if (!Immutable.is(prevValue, currValue)) {
- handler.call(null, currValue)
+ const handlers = this.observerState.getIn(['gettersMap', getterId])
+ .map(observerId => this.observerState.getIn(['observersMap', observerId, 'handler']))
+ // don't notify here in the case a handler called unobserve on another observer
+
+ handlers.forEach(handler => handler.call(null, currValue))
}
})
diff --git a/src/reactor/fns.js b/src/reactor/fns.js
index 4afea7d..ce49937 100644
--- a/src/reactor/fns.js
+++ b/src/reactor/fns.js
@@ -156,45 +156,73 @@ exports.loadState = function(reactorState, state) {
*/
exports.addObserver = function(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)
- }
+ try {
+ const getterKey = getter
+ if (isKeyPath(getter)) {
+ getter = fromKeyPath(getter)
+ }
- const currId = observerState.get('nextId')
- const storeDeps = getStoreDeps(getter)
- const entry = Immutable.Map({
- id: currId,
- storeDeps: storeDeps,
- getterKey: getterKey,
- getter: getter,
- handler: handler,
- })
+ const currId = observerState.get('nextId')
+ const storeDeps = getStoreDeps(getter)
+ const entry = Immutable.Map({
+ id: currId,
+ storeDeps: storeDeps,
+ getterKey: getterKey,
+ getter: getter,
+ handler: handler,
+ })
- let updatedObserverState
- if (storeDeps.size === 0) {
- // no storeDeps means the observer is dependent on any of the state changing
- updatedObserverState = observerState.update('any', observerIds => observerIds.add(currId))
- } else {
- updatedObserverState = observerState.withMutations(map => {
- storeDeps.forEach(storeId => {
- let path = ['stores', storeId]
- if (!map.hasIn(path)) {
- map.setIn(path, Immutable.Set([]))
- }
- map.updateIn(['stores', storeId], observerIds => observerIds.add(currId))
+ let updatedObserverState
+
+ let existingGetters = observerState.get('getters');
+
+ let getterId = existingGetters.indexOf(getter);
+
+ if (getterId < 0) {
+ existingGetters.push(getter);
+ getterId = existingGetters.length - 1;
+ }
+ //update getterMap
+
+ let observerIdsForGetter = observerState.getIn(['gettersMap', getterId])
+
+ if (!observerIdsForGetter) {
+ observerIdsForGetter = Immutable.Set([])
+ }
+
+ observerIdsForGetter = observerIdsForGetter.add(currId);
+
+ updatedObserverState = observerState.setIn(['gettersMap', getterId], observerIdsForGetter);
+
+ if (storeDeps.size === 0) {
+ // no storeDeps means the observer is dependent on any of the state changing
+
+ updatedObserverState = updatedObserverState.updateIn(['any'], getters => getters.add(getterId))
+ } else {
+ updatedObserverState = updatedObserverState.withMutations(map => {
+ storeDeps.forEach(storeId => {
+ let path = ['stores', storeId]
+ if (!map.hasIn(path)) {
+ map.setIn(path, Immutable.Set([]))
+ }
+ map.updateIn(['stores', storeId], getters => getters.add(getterId))
+ })
})
- })
- }
+ }
- updatedObserverState = updatedObserverState
- .set('nextId', currId + 1)
- .setIn(['observersMap', currId], entry)
+ updatedObserverState = updatedObserverState
+ .set('nextId', currId + 1)
+ .setIn(['observersMap', currId], entry)
- return {
- observerState: updatedObserverState,
- entry: entry,
+ return {
+ observerState: updatedObserverState,
+ entry: entry,
+ }
+ } catch (e) {
+ debugger;
}
+
+
}
/**
@@ -239,18 +267,31 @@ exports.removeObserver = function(observerState, getter, handler) {
*/
exports.removeObserverByEntry = function(observerState, entry) {
return observerState.withMutations(map => {
- const id = entry.get('id')
- const storeDeps = entry.get('storeDeps')
+ try {
+ const id = entry.get('id')
+ const getter = entry.get('getter')
+ 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 => observers.remove(id))
- })
+ const existingGetters = observerState.get('getters');
+
+ const getterId = existingGetters.indexOf(getter);
+
+ //cleaning the gettersMap
+ map.updateIn(['gettersMap', getterId], observerIds => observerIds.remove(id));
+
+ if (storeDeps.size === 0 && map.getIn(['gettersMap', getterId]).size === 0) {
+ map.update('any', anyGetters => anyGetters.remove(getterId))
+ } else {
+ storeDeps.forEach(storeId => {
+ map.updateIn(['stores', storeId], getters => getters.remove(getterId))
+ })
+ }
+
+ map.removeIn(['observersMap', id])
+ } catch (e) {
+ debugger;
}
- map.removeIn(['observersMap', id])
})
}
diff --git a/src/reactor/records.js b/src/reactor/records.js
index 33aaff9..4018941 100644
--- a/src/reactor/records.js
+++ b/src/reactor/records.js
@@ -12,11 +12,15 @@ const ReactorState = Immutable.Record({
})
const ObserverState = Immutable.Record({
- // observers registered to any store change
+ // getters registered to any store change
any: Immutable.Set([]),
- // observers registered to specific store changes
+ // getters registered to specific store changes
stores: Immutable.Map({}),
+ getters: [],
+
+ gettersMap: Immutable.Map({}),
+
observersMap: Immutable.Map({}),
nextId: 1,
From c1dc8c4962a449d24fa88a69e3cb884f274870a9 Mon Sep 17 00:00:00 2001
From: Yun Lai <lyonlai1984@gmail.com>
Date: Sun, 1 Nov 2015 00:05:44 +1100
Subject: [PATCH 2/9] fixed the problem. now can use to compare with the
previous one
---
src/reactor.js | 6 ++--
src/reactor/fns.js | 67 ++++++++++++++++--------------------------
src/reactor/records.js | 2 --
3 files changed, 27 insertions(+), 48 deletions(-)
diff --git a/src/reactor.js b/src/reactor.js
index 6e0cf49..224f8d4 100644
--- a/src/reactor.js
+++ b/src/reactor.js
@@ -208,9 +208,7 @@ class Reactor {
})
})
- gettersToNotify.forEach((getterId) => {
- const getter = this.observerState.get('getters')[getterId];
-
+ gettersToNotify.forEach(getter => {
const prevEvaluateResult = fns.evaluate(this.prevReactorState, getter)
const currEvaluateResult = fns.evaluate(this.reactorState, getter)
@@ -218,7 +216,7 @@ class Reactor {
const currValue = currEvaluateResult.result
if (!Immutable.is(prevValue, currValue)) {
- const handlers = this.observerState.getIn(['gettersMap', getterId])
+ const handlers = this.observerState.getIn(['gettersMap', getter])
.map(observerId => this.observerState.getIn(['observersMap', observerId, 'handler']))
// don't notify here in the case a handler called unobserve on another observer
diff --git a/src/reactor/fns.js b/src/reactor/fns.js
index ce49937..f4f9baf 100644
--- a/src/reactor/fns.js
+++ b/src/reactor/fns.js
@@ -172,40 +172,26 @@ exports.addObserver = function(observerState, getter, handler) {
handler: handler,
})
- let updatedObserverState
-
- let existingGetters = observerState.get('getters');
-
- let getterId = existingGetters.indexOf(getter);
-
- if (getterId < 0) {
- existingGetters.push(getter);
- getterId = existingGetters.length - 1;
- }
- //update getterMap
-
- let observerIdsForGetter = observerState.getIn(['gettersMap', getterId])
-
- if (!observerIdsForGetter) {
- observerIdsForGetter = Immutable.Set([])
- }
-
- observerIdsForGetter = observerIdsForGetter.add(currId);
-
- updatedObserverState = observerState.setIn(['gettersMap', getterId], observerIdsForGetter);
+ let updatedObserverState = observerState.updateIn(['gettersMap', getter]
+ , observerIds =>
+ observerIds
+ ? observerIds.add(currId)
+ : Immutable.Set([]).add(currId)
+ )
if (storeDeps.size === 0) {
// no storeDeps means the observer is dependent on any of the state changing
- updatedObserverState = updatedObserverState.updateIn(['any'], getters => getters.add(getterId))
+ updatedObserverState = updatedObserverState.updateIn(['any'], getters => getters.add(getter))
} else {
updatedObserverState = updatedObserverState.withMutations(map => {
storeDeps.forEach(storeId => {
- let path = ['stores', storeId]
- if (!map.hasIn(path)) {
- map.setIn(path, Immutable.Set([]))
- }
- map.updateIn(['stores', storeId], getters => getters.add(getterId))
+ map.updateIn(['stores', storeId]
+ , getters =>
+ getters
+ ? getters.add(getter)
+ : Immutable.Set([]).add(getter)
+ )
})
})
}
@@ -267,31 +253,28 @@ exports.removeObserver = function(observerState, getter, handler) {
*/
exports.removeObserverByEntry = function(observerState, entry) {
return observerState.withMutations(map => {
- try {
- const id = entry.get('id')
- const getter = entry.get('getter')
- const storeDeps = entry.get('storeDeps')
-
- const existingGetters = observerState.get('getters');
+ const id = entry.get('id')
+ const getter = entry.get('getter')
+ const storeDeps = entry.get('storeDeps')
- const getterId = existingGetters.indexOf(getter);
+ map.updateIn(['gettersMap', getter], observerIds => observerIds.remove(id));
- //cleaning the gettersMap
- map.updateIn(['gettersMap', getterId], observerIds => observerIds.remove(id));
+ if (map.getIn(['gettersMap', getter]).size <= 0) {
- if (storeDeps.size === 0 && map.getIn(['gettersMap', getterId]).size === 0) {
- map.update('any', anyGetters => anyGetters.remove(getterId))
+ if (storeDeps.size === 0) {
+ // no storeDeps means the observer is dependent on any of the state changing
+ map.update('any', getters => getters.remove(getter));
} else {
storeDeps.forEach(storeId => {
- map.updateIn(['stores', storeId], getters => getters.remove(getterId))
+ map.updateIn(['stores', storeId]
+ , getters => getters.remove(getter) )
})
}
- map.removeIn(['observersMap', id])
- } catch (e) {
- debugger;
}
+ map.removeIn(['observersMap', id])
+
})
}
diff --git a/src/reactor/records.js b/src/reactor/records.js
index 4018941..fe5903f 100644
--- a/src/reactor/records.js
+++ b/src/reactor/records.js
@@ -17,8 +17,6 @@ const ObserverState = Immutable.Record({
// getters registered to specific store changes
stores: Immutable.Map({}),
- getters: [],
-
gettersMap: Immutable.Map({}),
observersMap: Immutable.Map({}),
From f77202cc68814c5b9e99d68dea55b4b3506e598d Mon Sep 17 00:00:00 2001
From: Yun Lai <lyonlai1984@gmail.com>
Date: Sun, 1 Nov 2015 16:53:40 +1100
Subject: [PATCH 3/9] caching fix for notify.
---
src/reactor.js | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/reactor.js b/src/reactor.js
index 224f8d4..ce05b0e 100644
--- a/src/reactor.js
+++ b/src/reactor.js
@@ -212,6 +212,9 @@ class Reactor {
const prevEvaluateResult = fns.evaluate(this.prevReactorState, getter)
const currEvaluateResult = fns.evaluate(this.reactorState, getter)
+ this.prevReactorState = prevEvaluateResult.reactorState
+ this.reactorState = currEvaluateResult.reactorState
+
const prevValue = prevEvaluateResult.result
const currValue = currEvaluateResult.result
From d38b04e2fe29736ca0ad2bcaf73d440cc0514c2b Mon Sep 17 00:00:00 2001
From: Yun Lai <lyonlai1984@gmail.com>
Date: Sun, 1 Nov 2015 16:58:44 +1100
Subject: [PATCH 4/9] restore the package.json
---
package.json | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/package.json b/package.json
index 123671e..0cf02f5 100644
--- a/package.json
+++ b/package.json
@@ -2,7 +2,7 @@
"name": "nuclear-js",
"version": "1.1.2",
"description": "Immutable, reactive Flux architecture. UI Agnostic.",
- "main": "src/main.js",
+ "main": "dist/nuclear.js",
"scripts": {
"test": "grunt ci"
},
From fe88c19d36268132c6cf3fe2c3744d461db8a970 Mon Sep 17 00:00:00 2001
From: Yun Lai <lyonlai1984@gmail.com>
Date: Sun, 1 Nov 2015 17:13:18 +1100
Subject: [PATCH 5/9] remove the testing bit.
---
src/reactor/fns.js | 85 ++++++++++++++++++++++------------------------
1 file changed, 40 insertions(+), 45 deletions(-)
diff --git a/src/reactor/fns.js b/src/reactor/fns.js
index f4f9baf..b25f390 100644
--- a/src/reactor/fns.js
+++ b/src/reactor/fns.js
@@ -156,59 +156,54 @@ exports.loadState = function(reactorState, state) {
*/
exports.addObserver = function(observerState, getter, handler) {
// use the passed in getter as the key so we can rely on a byreference call for unobserve
- try {
- const getterKey = getter
- if (isKeyPath(getter)) {
- getter = fromKeyPath(getter)
- }
-
- const currId = observerState.get('nextId')
- const storeDeps = getStoreDeps(getter)
- const entry = Immutable.Map({
- id: currId,
- storeDeps: storeDeps,
- getterKey: getterKey,
- getter: getter,
- handler: handler,
- })
+ const getterKey = getter
+ if (isKeyPath(getter)) {
+ getter = fromKeyPath(getter)
+ }
- let updatedObserverState = observerState.updateIn(['gettersMap', getter]
- , observerIds =>
- observerIds
- ? observerIds.add(currId)
- : Immutable.Set([]).add(currId)
- )
+ const currId = observerState.get('nextId')
+ const storeDeps = getStoreDeps(getter)
+ const entry = Immutable.Map({
+ id: currId,
+ storeDeps: storeDeps,
+ getterKey: getterKey,
+ getter: getter,
+ handler: handler,
+ })
- if (storeDeps.size === 0) {
- // no storeDeps means the observer is dependent on any of the state changing
+ let updatedObserverState = observerState.updateIn(['gettersMap', getter]
+ , observerIds =>
+ observerIds
+ ? observerIds.add(currId)
+ : Immutable.Set([]).add(currId)
+ )
- updatedObserverState = updatedObserverState.updateIn(['any'], getters => getters.add(getter))
- } else {
- updatedObserverState = updatedObserverState.withMutations(map => {
- storeDeps.forEach(storeId => {
- map.updateIn(['stores', storeId]
- , getters =>
- getters
- ? getters.add(getter)
- : Immutable.Set([]).add(getter)
- )
- })
+ if (storeDeps.size === 0) {
+ // no storeDeps means the observer is dependent on any of the state changing
+
+ updatedObserverState = updatedObserverState.updateIn(['any'], getters => getters.add(getter))
+ } else {
+ updatedObserverState = updatedObserverState.withMutations(map => {
+ storeDeps.forEach(storeId => {
+ map.updateIn(['stores', storeId]
+ , getters =>
+ getters
+ ? getters.add(getter)
+ : Immutable.Set([]).add(getter)
+ )
})
- }
+ })
+ }
- updatedObserverState = updatedObserverState
- .set('nextId', currId + 1)
- .setIn(['observersMap', currId], entry)
+ updatedObserverState = updatedObserverState
+ .set('nextId', currId + 1)
+ .setIn(['observersMap', currId], entry)
- return {
- observerState: updatedObserverState,
- entry: entry,
- }
- } catch (e) {
- debugger;
+ return {
+ observerState: updatedObserverState,
+ entry: entry,
}
-
}
/**
From b75d528224ebcb1276adb251ebc56cef92fc036d Mon Sep 17 00:00:00 2001
From: Yun Lai <lyonlai1984@gmail.com>
Date: Mon, 2 Nov 2015 07:43:02 +1100
Subject: [PATCH 6/9] tests passed
---
tests/reactor-fns-tests.js | 38 ++++++++++++++++++++++++++++----------
1 file changed, 28 insertions(+), 10 deletions(-)
diff --git a/tests/reactor-fns-tests.js b/tests/reactor-fns-tests.js
index 8d8bf08..de1950f 100644
--- a/tests/reactor-fns-tests.js
+++ b/tests/reactor-fns-tests.js
@@ -321,12 +321,12 @@ describe('reactor fns', () => {
entry = result.entry
})
- it('should update the "any" observers', () => {
- const expected = Set.of(1)
+ it('should update the "any" with getter reference', () => {
+ const expected = Set.of(getter)
const result = nextObserverState.get('any')
expect(is(expected, result)).toBe(true)
})
- it('should not update the "store" observers', () => {
+ it('should not update the "store" with getter reference', () => {
const expected = Map({})
const result = nextObserverState.get('stores')
expect(is(expected, result)).toBe(true)
@@ -336,6 +336,11 @@ describe('reactor fns', () => {
const result = nextObserverState.get('nextId')
expect(is(expected, result)).toBe(true)
})
+ it('should update the gettersMap with getter as ref, id as value', () => {
+ const expected = Set.of(1)
+ const result = nextObserverState.getIn(['gettersMap', getter])
+ expect(is(expected, result)).toBe(true)
+ })
it('should update the observerMap', () => {
const expected = Map([
[1, Map({
@@ -375,20 +380,25 @@ describe('reactor fns', () => {
nextObserverState = result.observerState
entry = result.entry
})
- it('should not update the "any" observers', () => {
+ it('should not update the "any" getters', () => {
const expected = Set.of()
const result = nextObserverState.get('any')
expect(is(expected, result)).toBe(true)
})
- it('should not update the "store" observers', () => {
+ it('should update the "store" with getter reference', () => {
const expected = Map({
- store1: Set.of(1),
- store2: Set.of(1),
+ store1: Set.of(getter),
+ store2: Set.of(getter),
})
const result = nextObserverState.get('stores')
expect(is(expected, result)).toBe(true)
})
+ it('should update the gettersMap with getter as ref, id as value', () => {
+ const expected = Set.of(1)
+ const result = nextObserverState.getIn(['gettersMap', getter])
+ expect(is(expected, result)).toBe(true)
+ })
it('should increment the nextId', () => {
const expected = 2
const result = nextObserverState.get('nextId')
@@ -448,12 +458,16 @@ describe('reactor fns', () => {
it('should return a new ObserverState with all entries containing the getter removed', () => {
nextObserverState = fns.removeObserver(initialObserverState, getter1)
const expected = Map({
- any: Set.of(3),
+ any: Set.of(getter2),
stores: Map({
store1: Set(),
store2: Set(),
}),
nextId: 4,
+ gettersMap: Map([
+ [getter1, Set()],
+ [getter2, Set.of(3)]
+ ]),
observersMap: Map([
[3, Map({
id: 3,
@@ -475,10 +489,14 @@ describe('reactor fns', () => {
const expected = Map({
any: Set(),
stores: Map({
- store1: Set.of(1, 2),
- store2: Set.of(1, 2),
+ store1: Set.of(getter1),
+ store2: Set.of(getter1),
}),
nextId: 4,
+ gettersMap: Map([
+ [getter1, Set.of(1, 2)],
+ [getter2, Set()]
+ ]),
observersMap: Map([
[1, Map({
id: 1,
From eb162a1cecf3bbd6f0bc3e16cf9faafe3541c0d8 Mon Sep 17 00:00:00 2001
From: Yun Lai <lyonlai1984@gmail.com>
Date: Mon, 2 Nov 2015 07:44:37 +1100
Subject: [PATCH 7/9] solve the formatting issue
---
src/reactor/fns.js | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/src/reactor/fns.js b/src/reactor/fns.js
index b25f390..4ded614 100644
--- a/src/reactor/fns.js
+++ b/src/reactor/fns.js
@@ -171,12 +171,9 @@ exports.addObserver = function(observerState, getter, handler) {
handler: handler,
})
- let updatedObserverState = observerState.updateIn(['gettersMap', getter]
- , observerIds =>
- observerIds
- ? observerIds.add(currId)
- : Immutable.Set([]).add(currId)
- )
+ let updatedObserverState = observerState.updateIn(['gettersMap', getter], observerIds => {
+ return observerIds ? observerIds.add(currId) : Immutable.Set.of(currId)
+ })
if (storeDeps.size === 0) {
// no storeDeps means the observer is dependent on any of the state changing
@@ -185,12 +182,9 @@ exports.addObserver = function(observerState, getter, handler) {
} else {
updatedObserverState = updatedObserverState.withMutations(map => {
storeDeps.forEach(storeId => {
- map.updateIn(['stores', storeId]
- , getters =>
- getters
- ? getters.add(getter)
- : Immutable.Set([]).add(getter)
- )
+ map.updateIn(['stores', storeId], getters => {
+ return getters ? getters.add(getter) : Immutable.Set.of(getter)
+ })
})
})
}
From 22bedfba0b9ee481487489908e53503ecbc9842e Mon Sep 17 00:00:00 2001
From: Yun Lai <lyonlai1984@gmail.com>
Date: Mon, 2 Nov 2015 08:38:52 +1100
Subject: [PATCH 8/9] cover the test case where removing a getter with handler,
if there is still handler for the getter, do not remove the reference in
store.
---
tests/reactor-fns-tests.js | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/tests/reactor-fns-tests.js b/tests/reactor-fns-tests.js
index de1950f..959e825 100644
--- a/tests/reactor-fns-tests.js
+++ b/tests/reactor-fns-tests.js
@@ -518,6 +518,40 @@ describe('reactor fns', () => {
expect(is(expected, result)).toBe(true)
})
})
+
+ it('should not remove the getter reference in store when there is still listeners for the getter', () => {
+ nextObserverState = fns.removeObserver(initialObserverState, getter1, handler2)
+ const expected = Map({
+ any: Set.of(getter2),
+ stores: Map({
+ store1: Set.of(getter1),
+ store2: Set.of(getter1),
+ }),
+ nextId: 4,
+ gettersMap: Map([
+ [getter1, Set.of(1)],
+ [getter2, Set.of(3)]
+ ]),
+ observersMap: Map([
+ [1, Map({
+ id: 1,
+ storeDeps: Set.of('store1', 'store2'),
+ getterKey: getter1,
+ getter: getter1,
+ handler: handler1,
+ })],
+ [3, Map({
+ id: 3,
+ storeDeps: Set(),
+ getterKey: getter2,
+ getter: getter2,
+ handler: handler3,
+ })]
+ ])
+ })
+ const result = nextObserverState
+ expect(is(expected, result)).toBe(true)
+ })
})
})
/*eslint-enable one-var, comma-dangle*/
From f712e7d13df0207bcdf1012ebd3b893e832f5fa2 Mon Sep 17 00:00:00 2001
From: jordangarcia <jordan.garcia@gmail.com>
Date: Tue, 3 Nov 2015 13:51:36 -0600
Subject: [PATCH 9/9] Update to fix edge cases of unwatch fn
---
src/reactor.js | 18 ++++++++++--------
src/reactor/fns.js | 25 +++++++++++++++++--------
2 files changed, 27 insertions(+), 16 deletions(-)
diff --git a/src/reactor.js b/src/reactor.js
index ce05b0e..74d6c47 100644
--- a/src/reactor.js
+++ b/src/reactor.js
@@ -201,10 +201,9 @@ class Reactor {
dirtyStores.forEach(id => {
const entries = this.observerState.getIn(['stores', id])
- if (!entries) {
- return
+ if (entries) {
+ set.union(entries)
}
- set.union(entries)
})
})
@@ -219,11 +218,14 @@ class Reactor {
const currValue = currEvaluateResult.result
if (!Immutable.is(prevValue, currValue)) {
- const handlers = this.observerState.getIn(['gettersMap', getter])
- .map(observerId => this.observerState.getIn(['observersMap', observerId, 'handler']))
- // don't notify here in the case a handler called unobserve on another observer
-
- handlers.forEach(handler => handler.call(null, currValue))
+ const observerIds = this.observerState.getIn(['gettersMap', getter], [])
+ observerIds.forEach(observerId => {
+ const handler = this.observerState.getIn(['observersMap', observerId, 'handler'])
+ // don't notify here in the case a handler called unobserve on another observer
+ if (handler) {
+ handler.call(null, currValue)
+ }
+ })
}
})
diff --git a/src/reactor/fns.js b/src/reactor/fns.js
index 4ded614..6fe3a92 100644
--- a/src/reactor/fns.js
+++ b/src/reactor/fns.js
@@ -177,7 +177,6 @@ exports.addObserver = function(observerState, getter, handler) {
if (storeDeps.size === 0) {
// no storeDeps means the observer is dependent on any of the state changing
-
updatedObserverState = updatedObserverState.updateIn(['any'], getters => getters.add(getter))
} else {
updatedObserverState = updatedObserverState.withMutations(map => {
@@ -246,24 +245,34 @@ exports.removeObserverByEntry = function(observerState, entry) {
const getter = entry.get('getter')
const storeDeps = entry.get('storeDeps')
- map.updateIn(['gettersMap', getter], observerIds => observerIds.remove(id));
-
- if (map.getIn(['gettersMap', getter]).size <= 0) {
+ const observerIds = map.getIn(['gettersMap', getter])
+ if (!observerIds) {
+ // getter doesn't exist if reactor.reset() is called before the unwatchFn()
+ return
+ }
+ const updatedObserverIds = observerIds.remove(id)
+ map.setIn(['gettersMap', getter], updatedObserverIds)
+ if (updatedObserverIds.size === 0) {
+ // all observers have been removed for this getter, remove other entries
if (storeDeps.size === 0) {
// no storeDeps means the observer is dependent on any of the state changing
map.update('any', getters => getters.remove(getter));
} else {
storeDeps.forEach(storeId => {
- map.updateIn(['stores', storeId]
- , getters => getters.remove(getter) )
+ map.updateIn(['stores', storeId], getters => {
+ if (getters) {
+ // check to make sure the getters Set exists for this store,
+ // in the case of reactor.reset() is called before the unwatchFn()
+ return getters.remove(getter)
+ }
+ return getters
+ })
})
}
-
}
map.removeIn(['observersMap', id])
-
})
}