Iterating on Profiling tab suspense. Stashing changes but planning to refactor immediately.

src/devtools/store.js

@@ -275,16 +275,24 @@ export default class Store extends EventEmitter {
   }
 
   startProfiling(): void {
+    this._bridge.send('startProfiling');
+
     // Invalidate suspense cache if profiling data is being (re-)recorded.
+    // Note that we clear now because any existing data is "stale".
     this._profilingCache.invalidate();
 
-    this._bridge.send('startProfiling');
     this._isProfiling = false;
     this.emit('isProfiling');
   }
 
   stopProfiling(): void {
     this._bridge.send('stopProfiling');
+
+    // Invalidate suspense cache if profiling data is being (re-)recorded.
+    // Note that we clear again, in case any views read from the cache while profiling.
+    // (That would have resolved a now-stale value without any profiling data.)
+    this._profilingCache.invalidate();
+
     this._isProfiling = false;
     this.emit('isProfiling');
   }

src/devtools/views/Profiler/FilterModal.js

@@ -1,8 +1,7 @@
 // @flow
 
-import React, { useCallback, useContext, useEffect, useRef } from 'react';
-import { useModalDismissSignal } from '../hooks';
-import { ProfilerContext } from './ProfilerContext';
+import React, { useCallback, useEffect, useRef } from 'react';
+import { useLocalStorage, useModalDismissSignal } from '../hooks';
 
 import styles from './FilterModal.css';
 
