experiment with different method for batching subscriptions using whi…

…ch uses the base redux subscribe.
tappleby · Apr 3, 2016 · ad8935d · ad8935d
1 parent 4093d5c
commit ad8935d
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 52 deletions.
diff --git a/package.json b/package.json
@@ -35,6 +35,7 @@
     "eslint-config-airbnb": "2.1.1",
     "expect": "^1.13.4",
     "mocha": "^2.2.5",
+    "redux": "^3.3.1",
     "rimraf": "^2.5.0"
   }
 }
diff --git a/src/__tests__/batchedSubscribe-test.js b/src/__tests__/batchedSubscribe-test.js
@@ -1,19 +1,17 @@
 import { batchedSubscribe } from '../';
 import expect from 'expect';
-
-function createStoreShape() {
-  return {
-    dispatch: expect.createSpy(),
-    subscribe: expect.createSpy()
-  };
-}
+import { createStore } from 'redux';
 
 function createBatchedStore(batch = (cb) => cb()) {
-  const baseStore = createStoreShape();
-  const createStore = () => baseStore;
-  const batchedStore = batchedSubscribe(batch)(createStore)();
-  batchedStore.base = baseStore;
+  const baseStore = createStore(() => ({}), {});
+
+  expect.spyOn(baseStore, 'dispatch').andCallThrough();
+  expect.spyOn(baseStore, 'subscribe').andCallThrough();
+
+  const createBaseStore = () => baseStore;
+  const batchedStore = batchedSubscribe(batch)(createBaseStore)();
 
+  batchedStore.base = baseStore;
   return batchedStore;
 }
 
@@ -33,14 +31,15 @@ describe('batchedSubscribe()', () => {
 
     store.subscribe(subscribeCallbackSpy);
     store.dispatch({ type: 'foo' });
+    store.dispatch({ type: 'foo' });
+    store.dispatch({ type: 'foo' });
 
-    expect(store.base.subscribe.calls.length).toEqual(0);
-    expect(subscribeCallbackSpy.calls.length).toEqual(1);
+    expect(subscribeCallbackSpy.calls.length).toEqual(3);
   });
 
   it('it exposes base subscribe as subscribeImmediate', () => {
     const store = createBatchedStore();
-    store.subscribeImmediate();
+    store.subscribeImmediate(() => {});
 
     expect(store.base.subscribe.calls.length).toEqual(1);
   });
@@ -71,8 +70,8 @@ describe('batchedSubscribe()', () => {
     });
     store.subscribe(listenerC);
 
-    store.dispatch({});
-    store.dispatch({});
+    store.dispatch({ type: 'foo' });
+    store.dispatch({ type: 'foo' });
 
     expect(listenerA.calls.length).toEqual(2);
     expect(listenerB.calls.length).toEqual(1);

diff --git a/src/index.js b/src/index.js
@@ -3,54 +3,60 @@ export function batchedSubscribe(batch) {
     throw new Error('Expected batch to be a function.');
   }
 
-  let currentListeners = [];
-  let nextListeners = currentListeners;
+  return next => (...args) => {
+    const store = next(...args);
+    const subscribeImmediate = store.subscribe;
+    let subscriptions = [];
 
-  function ensureCanMutateNextListeners() {
-    if (nextListeners === currentListeners) {
-      nextListeners = currentListeners.slice();
-    }
-  }
+    function subscribe(listener) {
+      if (typeof listener !== 'function') {
+        throw new Error('Expected listener to be a function.');
+      }
 
-  function subscribe(listener) {
-    if (typeof listener !== 'function') {
-      throw new Error('Expected listener to be a function.');
-    }
+      const subscription = {
+        cb: listener, trigger: false, unsubscribe: false, ready: false
+      };
 
-    let isSubscribed = true;
+      const unsubscribeImmediate = subscribeImmediate(() => {
+        subscription.trigger = true;
+      });
 
-    ensureCanMutateNextListeners();
-    nextListeners.push(listener);
+      subscriptions.push(subscription);
 
-    return function unsubscribe() {
-      if (!isSubscribed) {
-        return;
-      }
+      return function unsubscribe() {
+        unsubscribeImmediate();
+        subscription.unsubscribe = true;
+      };
+    }
 
-      isSubscribed = false;
+    function notifyListeners() {
+      let cleanupSubscriptions = false;
 
-      ensureCanMutateNextListeners();
-      const index = nextListeners.indexOf(listener);
-      nextListeners.splice(index, 1);
-    };
-  }
+      for (const subscription of subscriptions) {
+        if (subscription.ready && subscription.trigger) {
+          subscription.cb();
+          subscription.trigger = false;
+        }
 
-  function notifyListeners() {
-    const listeners = currentListeners = nextListeners;
-    for (let i = 0; i < listeners.length; i++) {
-      listeners[i]();
-    }
-  }
+        if (subscription.unsubscribe) {
+          cleanupSubscriptions = true;
+        }
+      }
 
-  function notifyListenersBatched() {
-    batch(notifyListeners);
-  }
+      if (cleanupSubscriptions) {
+        subscriptions = subscriptions.filter((subscription) => !subscription.unsubscribe);
+      }
+    }
 
-  return next => (...args) => {
-    const store = next(...args);
-    const subscribeImmediate = store.subscribe;
+    function notifyListenersBatched() {
+      batch(notifyListeners);
+    }
 
     function dispatch(...dispatchArgs) {
+      for (const subscription of subscriptions) {
+        subscription.ready = subscription.unsubscribe === false;
+      }
+
       const res = store.dispatch(...dispatchArgs);
       notifyListenersBatched();
       return res;