[lib] Make useAncestorThreads more efficient
038aad4a2924
Actions

Description

[lib] Make useAncestorThreads more efficient

Summary:
When working on another urgent issue, @ashoat pointed out that useAncestorThreads currently returns a new result array every time Redux changes. It made more sense to extract the relevant parts of the state out, then do the rest of the calculations inside a useMemo.

Addresses ENG-6160

Test Plan:
Tested three things here:

I made sure my change to calculating ancestorThreads from ancestorThreadInfos was a no-op:

const  ancestorThreads1  =  useSelector(state  =>
    ancestorThreadInfos(threadInfo.id)(state),
);
const ancestorThreads2  =  useSelector(ancestorThreadInfos(threadInfo.id));

console.log(_isEqual(ancestorThreads, ancestorThreads1));

I compared the result arrays returned from the original code and the new code:

function useAncestorThreads(
  threadInfo: ThreadInfo,
): $ReadOnlyArray<ThreadInfo> {
  // This is the original code on `master`
  const currentResult = useSelector(state => {
    if (!threadIsPending(threadInfo.id)) {
      const ancestorThreads = ancestorThreadInfos(threadInfo.id)(state);
      if (ancestorThreads.length > 1) {
        return ancestorThreads.slice(0, -1);
      }

      return ancestorThreads;
    }
    const genesisThreadInfo = threadInfoSelector(state)[genesis.id];
    return genesisThreadInfo ? [genesisThreadInfo] : [];
  });

  // This is the new code provided in this diff
  const ancestorThreads1 = useSelector(ancestorThreadInfos(threadInfo.id));

  const genesisThreadInfo1 = useSelector(
    state => threadInfoSelector(state)[genesis.id],
  );

  const newResult = React.useMemo(() => {
    if (!threadIsPending(threadInfo.id)) {
      return ancestorThreads1.length > 1
        ? ancestorThreads1.slice(0, -1)
        : ancestorThreads1;
    }
    return genesisThreadInfo1 ? [genesisThreadInfo1] : [];
  }, [ancestorThreads1, genesisThreadInfo1, threadInfo.id]);

  console.log('current vs. new result: ', _isEqual(currentResult, newResult));

  return newResult;
}

I went through the app and made sure I didn't notice any regressions from this. I also put log statements outside and inside the useMemo and made sure when unrelated parts of Redux change, only the outside log statement is run again

Reviewers: ashoat, atul

Reviewed By: ashoat, atul

Subscribers: atul, tomek, ashoat

Differential Revision: https://phab.comm.dev/D10467