@@ -11,12 +10,14 @@ type Props = {|
 |};
 
 export default function FilterModal({ dismissModal }: Props) {
-  const {
-    isMinCommitDurationEnabled,
-    minCommitDuration,
-    setMinCommitDuration,
-    setIsMinCommitDurationEnabled,
-  } = useContext(ProfilerContext);
+  const [
+    isCommitFilterEnabled,
+    setIsCommitFilterEnabled,
+  ] = useLocalStorage<boolean>('isCommitFilterEnabled', false);
+  const [minCommitDuration, setMinCommitDuration] = useLocalStorage<number>(
+    'minCommitDuration',
+    0
+  );
 
   const handleNumberChange = useCallback(
     ({ currentTarget }) => {
@@ -30,14 +31,14 @@ export default function FilterModal({ dismissModal }: Props) {
 
   const handleEnabledChange = useCallback(
     ({ currentTarget }) => {
-      setIsMinCommitDurationEnabled(currentTarget.checked);
+      setIsCommitFilterEnabled(currentTarget.checked);
       if (currentTarget.checked) {
         if (inputRef.current !== null) {
           inputRef.current.focus();
         }
       }
     },
-    [setIsMinCommitDurationEnabled]
+    [setIsCommitFilterEnabled]
   );
 
   const inputRef = useRef<HTMLInputElement | null>(null);
@@ -56,7 +57,7 @@ export default function FilterModal({ dismissModal }: Props) {
       <div className={styles.Modal} ref={modalRef}>
         <label>
           <input
-            checked={isMinCommitDurationEnabled}
+            checked={isCommitFilterEnabled}
             onChange={handleEnabledChange}
             type="checkbox"
           />{' '}

src/devtools/views/Profiler/Profiler.js

@@ -1,7 +1,11 @@
 // @flow
 
-import React, { useCallback, useContext, useState } from 'react';
-import { ProfilerContext, ProfilerContextController } from './ProfilerContext';
+import React, { Suspense, useCallback, useContext, useState } from 'react';
+import { ProfilerDataContextController } from './ProfilerDataContext';
+import {
+  ProfilerStatusContext,
+  ProfilerStatusContextController,
+} from './ProfilerStatusContext';
 import Button from '../Button';
 import ButtonIcon from '../ButtonIcon';
 import TabBar from '../TabBar';
@@ -15,14 +19,23 @@ export type Props = {||};
 
 export default function ProfilerOuter(_: Props) {
   return (
-    <ProfilerContextController>
-      <ProfilerInner />
-    </ProfilerContextController>
+    <ProfilerStatusContextController>
+      <Suspense fallback={<ProfilerFallback />}>
+        <ProfilerDataContextController>
+          <ProfilerInner />
+        </ProfilerDataContextController>
+      </Suspense>
+    </ProfilerStatusContextController>
   );
 }
 
+function ProfilerFallback() {
+  // TODO (profiling) Real fallback UI
+  return null;
+}
+
 function ProfilerInner(_: Props) {
-  const { hasProfilingData, isProfiling } = useContext(ProfilerContext);
+  const { hasProfilingData, isProfiling } = useContext(ProfilerStatusContext);
 
   const showProfilingControls = !isProfiling && hasProfilingData;
 

src/devtools/views/Profiler/ProfilerContext.js

@@ -1,149 +0,0 @@
-// @flow
-
-import React, {
-  createContext,
-  useCallback,
-  useContext,
-  useMemo,
-  useState,
-} from 'react';
-import { useSubscription } from '../hooks';
-import { TreeContext } from 'src/devtools/views/Elements/TreeContext';
-import { StoreContext } from '../context';
-import { useLocalStorage } from '../hooks';
-import Store from '../../store';
-
-type Context = {|
-  commitIndex: number,
-  hasProfilingData: boolean,
-  isMinCommitDurationEnabled: boolean,
-  isProfiling: boolean,
-  minCommitDuration: number,
-  rendererID: number | null,
-  rootID: number | null,
-  setCommitIndex: (value: number) => void,
-  setMinCommitDuration: (value: number) => void,
-  setIsMinCommitDurationEnabled: (value: boolean) => void,
-  startProfiling(value: boolean): void,
-  stopProfiling(value: boolean): void,
-|};
-
-const ProfilerContext = createContext<Context>(((null: any): Context));
-ProfilerContext.displayName = 'ProfilerContext';
-
-type StoreProfilingState = {|
-  hasProfilingData: boolean,
-  isProfiling: boolean,
-|};
-
-type Props = {|
-  children: React$Node,
-|};
-
-function ProfilerContextController({ children }: Props) {
-  const store = useContext(StoreContext);
-
-  const { isProfiling, hasProfilingData } = useSubscription<
-    StoreProfilingState,
-    Store
-  >(
-    useMemo(
-      () => ({
-        source: store,
-        getCurrentValue: (store: Store) => ({
-          hasProfilingData: store.hasProfilingData,
-          isProfiling: store.isProfiling,
-        }),
-        subscribe: (store: Store, callback: Function) => {
-          store.addListener('isProfiling', callback);
-          return () => store.removeListener('isProfiling', callback);
-        },
-      }),
-      [store]
-    )
-  );
-
-  const startProfiling = useCallback(() => store.startProfiling(), [store]);
-  const stopProfiling = useCallback(() => store.stopProfiling(), [store]);
-
-  // TODO (profiling) The browser extension is a multi-root app,
-  // so ti won't work for the "Profiling" root to depend on a value that's set by the "Elements" root.
-  // We'll either need to lift that state up into the (shared) Store,
-  // or use a portal to share the contexts themselves between Chrome tabs.
-  const { selectedElementID } = useContext(TreeContext);
-
-  // If no root is selected, assume the first root.
-  // Many React apps are single root anyway.
-  let rendererID = null;
-  let rootID = null;
-  if (selectedElementID) {
-    rendererID = store.getRendererIDForElement(
-      ((selectedElementID: any): number)
-    );
-    rootID = store.getRootIDForElement(((selectedElementID: any): number));
-  } else if (store.roots.length > 0) {
-    rootID = store.roots[0];
-    rendererID = store.getRendererIDForElement(((rootID: any): number));
-  }
-
-  const [commitIndex, setCommitIndex] = useState(0);
-  const [prevRootID, setPrevRootID] = useState(rootID);
-  if (prevRootID !== rootID) {
-    setPrevRootID(rootID);
-    setCommitIndex(0);
-  }
-
-  const [prevIsProfiling, setPrevIsProfiling] = useState(isProfiling);
-  if (prevIsProfiling !== isProfiling) {
-    setPrevIsProfiling(isProfiling);
-    setCommitIndex(0);
-  }
-
-  const [
-    isMinCommitDurationEnabled,
-    setIsMinCommitDurationEnabled,
-  ] = useLocalStorage<boolean>('isMinCommitDurationEnabled', false);
-  const [minCommitDuration, setMinCommitDuration] = useLocalStorage<number>(
-    'minCommitDuration',
-    0
-  );
-
-  const value = useMemo(
-    () => ({
-      commitIndex,
-      hasProfilingData,
-      isMinCommitDurationEnabled,
-      isProfiling,
-      minCommitDuration,
-      rendererID,
-      rootID,
-      setMinCommitDuration,
-      setIsMinCommitDurationEnabled,
-      setCommitIndex,
-      startProfiling,
-      stopProfiling,
-    }),
-    [
-      commitIndex,
-      hasProfilingData,
-      isMinCommitDurationEnabled,
-      isProfiling,
-      minCommitDuration,
-      rendererID,
-      rootID,
-      setMinCommitDuration,
-      setIsMinCommitDurationEnabled,
-      setCommitIndex,
-      startProfiling,
-      stopProfiling,
-    ]
-  );
-
-  return (
-    <ProfilerContext.Provider value={value}>
-      {children}
-    </ProfilerContext.Provider>
-  );
-}
-
-export { ProfilerContext, ProfilerContextController };

src/devtools/views/Profiler/ProfilerDataContext.js

@@ -0,0 +1,94 @@
+// @flow
+
+import React, { createContext, useContext, useMemo, useState } from 'react';
+import { TreeContext } from 'src/devtools/views/Elements/TreeContext';
+import { StoreContext } from '../context';
+import { useLocalStorage } from '../hooks';
+import { ProfilerStatusContext } from './ProfilerStatusContext';
+
+type Context = {|
+  commitIndex: number | null,
+  filteredCommitIndices: Array<number>,
+  rendererID: number | null,
+  rootID: number | null,
+  setCommitIndex: (value: number) => void,
+|};
+
+const ProfilerDataContext = createContext<Context>(((null: any): Context));
+ProfilerDataContext.displayName = 'ProfilerDataContext';
+
+type Props = {|
+  children: React$Node,
+|};
+
+function ProfilerDataContextController({ children }: Props) {
+  const store = useContext(StoreContext);
+
+  // TODO (profiling) The browser extension is a multi-root app,
+  // so it won't work for the "Profiling" root to depend on a value that's set by the "Elements" root.
+  // We'll either need to lift that state up into the (shared) Store,
+  // or use a portal to share the contexts themselves between Chrome tabs.
+  const { selectedElementID } = useContext(TreeContext);
+
+  // If no root is selected, assume the first root; many React apps are single root anyway.
+  let rendererID = null;
+  let rootID = null;
+  if (selectedElementID) {
+    rendererID = store.getRendererIDForElement(
+      ((selectedElementID: any): number)
+    );
+    rootID = store.getRootIDForElement(((selectedElementID: any): number));
+  } else if (store.roots.length > 0) {
+    rootID = store.roots[0];
+    rendererID = store.getRendererIDForElement(((rootID: any): number));
+  }
+
+  // This value is important because it ensure we re-render after our suspense cache has been cleared.
+  const { isProfiling } = useContext(ProfilerStatusContext);
+
+  const profilingSummary = store.profilingCache.ProfilingSummary.read({
+    rendererID: ((rendererID: any): number),
+    rootID: ((rootID: any): number),
+  });
+
+  const [isCommitFilterEnabled] = useLocalStorage<boolean>(
+    'isCommitFilterEnabled',
+    false
+  );
+  const [minCommitDuration] = useLocalStorage<number>('minCommitDuration', 0);
+  const { commitDurations } = profilingSummary;
+  const filteredCommitIndices = useMemo(() => {
+    const array = [];
+    if (!isProfiling) {
+      for (let i = 0; i < commitDurations.length; i++) {
+        if (!isCommitFilterEnabled || commitDurations[i] >= minCommitDuration) {
+          array.push(i);
+        }
+      }
+    }
+    return array;
+  }, [commitDurations, isCommitFilterEnabled, isProfiling, minCommitDuration]);
+
+  const [commitIndex, setCommitIndex] = useState<number | null>(
+    commitDurations.length > 0 ? 0 : null
+  );
+
+  const value = useMemo(
+    () => ({
+      commitIndex,
+      filteredCommitIndices,
+      rendererID,
+      rootID,
+      setCommitIndex,
+    }),
+    [commitIndex, filteredCommitIndices, rendererID, rootID, setCommitIndex]
+  );
+
+  return (
+    <ProfilerDataContext.Provider value={value}>
+      {children}
+    </ProfilerDataContext.Provider>
+  );
+}
+
+export { ProfilerDataContext, ProfilerDataContextController };

src/devtools/views/Profiler/ProfilerStatusContext.js

@@ -0,0 +1,69 @@
+// @flow
+
+import React, { createContext, useCallback, useContext, useMemo } from 'react';
+import { useSubscription } from '../hooks';
+import { StoreContext } from '../context';
+import Store from '../../store';
+
+type Context = {|
+  hasProfilingData: boolean,
+  isProfiling: boolean,
+  startProfiling(value: boolean): void,
+  stopProfiling(value: boolean): void,
+|};
+
+const ProfilerStatusContext = createContext<Context>(((null: any): Context));
+ProfilerStatusContext.displayName = 'ProfilerStatusContext';
+
+type StoreProfilingState = {|
+  hasProfilingData: boolean,
+  isProfiling: boolean,
+|};
+
+type Props = {|
+  children: React$Node,
+|};
+
+function ProfilerStatusContextController({ children }: Props) {
+  const store = useContext(StoreContext);
+
+  const subscription = useMemo(
+    () => ({
+      getCurrentValue: () => ({
+        hasProfilingData: store.hasProfilingData,
+        isProfiling: store.isProfiling,
+      }),
+      subscribe: (callback: Function) => {
+        store.addListener('isProfiling', callback);
+        return () => store.removeListener('isProfiling', callback);
+      },
+    }),
+    [store]
+  );
+
+  const { isProfiling, hasProfilingData } = useSubscription<
+    StoreProfilingState,
+    Store
+  >(subscription);
+
+  const startProfiling = useCallback(() => store.startProfiling(), [store]);
+  const stopProfiling = useCallback(() => store.stopProfiling(), [store]);
+
+  const value = useMemo(
+    () => ({
+      hasProfilingData,
+      isProfiling,
+      startProfiling,
+      stopProfiling,
+    }),
+    [hasProfilingData, isProfiling, startProfiling, stopProfiling]
+  );
+
+  return (
+    <ProfilerStatusContext.Provider value={value}>
+      {children}
+    </ProfilerStatusContext.Provider>
+  );
+}
+
+export { ProfilerStatusContext, ProfilerStatusContextController };

src/devtools/views/Profiler/RecordToggle.js

@@ -3,7 +3,7 @@
 import React, { useContext } from 'react';
 import Button from '../Button';
 import ButtonIcon from '../ButtonIcon';
-import { ProfilerContext } from './ProfilerContext';
+import { ProfilerStatusContext } from './ProfilerStatusContext';
 
 import styles from './RecordToggle.css';
 
@@ -11,7 +11,7 @@ export type Props = {||};
 
 export default function RecordToggle(_: Props) {
   const { isProfiling, startProfiling, stopProfiling } = useContext(
-    ProfilerContext
+    ProfilerStatusContext
   );
 
   return (

src/devtools/views/Profiler/SnapshotCommitList.js

@@ -2,6 +2,7 @@
 
 import React, {
   useCallback,
+  useContext,
   useEffect,
   useMemo,
   useRef,
@@ -11,67 +12,64 @@ import AutoSizer from 'react-virtualized-auto-sizer';
 import { FixedSizeList } from 'react-window';
 import SnapshotCommitListItem from './SnapshotCommitListItem';
 import { minBarWidth } from './constants';
+import { ProfilerDataContext } from './ProfilerDataContext';
+import { StoreContext } from '../context';
 
 export type ItemData = {|
   commitDurations: Array<number>,
+  commitIndex: number | null,
   commitTimes: Array<number>,
+  filteredCommitIndices: Array<number>,
   isMouseDown: boolean,
   maxDuration: number,
-  selectedCommitIndex: number,
   setCommitIndex: (index: number) => void,
 |};
 
-type Props = {|
-  commitDurations: Array<number>,
-  commitTimes: Array<number>,
-  selectedCommitIndex: number,
-  setCommitIndex: (index: number) => void,
-  viewNextCommit: () => void,
-  viewPrevCommit: () => void,
-|};
+type Props = {||};
 
-export default function SnapshotCommitList(props: Props) {
+export default function SnapshotCommitList(_: Props) {
   return (
     <AutoSizer>
-      {({ height, width }) => <List height={height} width={width} {...props} />}
+      {({ height, width }) => <List height={height} width={width} />}
     </AutoSizer>
   );
 }
 
 type ListProps = {|
   height: number,
-  commitDurations: Array<number>,
-  commitTimes: Array<number>,
-  selectedCommitIndex: number,
-  setCommitIndex: (index: number) => void,
-  viewNextCommit: () => void,
-  viewPrevCommit: () => void,
   width: number,
 |};
 
-function List({
-  height,
-  commitDurations,
-  commitTimes,
-  selectedCommitIndex,
-  setCommitIndex,
-  viewNextCommit,
-  viewPrevCommit,
-  width,
-}: ListProps) {
+function List({ height, width }: ListProps) {
   const listRef = useRef<FixedSizeList<ItemData> | null>(null);
   const [isMouseDown, setIsMouseDown] = useState(false);
-  const prevSelectedCommitIndexRef = useRef<number>(-1);
+  const prevCommitIndexRef = useRef<number | null>(null);
+
+  const { profilingCache } = useContext(StoreContext);
+  const {
+    commitIndex,
+    filteredCommitIndices,
+    rendererID,
+    rootID,
+    setCommitIndex,
+  } = useContext(ProfilerDataContext);
+
+  const { commitDurations, commitTimes } = profilingCache.ProfilingSummary.read(
+    {
+      rendererID: ((rendererID: any): number),
+      rootID: ((rootID: any): number),
+    }
+  );
 
   // Make sure any newly selected snapshot is visible within the list.
   useEffect(() => {
-    if (selectedCommitIndex !== prevSelectedCommitIndexRef.current) {
-      prevSelectedCommitIndexRef.current = selectedCommitIndex;
-      if (listRef.current !== null) {
-        listRef.current.scrollToItem(selectedCommitIndex);
+    if (commitIndex !== prevCommitIndexRef.current) {
+      prevCommitIndexRef.current = commitIndex;
+      if (commitIndex !== null && listRef.current !== null) {
+        listRef.current.scrollToItem(commitIndex);
       }
     }
-  }, [listRef, selectedCommitIndex]);
+  }, [listRef, commitIndex]);
 
   const handleMouseDown = useCallback(() => {
     setIsMouseDown(true);
@@ -104,18 +102,20 @@ function List({
   const itemData = useMemo<ItemData>(
     () => ({
       commitDurations,
+      commitIndex,
       commitTimes,
+      filteredCommitIndices,
       isMouseDown,
       maxDuration,
-      selectedCommitIndex,
       setCommitIndex,
     }),
     [
       commitDurations,
+      commitIndex,
       commitTimes,
+      filteredCommitIndices,
       isMouseDown,
       maxDuration,
-      selectedCommitIndex,
       setCommitIndex,
     ]
   );
@@ -130,7 +130,7 @@ function List({
         <FixedSizeList
           direction="horizontal"
           height={height}
-          itemCount={commitDurations.length}
+          itemCount={filteredCommitIndices.length}
           itemData={itemData}
           itemSize={itemSize}
           ref={(listRef: any) /* Flow bug? */}

src/devtools/views/Profiler/SnapshotCommitListItem.js

@@ -17,13 +17,16 @@ type Props = {
 function SnapshotCommitListItem({ data: itemData, index, style }: Props) {
   const {
     commitDurations,
+    commitIndex,
     commitTimes,
+    filteredCommitIndices,
     isMouseDown,
     maxDuration,
-    selectedCommitIndex,
     setCommitIndex,
   } = itemData;
 
+  index = filteredCommitIndices[index];
+
   const commitDuration = commitDurations[index];
   const commitTime = commitTimes[index];
 
@@ -35,7 +38,7 @@ function SnapshotCommitListItem({ data: itemData, index, style }: Props) {
   // Guard against commits with duration 0
   const percentage =
     Math.min(1, Math.max(0, commitDuration / maxDuration)) || 0;
-  const isSelected = selectedCommitIndex === index;
+  const isSelected = commitIndex === index;
 
   // Leave a 1px gap between snapshots
   const width = parseFloat(style.width) - 1;

src/devtools/views/Profiler/SnapshotSelector.js

@@ -1,106 +1,72 @@
 // @flow
 
-import React, { Fragment, Suspense, useContext } from 'react';
+import React, { Fragment, useCallback, useContext } from 'react';
 import Button from '../Button';
 import ButtonIcon from '../ButtonIcon';
-import { StoreContext } from '../context';
-import { ProfilerContext } from './ProfilerContext';
+import { ProfilerDataContext } from './ProfilerDataContext';
 import SnapshotCommitList from './SnapshotCommitList';
 
 import styles from './SnapshotSelector.css';
 
 export type Props = {||};
 
-export default function SnapshotSelectorSuspense(_: Props) {
-  return (
-    <Suspense fallback={<SnapshotSelectorFallback />}>
-      <SnapshotSelector />
-    </Suspense>
-  );
-}
-
-function SnapshotSelector(_: Props) {
-  const { profilingCache } = useContext(StoreContext);
+export default function SnapshotSelector(_: Props) {
   const {
     commitIndex,
-    isMinCommitDurationEnabled,
-    minCommitDuration,
+    filteredCommitIndices,
     rendererID,
     rootID,
     setCommitIndex,
-  } = useContext(ProfilerContext);
+  } = useContext(ProfilerDataContext);
+
+  const numCommits = filteredCommitIndices.length;
+  let currentCommitNumber = '-';
+  if (numCommits > 0) {
+    currentCommitNumber = `${commitIndex + 1}`.padStart(
+      `${numCommits}`.length,
+      '0'
+    );
+  }
+
+  const viewNextCommit = useCallback(() => {
+    const nextCommitIndex = Math.min(
+      ((commitIndex: any): number) + 1,
+      filteredCommitIndices.length - 1
+    );
+    setCommitIndex(filteredCommitIndices[nextCommitIndex]);
+  }, [commitIndex, filteredCommitIndices, setCommitIndex]);
+  const viewPrevCommit = useCallback(() => {
+    const nextCommitIndex = Math.max(((commitIndex: any): number) - 1, 0);
+    setCommitIndex(filteredCommitIndices[nextCommitIndex]);
+  }, [commitIndex, filteredCommitIndices, setCommitIndex]);
 
   if (rendererID === null || rootID === null) {
     return null;
   }
 
-  const { commitDurations, commitTimes } = profilingCache.ProfilingSummary.read(
-    {
-      rendererID: ((rendererID: any): number),
-      rootID: ((rootID: any): number),
-    }
-  );
-
-  // TODO (profiling) This is not sufficient; index here doesn't map to a meaningful index in the profiling data.
-  let filteredCommitDurations = commitDurations;
-  let filteredCommitTimes = commitTimes;
-  if (isMinCommitDurationEnabled) {
-    filteredCommitDurations = [];
-    filteredCommitTimes = [];
-    for (let i = 0; i < commitDurations.length; i++) {
-      if (commitDurations[i] >= minCommitDuration) {
-        filteredCommitDurations.push(commitDurations[i]);
-        filteredCommitTimes.push(commitTimes[i]);
-      }
-    }
-  }
-
-  const numCommits = filteredCommitDurations.length;
-  const currentCommitNumber = `${
-    numCommits > 0 ? commitIndex + 1 : '-'
-  }`.padStart(`${numCommits}`.length, '0');
-
-  // TODO (profiler) We need to guard commit index and share filterd statuses in a better way.
-
-  const viewNextCommit = () => {
-    setCommitIndex(Math.min(commitIndex + 1, numCommits - 1));
-  };
-  const viewPrevCommit = () => {
-    setCommitIndex(Math.max(commitIndex - 1, 0));
-  };
-
   return (
     <Fragment>
       <div className={styles.VRule} />
       <div className={styles.SnapshotSelector}>
         <span className={styles.Number}>
-          {currentCommitNumber} / {numCommits}
+          {numCommits > 0 ? `${currentCommitNumber} / ${numCommits}` : '-'}
         </span>
         <Button
           className={styles.Button}
-          disabled={numCommits === 0 || commitIndex <= 0}
+          disabled={commitIndex === null || commitIndex <= 0}
           onClick={viewPrevCommit}
         >
           <ButtonIcon type="previous" />
         </Button>
         <div className={styles.Commits}>
-          {numCommits > 0 && (
-            <SnapshotCommitList
-              commitDurations={filteredCommitDurations}
-              commitTimes={filteredCommitTimes}
-              selectedCommitIndex={commitIndex}
-              setCommitIndex={setCommitIndex}
-              viewNextCommit={viewNextCommit}
-              viewPrevCommit={viewPrevCommit}
-            />
-          )}
+          {numCommits > 0 && <SnapshotCommitList />}
           {numCommits === 0 && (
             <div className={styles.NoCommits}>No commits</div>
           )}
         </div>
         <Button
           className={styles.Button}
-          disabled={commitIndex >= numCommits - 1}
+          disabled={commitIndex === null || commitIndex >= numCommits - 1}
           onClick={viewNextCommit}
         >
           <ButtonIcon type="next" />
@@ -109,8 +75,3 @@ function SnapshotSelector(_: Props) {
     </Fragment>
   );
 }
-
-function SnapshotSelectorFallback() {
-  // TODO (profiling) Better loading UI
-  return <div className={styles.SnapshotSelector}>Loading...</div>;
-}

src/devtools/views/TabBar.css

@@ -66,11 +66,14 @@
   .TabLabelSmall {
     display: none;
   }
+
+  .IconSizeSmall {
+    margin-right: 0;
+  }
 }
 
 @media screen and (max-width: 600px) {
-  .IconSizeLarge,
-  .IconSizeSmall {
+  .IconSizeLarge {
     margin-right: 0;
   }
 

src/devtools/views/hooks.js

@@ -2,9 +2,15 @@
 
 import { useCallback, useEffect, useLayoutEffect, useState } from 'react';
 
+type LocalStorageKey =
+  | 'displayDensity'
+  | 'isCommitFilterEnabled'
+  | 'minCommitDuration'
+  | 'theme';
+
 // Forked from https://usehooks.com/useLocalStorage/
 export function useLocalStorage<T>(
-  key: string,
+  key: LocalStorageKey,
   initialValue: T
 ): [T, (value: T | (() => T)) => void] {
   const getValueFromLocalStorage = useCallback(() => {
@@ -85,24 +91,27 @@ export function useModalDismissSignal(
 }
 
 // Copied from https://github.com/facebook/react/pull/15022
-export function useSubscription<Value, Source>({
-  source,
+export function useSubscription<Value>({
   getCurrentValue,
   subscribe,
 }: {|
-  source: Source,
-  getCurrentValue: (source: Source) => Value,
-  subscribe: (source: Source, callback: Function) => () => void,
+  getCurrentValue: () => Value,
+  subscribe: (callback: Function) => () => void,
 |}): Value {
   const [state, setState] = useState({
-    source,
-    value: getCurrentValue(source),
+    getCurrentValue,
+    subscribe,
+    value: getCurrentValue(),
   });
 
-  if (state.source !== source) {
+  if (
+    state.getCurrentValue !== getCurrentValue ||
+    state.subscribe !== subscribe
+  ) {
     setState({
-      source,
-      value: getCurrentValue(source),
+      getCurrentValue,
+      subscribe,
+      value: getCurrentValue(),
     });
   }
 
@@ -115,11 +124,14 @@ export function useSubscription<Value, Source>({
       }
 
       setState(prevState => {
-        if (prevState.source !== source) {
+        if (
+          prevState.getCurrentValue !== getCurrentValue ||
+          prevState.subscribe !== subscribe
+        ) {
           return prevState;
         }
 
-        const value = getCurrentValue(source);
+        const value = getCurrentValue();
         if (prevState.value === value) {
           return prevState;
         }
@@ -127,7 +139,7 @@ export function useSubscription<Value, Source>({
         return { ...prevState, value };
       });
     };
-    const unsubscribe = subscribe(source, checkForUpdates);
+    const unsubscribe = subscribe(checkForUpdates);
 
     checkForUpdates();
 
@@ -135,7 +147,7 @@ export function useSubscription<Value, Source>({
       didUnsubscribe = true;
       unsubscribe();
     };
-  }, [getCurrentValue, source, subscribe]);
+  }, [getCurrentValue, subscribe]);
 
   return state.value;
 }