Merge pull request #2737 from github/robertbrignull/AlertTable-functional

robertbrignull · web-flow · commit d777427c0ad6 · 2023-08-22T16:59:00.000+01:00
Convert AlertTable to a function component
diff --git a/extensions/ql-vscode/src/view/results/AlertTable.tsx b/extensions/ql-vscode/src/view/results/AlertTable.tsx
@@ -20,37 +20,36 @@ import { AlertTableHeader } from "./AlertTableHeader";
 import { AlertTableNoResults } from "./AlertTableNoResults";
 import { AlertTableTruncatedMessage } from "./AlertTableTruncatedMessage";
 import { AlertTableResultRow } from "./AlertTableResultRow";
+import { useCallback, useEffect, useRef, useState } from "react";
 
 type AlertTableProps = ResultTableProps & {
   resultSet: InterpretedResultSet<SarifInterpretationData>;
 };
-interface AlertTableState {
-  expanded: Set<string>;
-  selectedItem: undefined | Keys.ResultKey;
-}
 
-export class AlertTable extends React.Component<
-  AlertTableProps,
-  AlertTableState
-> {
-  private scroller = new ScrollIntoViewHelper();
+export function AlertTable(props: AlertTableProps) {
+  const { databaseUri, resultSet } = props;
 
-  constructor(props: AlertTableProps) {
-    super(props);
-    this.state = { expanded: new Set<string>(), selectedItem: undefined };
-    this.handleNavigationEvent = this.handleNavigationEvent.bind(this);
+  const scroller = useRef<ScrollIntoViewHelper | undefined>(undefined);
+  if (scroller.current === undefined) {
+    scroller.current = new ScrollIntoViewHelper();
   }
+  useEffect(() => scroller.current?.update());
+
+  const [expanded, setExpanded] = useState<Set<string>>(new Set<string>());
+  const [selectedItem, setSelectedItem] = useState<Keys.ResultKey | undefined>(
+    undefined,
+  );
 
   /**
    * Given a list of `keys`, toggle the first, and if we 'open' the
    * first item, open all the rest as well. This mimics vscode's file
    * explorer tree view behavior.
    */
-  toggle(e: React.MouseEvent, keys: Keys.ResultKey[]) {
+  const toggle = useCallback((e: React.MouseEvent, keys: Keys.ResultKey[]) => {
     const keyStrings = keys.map(Keys.keyToString);
-    this.setState((previousState) => {
-      const expanded = new Set(previousState.expanded);
-      if (previousState.expanded.has(keyStrings[0])) {
+    setExpanded((previousExpanded) => {
+      const expanded = new Set(previousExpanded);
+      if (previousExpanded.has(keyStrings[0])) {
         expanded.delete(keyStrings[0]);
       } else {
         for (const str of keyStrings) {
@@ -60,99 +59,94 @@ export class AlertTable extends React.Component<
       if (expanded) {
         sendTelemetry("local-results-alert-table-path-expanded");
       }
-      return { expanded };
+      return expanded;
     });
     e.stopPropagation();
     e.preventDefault();
-  }
-
-  render(): JSX.Element {
-    const { databaseUri, resultSet } = this.props;
-
-    const { numTruncatedResults, sourceLocationPrefix } =
-      resultSet.interpretation;
-
-    const updateSelectionCallback = (
-      resultKey: Keys.PathNode | Keys.Result | undefined,
-    ) => {
-      this.setState((previousState) => ({
-        ...previousState,
-        selectedItem: resultKey,
-      }));
-      sendTelemetry("local-results-alert-table-path-selected");
-    };
+  }, []);
 
-    if (!resultSet.interpretation.data.runs?.[0]?.results?.length) {
-      return <AlertTableNoResults {...this.props} />;
+  const getNewSelection = (
+    key: Keys.ResultKey | undefined,
+    direction: NavigationDirection,
+  ): Keys.ResultKey => {
+    if (key === undefined) {
+      return { resultIndex: 0 };
     }
+    const { resultIndex, pathIndex, pathNodeIndex } = key;
+    switch (direction) {
+      case NavigationDirection.up:
+      case NavigationDirection.down: {
+        const delta = direction === NavigationDirection.up ? -1 : 1;
+        if (key.pathNodeIndex !== undefined) {
+          return {
+            resultIndex,
+            pathIndex: key.pathIndex,
+            pathNodeIndex: key.pathNodeIndex + delta,
+          };
+        } else if (pathIndex !== undefined) {
+          return { resultIndex, pathIndex: pathIndex + delta };
+        } else {
+          return { resultIndex: resultIndex + delta };
+        }
+      }
+      case NavigationDirection.left:
+        if (key.pathNodeIndex !== undefined) {
+          return { resultIndex, pathIndex: key.pathIndex };
+        } else if (pathIndex !== undefined) {
+          return { resultIndex };
+        } else {
+          return key;
+        }
+      case NavigationDirection.right:
+        if (pathIndex === undefined) {
+          return { resultIndex, pathIndex: 0 };
+        } else if (pathNodeIndex === undefined) {
+          return { resultIndex, pathIndex, pathNodeIndex: 0 };
+        } else {
+          return key;
+        }
+    }
+  };
 
-    return (
-      <table className={className}>
-        <AlertTableHeader sortState={resultSet.interpretation.data.sortState} />
-        <tbody>
-          {resultSet.interpretation.data.runs[0].results.map(
-            (result, resultIndex) => (
-              <AlertTableResultRow
-                key={resultIndex}
-                result={result}
-                resultIndex={resultIndex}
-                expanded={this.state.expanded}
-                selectedItem={this.state.selectedItem}
-                databaseUri={databaseUri}
-                sourceLocationPrefix={sourceLocationPrefix}
-                updateSelectionCallback={updateSelectionCallback}
-                toggleExpanded={this.toggle.bind(this)}
-                scroller={this.scroller}
-              />
-            ),
-          )}
-          <AlertTableTruncatedMessage
-            numTruncatedResults={numTruncatedResults}
-          />
-        </tbody>
-      </table>
-    );
-  }
-
-  private handleNavigationEvent(event: NavigateMsg) {
-    this.setState((prevState) => {
-      const key = this.getNewSelection(prevState.selectedItem, event.direction);
-      const data = this.props.resultSet.interpretation.data;
+  const handleNavigationEvent = useCallback(
+    (event: NavigateMsg) => {
+      const key = getNewSelection(selectedItem, event.direction);
+      const data = resultSet.interpretation.data;
 
       // Check if the selected node actually exists (bounds check) and get its location if relevant
       let jumpLocation: Sarif.Location | undefined;
       if (key.pathNodeIndex !== undefined) {
         jumpLocation = Keys.getPathNode(data, key);
         if (jumpLocation === undefined) {
-          return prevState; // Result does not exist
+          return; // Result does not exist
         }
       } else if (key.pathIndex !== undefined) {
         if (Keys.getPath(data, key) === undefined) {
-          return prevState; // Path does not exist
+          return; // Path does not exist
         }
         jumpLocation = undefined; // When selecting a 'path', don't jump anywhere.
       } else {
         jumpLocation = Keys.getResult(data, key)?.locations?.[0];
         if (jumpLocation === undefined) {
-          return prevState; // Path step does not exist.
+          return; // Path step does not exist.
         }
       }
       if (jumpLocation !== undefined) {
         const parsedLocation = parseSarifLocation(
           jumpLocation,
-          this.props.resultSet.interpretation.sourceLocationPrefix,
+          resultSet.interpretation.sourceLocationPrefix,
         );
         if (!isNoLocation(parsedLocation)) {
-          jumpToLocation(parsedLocation, this.props.databaseUri);
+          jumpToLocation(parsedLocation, databaseUri);
         }
       }
 
-      const expanded = new Set(prevState.expanded);
+      const newExpanded = new Set(expanded);
       if (event.direction === NavigationDirection.right) {
         // When stepping right, expand to ensure the selected node is visible
-        expanded.add(Keys.keyToString({ resultIndex: key.resultIndex }));
+        newExpanded.add(Keys.keyToString({ resultIndex: key.resultIndex }));
         if (key.pathIndex !== undefined) {
-          expanded.add(
+          newExpanded.add(
             Keys.keyToString({
               resultIndex: key.resultIndex,
               pathIndex: key.pathIndex,
@@ -161,75 +155,64 @@ export class AlertTable extends React.Component<
         }
       } else if (event.direction === NavigationDirection.left) {
         // When stepping left, collapse immediately
-        expanded.delete(Keys.keyToString(key));
+        newExpanded.delete(Keys.keyToString(key));
       } else {
         // When stepping up or down, collapse the previous node
-        if (prevState.selectedItem !== undefined) {
-          expanded.delete(Keys.keyToString(prevState.selectedItem));
+        if (selectedItem !== undefined) {
+          newExpanded.delete(Keys.keyToString(selectedItem));
         }
       }
-      this.scroller.scrollIntoViewOnNextUpdate();
-      return {
-        ...prevState,
-        expanded,
-        selectedItem: key,
-      };
-    });
-  }
+      scroller.current?.scrollIntoViewOnNextUpdate();
+      setExpanded(newExpanded);
+      setSelectedItem(key);
+    },
+    [databaseUri, expanded, resultSet, selectedItem],
+  );
+
+  useEffect(() => {
+    onNavigation.addListener(handleNavigationEvent);
+    return () => {
+      onNavigation.removeListener(handleNavigationEvent);
+    };
+  }, [handleNavigationEvent]);
 
-  private getNewSelection(
-    key: Keys.ResultKey | undefined,
-    direction: NavigationDirection,
-  ): Keys.ResultKey {
-    if (key === undefined) {
-      return { resultIndex: 0 };
-    }
-    const { resultIndex, pathIndex, pathNodeIndex } = key;
-    switch (direction) {
-      case NavigationDirection.up:
-      case NavigationDirection.down: {
-        const delta = direction === NavigationDirection.up ? -1 : 1;
-        if (key.pathNodeIndex !== undefined) {
-          return {
-            resultIndex,
-            pathIndex: key.pathIndex,
-            pathNodeIndex: key.pathNodeIndex + delta,
-          };
-        } else if (pathIndex !== undefined) {
-          return { resultIndex, pathIndex: pathIndex + delta };
-        } else {
-          return { resultIndex: resultIndex + delta };
-        }
-      }
-      case NavigationDirection.left:
-        if (key.pathNodeIndex !== undefined) {
-          return { resultIndex, pathIndex: key.pathIndex };
-        } else if (pathIndex !== undefined) {
-          return { resultIndex };
-        } else {
-          return key;
-        }
-      case NavigationDirection.right:
-        if (pathIndex === undefined) {
-          return { resultIndex, pathIndex: 0 };
-        } else if (pathNodeIndex === undefined) {
-          return { resultIndex, pathIndex, pathNodeIndex: 0 };
-        } else {
-          return key;
-        }
-    }
-  }
+  const { numTruncatedResults, sourceLocationPrefix } =
+    resultSet.interpretation;
 
-  componentDidUpdate() {
-    this.scroller.update();
-  }
+  const updateSelectionCallback = useCallback(
+    (resultKey: Keys.PathNode | Keys.Result | undefined) => {
+      setSelectedItem(resultKey);
+      sendTelemetry("local-results-alert-table-path-selected");
+    },
+    [],
+  );
 
-  componentDidMount() {
-    this.scroller.update();
-    onNavigation.addListener(this.handleNavigationEvent);
+  if (!resultSet.interpretation.data.runs?.[0]?.results?.length) {
+    return <AlertTableNoResults {...props} />;
   }
 
-  componentWillUnmount() {
-    onNavigation.removeListener(this.handleNavigationEvent);
-  }
+  return (
+    <table className={className}>
+      <AlertTableHeader sortState={resultSet.interpretation.data.sortState} />
+      <tbody>
+        {resultSet.interpretation.data.runs[0].results.map(
+          (result, resultIndex) => (
+            <AlertTableResultRow
+              key={resultIndex}
+              result={result}
+              resultIndex={resultIndex}
+              expanded={expanded}
+              selectedItem={selectedItem}
+              databaseUri={databaseUri}
+              sourceLocationPrefix={sourceLocationPrefix}
+              updateSelectionCallback={updateSelectionCallback}
+              toggleExpanded={toggle}
+              scroller={scroller.current}
+            />
+          ),
+        )}
+        <AlertTableTruncatedMessage numTruncatedResults={numTruncatedResults} />
+      </tbody>
+    </table>
+  );
 }
diff --git a/extensions/ql-vscode/src/view/results/AlertTablePathNodeRow.tsx b/extensions/ql-vscode/src/view/results/AlertTablePathNodeRow.tsx
@@ -17,7 +17,7 @@ interface Props {
   updateSelectionCallback: (
     resultKey: Keys.PathNode | Keys.Result | undefined,
   ) => void;
-  scroller: ScrollIntoViewHelper;
+  scroller?: ScrollIntoViewHelper;
 }
 
 export function AlertTablePathNodeRow(props: Props) {
@@ -51,7 +51,7 @@ export function AlertTablePathNodeRow(props: Props) {
   const zebraIndex = resultIndex + stepIndex;
   return (
     <tr
-      ref={scroller.ref(isSelected)}
+      ref={scroller?.ref(isSelected)}
       className={isSelected ? "vscode-codeql__selected-path-node" : undefined}
     >
       <td className="vscode-codeql__icon-cell">
diff --git a/extensions/ql-vscode/src/view/results/AlertTablePathRow.tsx b/extensions/ql-vscode/src/view/results/AlertTablePathRow.tsx
@@ -19,7 +19,7 @@ interface Props {
     resultKey: Keys.PathNode | Keys.Result | undefined,
   ) => void;
   toggleExpanded: (e: React.MouseEvent, keys: Keys.ResultKey[]) => void;
-  scroller: ScrollIntoViewHelper;
+  scroller?: ScrollIntoViewHelper;
 }
 
 export function AlertTablePathRow(props: Props) {
@@ -50,7 +50,7 @@ export function AlertTablePathRow(props: Props) {
   return (
     <>
       <tr
-        ref={scroller.ref(isPathSpecificallySelected)}
+        ref={scroller?.ref(isPathSpecificallySelected)}
         {...selectableZebraStripe(isPathSpecificallySelected, resultIndex)}
       >
         <td className="vscode-codeql__icon-cell">
diff --git a/extensions/ql-vscode/src/view/results/AlertTableResultRow.tsx b/extensions/ql-vscode/src/view/results/AlertTableResultRow.tsx
@@ -21,7 +21,7 @@ interface Props {
     resultKey: Keys.PathNode | Keys.Result | undefined,
   ) => void;
   toggleExpanded: (e: React.MouseEvent, keys: Keys.ResultKey[]) => void;
-  scroller: ScrollIntoViewHelper;
+  scroller?: ScrollIntoViewHelper;
 }
 
 export function AlertTableResultRow(props: Props) {
@@ -81,7 +81,7 @@ export function AlertTableResultRow(props: Props) {
   return (
     <>
       <tr
-        ref={scroller.ref(resultRowIsSelected)}
+        ref={scroller?.ref(resultRowIsSelected)}
         {...selectableZebraStripe(resultRowIsSelected, resultIndex)}
       >
         {result.codeFlows === undefined ? (
