Merge pull request #2037 from hypothesis/add-watch-utility

Add `watch` utility to streamline reacting to store changes

src/sidebar/components/stream-content.js

+import { watch } from '../util/watch';
+
 // @ngInject
 function StreamContentController($scope, store, api, rootThread, searchFilter) {
   /** `offset` parameter for the next search API call. */
@@ -43,13 +45,8 @@ function StreamContentController($scope, store, api, rootThread, searchFilter) {
     fetch(20);
   }
 
-  let lastQuery = currentQuery();
-  const unsubscribe = store.subscribe(() => {
-    const query = currentQuery();
-    if (query !== lastQuery) {
-      lastQuery = query;
-      clearAndFetch();
-    }
+  const unsubscribe = watch(store.subscribe, currentQuery, () => {
+    clearAndFetch();
   });
   $scope.$on('$destroy', unsubscribe);
 

src/sidebar/services/frame-sync.js

@@ -5,6 +5,7 @@ import events from '../events';
 import Discovery from '../../shared/discovery';
 import uiConstants from '../ui-constants';
 import * as metadata from '../util/annotation-metadata';
+import { watch } from '../util/watch';
 
 /**
  * @typedef FrameInfo
@@ -49,76 +50,66 @@ export default function FrameSync($rootScope, $window, store, bridge) {
    * notify connected frames about new/updated/deleted annotations.
    */
   function setupSyncToFrame() {
-    // List of loaded annotations in previous state
-    let prevAnnotations = [];
-    let prevFrames = [];
     let prevPublicAnns = 0;
 
-    store.subscribe(function () {
-      const state = store.getState();
-      if (
-        state.annotations.annotations === prevAnnotations &&
-        state.frames === prevFrames
-      ) {
-        return;
-      }
+    watch(
+      store.subscribe,
+      [() => store.getState().annotations.annotations, () => store.frames()],
+      ([annotations, frames], [prevAnnotations]) => {
+        let publicAnns = 0;
+        const inSidebar = new Set();
+        const added = [];
 
-      let publicAnns = 0;
-      const inSidebar = new Set();
-      const added = [];
+        annotations.forEach(function (annot) {
+          if (metadata.isReply(annot)) {
+            // The frame does not need to know about replies
+            return;
+          }
 
-      state.annotations.annotations.forEach(function (annot) {
-        if (metadata.isReply(annot)) {
-          // The frame does not need to know about replies
-          return;
-        }
+          if (metadata.isPublic(annot)) {
+            ++publicAnns;
+          }
 
-        if (metadata.isPublic(annot)) {
-          ++publicAnns;
-        }
+          inSidebar.add(annot.$tag);
+          if (!inFrame.has(annot.$tag)) {
+            added.push(annot);
+          }
+        });
+        const deleted = prevAnnotations.filter(function (annot) {
+          return !inSidebar.has(annot.$tag);
+        });
 
-        inSidebar.add(annot.$tag);
-        if (!inFrame.has(annot.$tag)) {
-          added.push(annot);
+        // We currently only handle adding and removing annotations from the frame
+        // when they are added or removed in the sidebar, but not re-anchoring
+        // annotations if their selectors are updated.
+        if (added.length > 0) {
+          bridge.call('loadAnnotations', added.map(formatAnnot));
+          added.forEach(function (annot) {
+            inFrame.add(annot.$tag);
+          });
         }
-      });
-      const deleted = prevAnnotations.filter(function (annot) {
-        return !inSidebar.has(annot.$tag);
-      });
-      prevAnnotations = state.annotations.annotations;
-      prevFrames = state.frames;
-
-      // We currently only handle adding and removing annotations from the frame
-      // when they are added or removed in the sidebar, but not re-anchoring
-      // annotations if their selectors are updated.
-      if (added.length > 0) {
-        bridge.call('loadAnnotations', added.map(formatAnnot));
-        added.forEach(function (annot) {
-          inFrame.add(annot.$tag);
+        deleted.forEach(function (annot) {
+          bridge.call('deleteAnnotation', formatAnnot(annot));
+          inFrame.delete(annot.$tag);
         });
-      }
-      deleted.forEach(function (annot) {
-        bridge.call('deleteAnnotation', formatAnnot(annot));
-        inFrame.delete(annot.$tag);
-      });
 
-      const frames = store.frames();
-      if (frames.length > 0) {
-        if (
-          frames.every(function (frame) {
-            return frame.isAnnotationFetchComplete;
-          })
-        ) {
-          if (publicAnns === 0 || publicAnns !== prevPublicAnns) {
-            bridge.call(
-              bridgeEvents.PUBLIC_ANNOTATION_COUNT_CHANGED,
-              publicAnns
-            );
-            prevPublicAnns = publicAnns;
+        if (frames.length > 0) {
+          if (
+            frames.every(function (frame) {
+              return frame.isAnnotationFetchComplete;
+            })
+          ) {
+            if (publicAnns === 0 || publicAnns !== prevPublicAnns) {
+              bridge.call(
+                bridgeEvents.PUBLIC_ANNOTATION_COUNT_CHANGED,
+                publicAnns
+              );
+              prevPublicAnns = publicAnns;
+            }
           }
         }
       }
-    });
+    );
   }
 
   /**

src/sidebar/services/persisted-defaults.js

+import { watch } from '../util/watch';
+
 /**
  * A service for reading and persisting convenient client-side defaults for
  * the (browser) user.
@@ -10,26 +12,19 @@ const DEFAULT_KEYS = {
 
 // @ngInject
 export default function persistedDefaults(localStorage, store) {
-  let lastDefaults;
-
   /**
    * Store subscribe callback for persisting changes to defaults. It will only
    * persist defaults that it "knows about" via `DEFAULT_KEYS`.
    */
-  function persistChangedDefaults() {
-    const latestDefaults = store.getDefaults();
-    for (let defaultKey in latestDefaults) {
+  function persistChangedDefaults(defaults, prevDefaults) {
+    for (let defaultKey in defaults) {
       if (
-        lastDefaults[defaultKey] !== latestDefaults[defaultKey] &&
+        prevDefaults[defaultKey] !== defaults[defaultKey] &&
         defaultKey in DEFAULT_KEYS
       ) {
-        localStorage.setItem(
-          DEFAULT_KEYS[defaultKey],
-          latestDefaults[defaultKey]
-        );
+        localStorage.setItem(DEFAULT_KEYS[defaultKey], defaults[defaultKey]);
       }
     }
-    lastDefaults = latestDefaults;
   }
 
   return {
@@ -45,10 +40,9 @@ export default function persistedDefaults(localStorage, store) {
         const defaultValue = localStorage.getItem(DEFAULT_KEYS[defaultKey]);
         store.setDefault(defaultKey, defaultValue);
       });
-      lastDefaults = store.getDefaults();
 
       // Listen for changes to those defaults from the store and persist them
-      store.subscribe(persistChangedDefaults);
+      watch(store.subscribe, () => store.getDefaults(), persistChangedDefaults);
     },
   };
 }

src/sidebar/services/test/frame-sync-test.js

@@ -55,7 +55,7 @@ describe('sidebar/services/frame-sync', function () {
 
   beforeEach(function () {
     fakeStore = createFakeStore(
-      { annotations: [] },
+      { annotations: { annotations: [] } },
       {
         connectFrame: sinon.stub(),
         destroyFrame: sinon.stub(),

src/sidebar/util/test/watch-test.js

+import { createStore } from 'redux';
+
+import { watch } from '../watch';
+
+function counterReducer(state = { a: 0, b: 0, c: 0 }, action) {
+  switch (action.type) {
+    case 'INCREMENT_A':
+      return { ...state, a: state.a + 1 };
+    case 'INCREMENT_B':
+      return { ...state, b: state.b + 1 };
+    case 'INCREMENT_C':
+      return { ...state, c: state.c + 1 };
+    default:
+      return state;
+  }
+}
+
+describe('sidebar/util/watch', () => {
+  /**
+   * Create a Redux store as a data source for testing the `watch` function.
+   */
+  function counterStore() {
+    return createStore(counterReducer);
+  }
+
+  describe('watch', () => {
+    it('runs callback when computed value changes', () => {
+      const callback = sinon.stub();
+      const store = counterStore();
+
+      watch(store.subscribe, () => store.getState().a, callback);
+
+      store.dispatch({ type: 'INCREMENT_A' });
+      assert.calledWith(callback, 1, 0);
+
+      store.dispatch({ type: 'INCREMENT_A' });
+      assert.calledWith(callback, 2, 1);
+    });
+
+    it('does not run callback if computed value did not change', () => {
+      const callback = sinon.stub();
+      const store = counterStore();
+
+      watch(store.subscribe, () => store.getState().a, callback);
+      store.dispatch({ type: 'INCREMENT_B' });
+
+      assert.notCalled(callback);
+    });
+
+    it('compares watched values using strict equality', () => {
+      const callback = sinon.stub();
+      const store = counterStore();
+
+      const newEmptyObject = () => ({});
+      watch(store.subscribe, newEmptyObject, callback);
+
+      store.dispatch({ type: 'INCREMENT_A' });
+      store.dispatch({ type: 'INCREMENT_A' });
+
+      // This will trigger the callback because we're comparing values by
+      // strict equality rather than by shallow equality.
+      assert.calledTwice(callback);
+      assert.calledWith(callback, {});
+    });
+
+    it('runs callback if any of multiple watched values changes', () => {
+      const callback = sinon.stub();
+      const store = counterStore();
+
+      watch(
+        store.subscribe,
+        [() => store.getState().a, () => store.getState().b],
+        callback
+      );
+
+      // Dispatch action that changes the first watched value.
+      store.dispatch({ type: 'INCREMENT_A' });
+      assert.calledWith(callback, [1, 0], [0, 0]);
+
+      // Dispatch action that changes the second watched value.
+      callback.resetHistory();
+      store.dispatch({ type: 'INCREMENT_B' });
+      assert.calledWith(callback, [1, 1], [1, 0]);
+
+      // Dispatch action that doesn't change either watched value.
+      callback.resetHistory();
+      store.dispatch({ type: 'INCREMENT_C' });
+      assert.notCalled(callback);
+    });
+
+    it('returns unsubscription function', () => {
+      const callback = sinon.stub();
+      const store = counterStore();
+
+      const unsubscribe = watch(
+        store.subscribe,
+        () => store.getState().a,
+        callback
+      );
+      store.dispatch({ type: 'INCREMENT_A' });
+
+      assert.calledWith(callback, 1, 0);
+
+      callback.resetHistory();
+      unsubscribe();
+      store.dispatch({ type: 'INCREMENT_A' });
+
+      assert.notCalled(callback);
+    });
+  });
+});

src/sidebar/util/watch.js

+import shallowEqual from 'shallowequal';
+
+/**
+ * Watch for changes of computed values.
+ *
+ * This utility is a shorthand for a common pattern for reacting to changes in
+ * some data source:
+ *
+ * ```
+ * let prevValue = getCurrentValue();
+ * subscribe(() => {
+ *   const newValue = getCurrentValue();
+ *   if (prevValue !== newValue) {
+ *     // Respond to change of value.
+ *     // ...
+ *
+ *     // Update previous value.
+ *     prevValue = new value;
+ *   }
+ * });
+ * ```
+ *
+ * Where `getCurrentValue` calculates the value of interest and
+ * `subscribe` registers a callback to receive change notifications for
+ * whatever data source (eg. a Redux store) is used by `getCurrentValue`.
+ *
+ * With the `watch` utility this becomes:
+ *
+ * ```
+ * watch(subscribe, getCurrentValue, (newValue, prevValue) => {
+ *   // Respond to change of value
+ * });
+ * ```
+ *
+ * `watch` can watch a single value, if the second argument is a function,
+ * or many if the second argument is an array of functions. In the latter case
+ * the callback will be invoked whenever _any_ of the watched values changes.
+ *
+ * Values are compared using strict equality (`===`).
+ *
+ * @param {(callback: Function) => Function} subscribe - Function used to
+ *   subscribe to notifications of _potential_ changes in the watched values.
+ * @param {Function|Array<Function>} watchFns - A function or array of functions
+ *   which return the current watched values
+ * @param {(current: any, previous: any) => any} callback -
+ *   A callback that is invoked when the watched values changed. It is passed
+ *   the current and previous values respectively. If `watchFns` is an array,
+ *   the `current` and `previous` arguments will be arrays of current and
+ *   previous values.
+ * @return {Function} - Return value of `subscribe`. Typically this is a
+ *   function that removes the subscription.
+ */
+export function watch(subscribe, watchFns, callback) {
+  const isArray = Array.isArray(watchFns);
+
+  const getWatchedValues = () =>
+    isArray ? watchFns.map(fn => fn()) : watchFns();
+
+  let prevValues = getWatchedValues();
+  const unsubscribe = subscribe(() => {
+    const values = getWatchedValues();
+
+    const equal = isArray
+      ? shallowEqual(values, prevValues)
+      : values === prevValues;
+
+    if (equal) {
+      return;
+    }
+
+    // Save and then update `prevValues` before invoking `callback` in case
+    // `callback` triggers another update.
+    const savedPrevValues = prevValues;
+    prevValues = values;
+
+    callback(values, savedPrevValues);
+  });
+
+  return unsubscribe;
+}