Implement `useStoreProxy` hook

Implement a new `useStoreProxy` hook that provides access to read and
update the store in UI components. This will replace the existing
`useStore` hook.

The new hook has a more ergonomic API which should also prevent some of the
common mistakes made when using the previous hook.

`useStoreProxy` returns an ES proxy for the store. The UI component can
use the proxy as if it were using the store directly. The proxy however
tracks and caches the results of store selector calls. When the store
state changes the proxy checks whether any of the cached calls would
return different results and if so invalidates the cache and re-renders
the component.

src/sidebar/store/test/use-store-test.js

 import { mount } from 'enzyme';
 import { createElement } from 'preact';
 import { act } from 'preact/test-utils';
-import { createStore } from 'redux';
+import { createStore as createReduxStore } from 'redux';
 
-import useStore, { $imports } from '../use-store';
+import createStore from '../create-store';
+import useStore, { useStoreProxy, $imports } from '../use-store';
 
+// Plain Redux reducer used by `useStore` tests. Remove once `useStore` is removed.
 const initialState = { value: 10, otherValue: 20 };
 const reducer = (state = initialState, action) => {
   if (action.type === 'INCREMENT') {
@@ -16,90 +18,262 @@ const reducer = (state = initialState, action) => {
   }
 };
 
-describe('useStore', () => {
-  let renderCount;
-  let testStore;
-  let TestComponent;
-
-  beforeEach(() => {
-    renderCount = 0;
-
-    // eslint-disable-next-line react/display-name
-    TestComponent = () => {
-      renderCount += 1;
-      const aValue = useStore(store => store.getState().value);
-      return <div>{aValue}</div>;
-    };
-
-    testStore = createStore(reducer);
-    $imports.$mock({
-      '../util/service-context': {
-        useService: name => (name === 'store' ? testStore : null),
-      },
-    });
-  });
+// Store module for use with `createStore` in tests.
+const thingsModule = {
+  namespace: 'things',
+
+  init: () => ({
+    things: [],
+  }),
+
+  update: {
+    ADD_THING(state, action) {
+      if (state.things.some(t => t.id === action.thing.id)) {
+        return {};
+      }
+      return { things: [...state.things, action.thing] };
+    },
+  },
+
+  actions: {
+    addThing(id) {
+      return { type: 'ADD_THING', thing: { id } };
+    },
+  },
+
+  selectors: {
+    thingCount(state) {
+      return state.things.length;
+    },
+
+    getThing(state, id) {
+      return state.things.find(t => t.id === id);
+    },
+  },
+};
 
+describe('sidebar/store/use-store', () => {
   afterEach(() => {
     $imports.$restore();
   });
 
-  it('returns result of `callback(store)`', () => {
-    const wrapper = mount(<TestComponent />);
-    assert.equal(wrapper.text(), '10');
-  });
+  // Tests for deprecated `useStore` function.
+  describe('useStore', () => {
+    let renderCount;
+    let testStore;
+    let TestComponent;
+
+    beforeEach(() => {
+      renderCount = 0;
 
-  it('re-renders when the store changes and result of `callback(store)` also changes', () => {
-    // An update which changes result of `callback(store)` should cause a re-render.
-    const wrapper = mount(<TestComponent />);
-    act(() => {
-      testStore.dispatch({ type: 'INCREMENT' });
+      // eslint-disable-next-line react/display-name
+      TestComponent = () => {
+        renderCount += 1;
+        const aValue = useStore(store => store.getState().value);
+        return <div>{aValue}</div>;
+      };
+
+      testStore = createReduxStore(reducer);
+      $imports.$mock({
+        '../util/service-context': {
+          useService: name => (name === 'store' ? testStore : null),
+        },
+      });
     });
-    wrapper.update();
-    assert.equal(wrapper.text(), '11');
-
-    // The new result from `callback(store)` should be remembered so that another
-    // update which doesn't change the result doesn't cause a re-render.
-    const prevRenderCount = renderCount;
-    act(() => {
-      testStore.dispatch({ type: 'INCREMENT_OTHER' });
+
+    it('returns result of `callback(store)`', () => {
+      const wrapper = mount(<TestComponent />);
+      assert.equal(wrapper.text(), '10');
     });
-    wrapper.update();
-    assert.equal(renderCount, prevRenderCount);
-  });
 
-  it('does not re-render if the result of `callback(store)` did not change', () => {
-    mount(<TestComponent />);
-    const originalRenderCount = renderCount;
-    act(() => {
-      testStore.dispatch({ type: 'INCREMENT_OTHER' });
+    it('re-renders when the store changes and result of `callback(store)` also changes', () => {
+      // An update which changes result of `callback(store)` should cause a re-render.
+      const wrapper = mount(<TestComponent />);
+      act(() => {
+        testStore.dispatch({ type: 'INCREMENT' });
+      });
+      wrapper.update();
+      assert.equal(wrapper.text(), '11');
+
+      // The new result from `callback(store)` should be remembered so that another
+      // update which doesn't change the result doesn't cause a re-render.
+      const prevRenderCount = renderCount;
+      act(() => {
+        testStore.dispatch({ type: 'INCREMENT_OTHER' });
+      });
+      wrapper.update();
+      assert.equal(renderCount, prevRenderCount);
     });
-    assert.equal(renderCount, originalRenderCount);
-  });
 
-  it('warns if the callback always returns a different value', () => {
-    const warnOnce = sinon.stub();
-    $imports.$mock({
-      '../../shared/warn-once': warnOnce,
+    it('does not re-render if the result of `callback(store)` did not change', () => {
+      mount(<TestComponent />);
+      const originalRenderCount = renderCount;
+      act(() => {
+        testStore.dispatch({ type: 'INCREMENT_OTHER' });
+      });
+      assert.equal(renderCount, originalRenderCount);
+    });
+
+    it('warns if the callback always returns a different value', () => {
+      const warnOnce = sinon.stub();
+      $imports.$mock({
+        '../../shared/warn-once': warnOnce,
+      });
+      const BuggyComponent = () => {
+        // The result of the callback is an object with an `aValue` property
+        // which is a new array every time. This causes unnecessary re-renders.
+        useStore(() => ({ aValue: [] }));
+        return null;
+      };
+      mount(<BuggyComponent />);
+      assert.called(warnOnce);
+      assert.match(warnOnce.firstCall.args[0], /changes every time/);
+    });
+
+    it('unsubscribes when the component is unmounted', () => {
+      const unsubscribe = sinon.stub();
+      testStore.subscribe = sinon.stub().returns(unsubscribe);
+
+      const wrapper = mount(<TestComponent />);
+
+      assert.calledOnce(testStore.subscribe);
+      wrapper.unmount();
+      assert.calledOnce(unsubscribe);
     });
-    const BuggyComponent = () => {
-      // The result of the callback is an object with an `aValue` property
-      // which is a new array every time. This causes unnecessary re-renders.
-      useStore(() => ({ aValue: [] }));
-      return null;
-    };
-    mount(<BuggyComponent />);
-    assert.called(warnOnce);
-    assert.match(warnOnce.firstCall.args[0], /changes every time/);
   });
 
-  it('unsubscribes when the component is unmounted', () => {
-    const unsubscribe = sinon.stub();
-    testStore.subscribe = sinon.stub().returns(unsubscribe);
+  describe('useStoreProxy', () => {
+    let store;
+    let renderCount;
+
+    beforeEach(() => {
+      renderCount = 0;
+      store = createStore([thingsModule]);
+
+      store.addThing('foo');
+      store.addThing('bar');
+
+      $imports.$mock({
+        '../util/service-context': {
+          useService: name => (name === 'store' ? store : null),
+        },
+      });
+    });
+
+    function renderTestComponent() {
+      let proxy;
+
+      const TestComponent = () => {
+        ++renderCount;
+        proxy = useStoreProxy();
+
+        return <div>{proxy.thingCount()}</div>;
+      };
+
+      const wrapper = mount(<TestComponent />);
+      return { proxy, wrapper };
+    }
+
+    it('returns a proxy for the store', () => {
+      const addThingSpy = sinon.spy(store, 'addThing');
 
-    const wrapper = mount(<TestComponent />);
+      const { proxy } = renderTestComponent();
 
-    assert.calledOnce(testStore.subscribe);
-    wrapper.unmount();
-    assert.calledOnce(unsubscribe);
+      assert.ok(proxy);
+
+      // Test proxied selector method.
+      assert.deepEqual(proxy.getThing('foo'), { id: 'foo' });
+      assert.deepEqual(proxy.getThing('bar'), { id: 'bar' });
+
+      // Test proxied action dispatch.
+      proxy.addThing('baz');
+      assert.calledWith(addThingSpy, 'baz');
+    });
+
+    it('proxies non-function properties of the store', () => {
+      store.someString = 'foobar';
+
+      const { proxy } = renderTestComponent();
+
+      assert.equal(proxy.someString, 'foobar');
+    });
+
+    it('records and caches selector method calls', () => {
+      const getThingSpy = sinon.spy(store, 'getThing');
+
+      const { proxy } = renderTestComponent();
+
+      proxy.getThing('foo');
+      proxy.getThing('foo');
+
+      assert.calledWith(getThingSpy, 'foo');
+      assert.calledOnce(getThingSpy);
+      getThingSpy.resetHistory();
+
+      proxy.getThing('bar');
+      proxy.getThing('bar');
+
+      assert.calledWith(getThingSpy, 'bar');
+      assert.calledOnce(getThingSpy);
+    });
+
+    it('does not cache action dispatches', () => {
+      const addThingSpy = sinon.spy(store, 'addThing');
+
+      const { proxy } = renderTestComponent();
+
+      proxy.addThing('foo');
+      proxy.addThing('foo');
+
+      assert.calledTwice(addThingSpy);
+      assert.calledWith(addThingSpy, 'foo');
+    });
+
+    context('after a store update', () => {
+      it('clears cache and re-renders component if cache is invalid', () => {
+        const { wrapper } = renderTestComponent();
+        assert.equal(wrapper.text(), '2');
+        assert.equal(renderCount, 1);
+
+        // Dispatch an action which changes the store state used by the component.
+        act(() => {
+          store.addThing('baz');
+        });
+        wrapper.update();
+
+        assert.equal(renderCount, 2);
+        assert.equal(wrapper.text(), '3');
+      });
+
+      it('does not clear cache or re-render component if cache is still valid', () => {
+        const { wrapper } = renderTestComponent();
+        assert.equal(wrapper.text(), '2');
+        assert.equal(renderCount, 1);
+
+        // Dispatch an action which does not affect the store state used by the
+        // component.
+        act(() => {
+          store.addThing('foo'); // nb. `foo` item already exists in store.
+        });
+        wrapper.update();
+
+        assert.equal(renderCount, 1); // No re-render should happen.
+        assert.equal(wrapper.text(), '2');
+      });
+    });
+
+    it('unsubscribes from store when component is unmounted', () => {
+      const { wrapper } = renderTestComponent();
+      wrapper.unmount();
+
+      // Trigger a store change after unmounting. It should not re-render the
+      // component.
+      act(() => {
+        store.addThing('baz');
+      });
+      wrapper.update();
+
+      assert.equal(renderCount, 1);
+    });
   });
 });

src/sidebar/store/use-store.js

@@ -97,3 +97,142 @@ export default function useStore(callback) {
 
   return lastResult.current;
 }
+
+/**
+ * Result of a cached store selector method call.
+ */
+class CacheEntry {
+  /**
+   * @param {string} name - Method name
+   * @param {Function} method - Method implementation
+   * @param {any[]} args - Arguments to the selector
+   * @param {any} result - Result of the invocation
+   */
+  constructor(name, method, args, result) {
+    this.name = name;
+    this.method = method;
+    this.args = args;
+    this.result = result;
+  }
+
+  /**
+   * @param {string} name
+   * @param {any[]} args
+   */
+  matches(name, args) {
+    return (
+      this.name === name && this.args.every((value, i) => args[i] === value)
+    );
+  }
+}
+
+/**
+ * Return a wrapper around the `store` service that UI components can use to
+ * extract data from the store and call actions on it.
+ *
+ * Unlike using the `store` service directly, the wrapper tracks what data from
+ * the store the current component uses and re-renders the component when it
+ * changes.
+ *
+ * The returned wrapper has the same API as the store itself.
+ *
+ * @example
+ *   function MyComponent() {
+ *     const store = useStoreProxy();
+ *     const currentUser = store.currentUser();
+ *
+ *     return (
+ *       <div>
+ *         Current user: {currentUser}.
+ *         <button onClick={() => store.logOut()}>Log out</button>
+ *       </div>
+ *     );
+ *   }
+ *
+ * @return {SidebarStore}
+ */
+export function useStoreProxy() {
+  const store = useService('store');
+
+  // Hack to trigger a component re-render.
+  const [, forceUpdate] = useReducer(x => x + 1, 0);
+
+  // Cache of store method calls made by the current UI component and associated
+  // results. This is currently just an array on the assumption that it will
+  // only have a small number of entries. It could be changed to a map keyed
+  // by method name to handle many entries better.
+  const cacheRef = useRef(/** @type {CacheEntry[]} */ ([]));
+  const cache = cacheRef.current;
+
+  // Create the wrapper around the store.
+  const proxy = useRef(/** @type {SidebarStore|null} */ (null));
+  if (!proxy.current) {
+    // Cached method wrappers.
+    const wrappedMethods = {};
+
+    /**
+     * @param {typeof store} target
+     * @param {string} prop
+     */
+    const get = (target, prop) => {
+      const original = target[prop];
+      if (typeof original !== 'function') {
+        return original;
+      }
+
+      // Check for pre-existing method wrapper.
+      let wrapped = wrappedMethods[prop];
+      if (wrapped) {
+        return wrapped;
+      }
+
+      // Create method wrapper.
+      wrapped = (...args) => {
+        const cacheEntry = cache.find(entry => entry.matches(prop, args));
+        if (cacheEntry) {
+          return cacheEntry.result;
+        }
+
+        // Call the original method. It may be a selector that does not modify
+        // the store but returns a result, or an action that modifies the state.
+        const prevState = store.getState();
+        const result = original.apply(target, args);
+        const newState = store.getState();
+
+        if (prevState === newState) {
+          cache.push(new CacheEntry(prop, original, args, result));
+        }
+        return result;
+      };
+      wrappedMethods[prop] = wrapped;
+
+      return wrapped;
+    };
+
+    proxy.current = new Proxy(store, { get });
+  }
+
+  // Register a subscriber which clears cache and re-renders component when
+  // relevant store state changes.
+  useEffect(() => {
+    const cleanup = store.subscribe(() => {
+      const invalidEntry = cache.find(
+        // nb. A potential problem here is that the method arguments may refer
+        // to things which no longer exist (for example, an object ID for an object
+        // which has been unloaded). It is assumed that store selector methods are
+        // robust to this.
+        entry => entry.method.apply(store, entry.args) !== entry.result
+      );
+
+      if (invalidEntry) {
+        // We currently just invalidate the entire cache when any entry becomes
+        // invalid, but we could do more fine-grained checks.
+        cache.splice(0, cache.length);
+        forceUpdate(0);
+      }
+    });
+    return cleanup;
+  }, [cache, store]);
+
+  return proxy.current;
+}