Refactor `useRootThread` to memoize correctly

Also convert `selectionState` to use `createSelector` to avoid it
returning a different reference on each call.

src/sidebar/components/hooks/test/use-root-thread-test.js

+import { mount } from 'enzyme';
+import { createElement } from 'preact';
+
 import useRootThread from '../use-root-thread';
 import { $imports } from '../use-root-thread';
 
 describe('sidebar/components/hooks/use-root-thread', () => {
   let fakeStore;
   let fakeThreadAnnotations;
+  let lastRootThread;
 
   beforeEach(() => {
     fakeStore = {
@@ -13,12 +17,20 @@ describe('sidebar/components/hooks/use-root-thread', () => {
       selectionState: sinon.stub().returns({ hi: 'there' }),
       userFilter: sinon.stub().returns('hotspur'),
     };
-    fakeThreadAnnotations = sinon.stub();
+    fakeThreadAnnotations = sinon.stub().returns('fakeThreadAnnotations');
 
     $imports.$mock({
       '../../store/use-store': callback => callback(fakeStore),
       '../../util/thread-annotations': fakeThreadAnnotations,
     });
+
+    // Mount a dummy component to be able to use the `useRootThread` hook
+    // Do things that cause `useRootThread` to recalculate in the store and
+    // test them (hint: use `act`)
+    function DummyComponent() {
+      lastRootThread = useRootThread();
+    }
+    mount(<DummyComponent />);
   });
 
   afterEach(() => {
@@ -26,15 +38,13 @@ describe('sidebar/components/hooks/use-root-thread', () => {
   });
 
   it('should return results of `threadAnnotations` with current thread state', () => {
-    fakeThreadAnnotations.returns('a tisket, a tasket');
-
-    const results = useRootThread();
-
     const threadState = fakeThreadAnnotations.getCall(0).args[0];
+
     assert.deepEqual(threadState.annotations, ['1', '2']);
     assert.equal(threadState.selection.filterQuery, 'itchy');
     assert.equal(threadState.route, '66');
     assert.equal(threadState.selection.filters.user, 'hotspur');
-    assert.equal(results, 'a tisket, a tasket');
+
+    assert.equal(lastRootThread, 'fakeThreadAnnotations');
   });
 });

src/sidebar/components/hooks/use-root-thread.js

+import { useMemo } from 'preact/hooks';
+
 import useStore from '../../store/use-store';
 import threadAnnotations from '../../util/thread-annotations';
 
@@ -14,19 +16,22 @@ export default function useRootThread() {
   const query = useStore(store => store.filterQuery());
   const route = useStore(store => store.route());
   const selectionState = useStore(store => store.selectionState());
-  const filters = useStore(store => {
+  const userFilter = useStore(store => store.userFilter());
+
+  const threadState = useMemo(() => {
     /** @type {Object.<string,string>} */
     const filters = {};
-    const userFilter = store.userFilter();
     if (userFilter) {
       filters.user = userFilter;
     }
-    return filters;
-  });
-
-  return threadAnnotations({
+    return {
       annotations,
+      filters,
+      query,
       route,
       selection: { ...selectionState, filterQuery: query, filters },
-  });
+    };
+  }, [annotations, query, route, selectionState, userFilter]);
+
+  return threadAnnotations(threadState);
 }

src/sidebar/store/modules/selection.js

@@ -337,16 +337,18 @@ function selectedTab(state) {
 /**
  * @return {SelectionState}
  */
-function selectionState(state) {
-  const selectionState = {
-    expanded: expandedMap(state),
-    forcedVisible: forcedVisibleAnnotations(state),
-    selected: selectedAnnotations(state),
-    sortKey: sortKey(state),
-    selectedTab: selectedTab(state),
+const selectionState = createSelector(
+  state => state,
+  selection => {
+    return {
+      expanded: expandedMap(selection),
+      forcedVisible: forcedVisibleAnnotations(selection),
+      selected: selectedAnnotations(selection),
+      sortKey: sortKey(selection),
+      selectedTab: selectedTab(selection),
     };
-  return selectionState;
-}
+  }
+);
 
 /**
  * Retrieve the current sort option key

src/sidebar/test/integration/threading-test.js

 import { mount } from 'enzyme';
 import { createElement } from 'preact';
+import { useReducer } from 'preact/hooks';
 import { act } from 'preact/test-utils';
 
 import { Injector } from '../../../shared/injector';
@@ -39,6 +40,7 @@ const fixtures = {
 describe('integration: annotation threading', () => {
   let lastRootThread;
   let store;
+  let forceUpdate;
 
   beforeEach(function () {
     const fakeFeatures = {
@@ -56,6 +58,7 @@ describe('integration: annotation threading', () => {
     // test them (hint: use `act`)
     function DummyComponent() {
       lastRootThread = useRootThread();
+      [, forceUpdate] = useReducer(x => x + 1, 0);
     }
 
     store = container.get('store');
@@ -67,6 +70,24 @@ describe('integration: annotation threading', () => {
     );
   });
 
+  it('should update root thread only when relevant state changes', () => {
+    let prevRootThread = lastRootThread;
+
+    // Make a change which affects the thread.
+    act(() => {
+      store.addAnnotations(fixtures.annotations);
+    });
+
+    assert.notEqual(lastRootThread, prevRootThread);
+    prevRootThread = lastRootThread;
+
+    // Re-render the UI without changing any of the data that affects the thread.
+    act(() => {
+      forceUpdate();
+    });
+    assert.equal(lastRootThread, prevRootThread);
+  });
+
   it('should display newly loaded annotations', () => {
     act(() => {
       store.addAnnotations(fixtures.annotations);
@@ -109,9 +130,9 @@ describe('integration: annotation threading', () => {
         store.setSortKey(testCase.sortKey);
       });
 
-      const actualOrder = lastRootThread.children.map(thread => {
-        return thread.annotation.id;
-      });
+      const actualOrder = lastRootThread.children.map(
+        thread => thread.annotation.id
+      );
       assert.deepEqual(actualOrder, testCase.expectedOrder);
     });
   });