From 2204c6e50b564d068dd1fa09567715234fa7671f Mon Sep 17 00:00:00 2001 From: Luca Mussi Date: Sat, 16 Apr 2016 20:14:26 +0200 Subject: [PATCH 1/5] avoid useless calls to listeners when they've already been notified by the deepest dispatch call --- src/createStore.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/createStore.js b/src/createStore.js index 68097d1ce3..445e062aea 100644 --- a/src/createStore.js +++ b/src/createStore.js @@ -58,6 +58,7 @@ export default function createStore(reducer, initialState, enhancer) { var currentListeners = [] var nextListeners = currentListeners var isDispatching = false + var needToNotify = true function ensureCanMutateNextListeners() { if (nextListeners === currentListeners) { @@ -171,10 +172,12 @@ export default function createStore(reducer, initialState, enhancer) { isDispatching = false } + needToNotify = true var listeners = currentListeners = nextListeners - for (var i = 0; i < listeners.length; i++) { + for (var i = 0; needToNotify && i < listeners.length; i++) { listeners[i]() } + needToNotify = false return action } From 4dd056cc78eb1ba450c3ac663b3b7e7084f10e9d Mon Sep 17 00:00:00 2001 From: Luca Mussi Date: Sun, 17 Apr 2016 09:41:05 +0200 Subject: [PATCH 2/5] consider useless notification pruning during nested dispatch --- test/createStore.spec.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/createStore.spec.js b/test/createStore.spec.js index 3a8af0630b..64bf7e00b6 100644 --- a/test/createStore.spec.js +++ b/test/createStore.spec.js @@ -378,15 +378,15 @@ describe('createStore', () => { store.dispatch(unknownAction()) expect(listener1.calls.length).toBe(1) - expect(listener2.calls.length).toBe(2) - expect(listener3.calls.length).toBe(2) + expect(listener2.calls.length).toBe(1) + expect(listener3.calls.length).toBe(1) expect(listener4.calls.length).toBe(1) unsubscribe4() store.dispatch(unknownAction()) expect(listener1.calls.length).toBe(1) - expect(listener2.calls.length).toBe(3) - expect(listener3.calls.length).toBe(3) + expect(listener2.calls.length).toBe(2) + expect(listener3.calls.length).toBe(2) expect(listener4.calls.length).toBe(1) }) From cb2def2d7d844e4ae55ade2fa57ec540957d4931 Mon Sep 17 00:00:00 2001 From: Luca Mussi Date: Sun, 1 May 2016 18:45:37 +0200 Subject: [PATCH 3/5] call listener one last time within unsubscribe --- src/createStore.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/createStore.js b/src/createStore.js index 445e062aea..e29b411e8e 100644 --- a/src/createStore.js +++ b/src/createStore.js @@ -115,6 +115,8 @@ export default function createStore(reducer, initialState, enhancer) { isSubscribed = false + listener() + ensureCanMutateNextListeners() var index = nextListeners.indexOf(listener) nextListeners.splice(index, 1) From b4ddb6e68f3eccf4f921dcc6fa4104888ff2fb6e Mon Sep 17 00:00:00 2001 From: Luca Mussi Date: Sun, 1 May 2016 18:55:42 +0200 Subject: [PATCH 4/5] consider last call to listener within unsubscribe --- test/createStore.spec.js | 129 +++++++++++++++++++++++---------------- 1 file changed, 77 insertions(+), 52 deletions(-) diff --git a/test/createStore.spec.js b/test/createStore.spec.js index 64bf7e00b6..7a7da90cbf 100644 --- a/test/createStore.spec.js +++ b/test/createStore.spec.js @@ -211,28 +211,39 @@ describe('createStore', () => { expect(listenerB.calls.length).toBe(1) unsubscribeA() - expect(listenerA.calls.length).toBe(3) + expect(listenerA.calls.length).toBe(4) expect(listenerB.calls.length).toBe(1) store.dispatch(unknownAction()) - expect(listenerA.calls.length).toBe(3) + expect(listenerA.calls.length).toBe(4) expect(listenerB.calls.length).toBe(2) unsubscribeB() - expect(listenerA.calls.length).toBe(3) - expect(listenerB.calls.length).toBe(2) + expect(listenerA.calls.length).toBe(4) + expect(listenerB.calls.length).toBe(3) store.dispatch(unknownAction()) - expect(listenerA.calls.length).toBe(3) - expect(listenerB.calls.length).toBe(2) + expect(listenerA.calls.length).toBe(4) + expect(listenerB.calls.length).toBe(3) unsubscribeA = store.subscribe(listenerA) - expect(listenerA.calls.length).toBe(3) - expect(listenerB.calls.length).toBe(2) + expect(listenerA.calls.length).toBe(4) + expect(listenerB.calls.length).toBe(3) store.dispatch(unknownAction()) - expect(listenerA.calls.length).toBe(4) - expect(listenerB.calls.length).toBe(2) + expect(listenerA.calls.length).toBe(5) + expect(listenerB.calls.length).toBe(3) + }) + + it('calls listener when unsubscribe is called', () => { + const store = createStore(reducers.todos) + const listener = expect.createSpy(() => {}) + + const unsubscribe = store.subscribe(listener) + expect(listener.calls.length).toBe(0) + + unsubscribe() + expect(listener.calls.length).toBe(1) }) it('only removes listener once when unsubscribe is called', () => { @@ -245,24 +256,30 @@ describe('createStore', () => { unsubscribeA() unsubscribeA() + expect(listenerA.calls.length).toBe(1) + expect(listenerB.calls.length).toBe(0) store.dispatch(unknownAction()) - expect(listenerA.calls.length).toBe(0) + expect(listenerA.calls.length).toBe(1) expect(listenerB.calls.length).toBe(1) }) it('only removes relevant listener when unsubscribe is called', () => { const store = createStore(reducers.todos) - const listener = expect.createSpy(() => {}) + const listenerA = expect.createSpy(() => {}) + const listenerB = expect.createSpy(() => {}) - store.subscribe(listener) - const unsubscribeSecond = store.subscribe(listener) + store.subscribe(listenerA) + const unsubscribeB = store.subscribe(listenerB) - unsubscribeSecond() - unsubscribeSecond() + unsubscribeB() + unsubscribeB() + expect(listenerA.calls.length).toBe(0) + expect(listenerB.calls.length).toBe(1) store.dispatch(unknownAction()) - expect(listener.calls.length).toBe(1) + expect(listenerA.calls.length).toBe(1) + expect(listenerB.calls.length).toBe(1) }) it('supports removing a subscription within a subscription', () => { @@ -279,10 +296,13 @@ describe('createStore', () => { store.subscribe(listenerC) store.dispatch(unknownAction()) - store.dispatch(unknownAction()) + expect(listenerA.calls.length).toBe(1) + expect(listenerB.calls.length).toBe(2) + expect(listenerC.calls.length).toBe(1) + store.dispatch(unknownAction()) expect(listenerA.calls.length).toBe(2) - expect(listenerB.calls.length).toBe(1) + expect(listenerB.calls.length).toBe(2) expect(listenerC.calls.length).toBe(2) }) @@ -295,25 +315,24 @@ describe('createStore', () => { ) const listener1 = expect.createSpy(() => {}) - const listener2 = expect.createSpy(() => {}) + const listener2 = expect.createSpy(() => {}).andCall(() => { + doUnsubscribeAll() + }) const listener3 = expect.createSpy(() => {}) - unsubscribeHandles.push(store.subscribe(() => listener1())) - unsubscribeHandles.push(store.subscribe(() => { - listener2() - doUnsubscribeAll() - })) - unsubscribeHandles.push(store.subscribe(() => listener3())) + unsubscribeHandles.push(store.subscribe(listener1)) + unsubscribeHandles.push(store.subscribe(listener2)) + unsubscribeHandles.push(store.subscribe(listener3)) store.dispatch(unknownAction()) - expect(listener1.calls.length).toBe(1) - expect(listener2.calls.length).toBe(1) - expect(listener3.calls.length).toBe(1) + expect(listener1.calls.length).toBe(2) + expect(listener2.calls.length).toBe(2) + expect(listener3.calls.length).toBe(2) store.dispatch(unknownAction()) - expect(listener1.calls.length).toBe(1) - expect(listener2.calls.length).toBe(1) - expect(listener3.calls.length).toBe(1) + expect(listener1.calls.length).toBe(2) + expect(listener2.calls.length).toBe(2) + expect(listener3.calls.length).toBe(2) }) it('delays subscribe until the end of current dispatch', () => { @@ -351,43 +370,49 @@ describe('createStore', () => { it('uses the last snapshot of subscribers during nested dispatch', () => { const store = createStore(reducers.todos) - const listener1 = expect.createSpy(() => {}) + const listener1 = expect.createSpy().andCall(() => { + if (!unsubscribe4) { + expect(listener1.calls.length).toBe(1) + expect(listener2.calls.length).toBe(0) + expect(listener3.calls.length).toBe(0) + expect(listener4.calls.length).toBe(0) + + unsubscribe4 = store.subscribe(listener4) + unsubscribe1() + expect(listener1.calls.length).toBe(2) + + store.dispatch(unknownAction()) + + expect(listener1.calls.length).toBe(2) + expect(listener2.calls.length).toBe(1) + expect(listener3.calls.length).toBe(1) + expect(listener4.calls.length).toBe(1) + + } + }) const listener2 = expect.createSpy(() => {}) const listener3 = expect.createSpy(() => {}) const listener4 = expect.createSpy(() => {}) let unsubscribe4 - const unsubscribe1 = store.subscribe(() => { - listener1() - expect(listener1.calls.length).toBe(1) - expect(listener2.calls.length).toBe(0) - expect(listener3.calls.length).toBe(0) - expect(listener4.calls.length).toBe(0) - - unsubscribe1() - unsubscribe4 = store.subscribe(listener4) - store.dispatch(unknownAction()) - - expect(listener1.calls.length).toBe(1) - expect(listener2.calls.length).toBe(1) - expect(listener3.calls.length).toBe(1) - expect(listener4.calls.length).toBe(1) - }) + const unsubscribe1 = store.subscribe(listener1) store.subscribe(listener2) store.subscribe(listener3) store.dispatch(unknownAction()) - expect(listener1.calls.length).toBe(1) + expect(listener1.calls.length).toBe(2) expect(listener2.calls.length).toBe(1) expect(listener3.calls.length).toBe(1) expect(listener4.calls.length).toBe(1) unsubscribe4() + expect(listener4.calls.length).toBe(2) + store.dispatch(unknownAction()) - expect(listener1.calls.length).toBe(1) + expect(listener1.calls.length).toBe(2) expect(listener2.calls.length).toBe(2) expect(listener3.calls.length).toBe(2) - expect(listener4.calls.length).toBe(1) + expect(listener4.calls.length).toBe(2) }) it('provides an up-to-date state when a subscriber is notified', done => { From 4f08d203b5e3f70270cdf65ea145adbc76dfe0b1 Mon Sep 17 00:00:00 2001 From: Luca Mussi Date: Sun, 1 May 2016 23:26:32 +0200 Subject: [PATCH 5/5] consider last call to listener within unsubscribe --- test/createStore.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/createStore.spec.js b/test/createStore.spec.js index 5abccfcdb4..9d7d697df5 100644 --- a/test/createStore.spec.js +++ b/test/createStore.spec.js @@ -724,7 +724,7 @@ describe('createStore', () => { sub.unsubscribe() store.dispatch({ type: 'bar' }) - expect(results).toEqual([ { foo: 0, bar: 0 }, { foo: 1, bar: 0 } ]) + expect(results).toEqual([ { foo: 0, bar: 0 }, { foo: 1, bar: 0 }, { foo: 1, bar: 0 } ]) }) it('should pass an integration test with a common library (RxJS)', () => {