From 957a336349494f4586026a68be2fe11d7e5348e2 Mon Sep 17 00:00:00 2001
From: Luke De Feo <lukedefeo@meta.com>
Date: Fri, 21 Jul 2023 07:17:31 -0700
Subject: [PATCH] UID refactor 4/ Expose readonly UIState

Summary:
Currently state writes can either go through a named handler that is easy to find and debug or they can directly modify the state.

By exposing readonly atoms only we ensure that all state writes go through a UIACtions. This adds consistency and ease of future debugging.

E.g We could add a utility to wrap all ui actions with logging statements

Reviewed By: antonk52

Differential Revision: D47547531

fbshipit-source-id: f88651169d8e7c5f7e31068d64f9aa5b6b573647
---
 .../public/ui-debugger/DesktopTypes.tsx       | 18 +++-
 .../ui-debugger/components/Controls.tsx       | 27 +-----
 .../ui-debugger/components/PerfStats.tsx      |  4 +-
 .../public/ui-debugger/components/Tree.tsx    |  8 +-
 .../components/Visualization2D.tsx            |  9 +-
 .../components/util/UIDebuggerMenuItem.tsx    |  2 +-
 desktop/plugins/public/ui-debugger/index.tsx  | 87 ++++++++++++-------
 7 files changed, 86 insertions(+), 69 deletions(-)

diff --git a/desktop/plugins/public/ui-debugger/DesktopTypes.tsx b/desktop/plugins/public/ui-debugger/DesktopTypes.tsx
index e800b8db7..6c07b650e 100644
--- a/desktop/plugins/public/ui-debugger/DesktopTypes.tsx
+++ b/desktop/plugins/public/ui-debugger/DesktopTypes.tsx
@@ -7,7 +7,7 @@
  * @format
  */
 
-import {Atom} from 'flipper-plugin';
+import {Atom, _ReadOnlyAtom} from 'flipper-plugin';
 import {
   Id,
   FrameworkEventType,
@@ -35,6 +35,14 @@ export type UIState = {
   filterMainThreadMonitoring: Atom<boolean>;
 };
 
+//enumerates the keys of input type and casts each to ReadOnlyAtom, this is so we only expose read only atoms to the UI
+//and all writes come through UIActions
+type TransformToReadOnly<T> = {
+  [P in keyof T]: T[P] extends Atom<infer U> ? _ReadOnlyAtom<U> : T[P];
+};
+
+export type ReadOnlyUIState = TransformToReadOnly<UIState>;
+
 export type StreamFlowState = {paused: boolean};
 
 export type NestedNode = {
@@ -62,7 +70,7 @@ export type OnSelectNode = (
 ) => void;
 
 export type UIActions = {
-  onHoverNode: (node?: Id) => void;
+  onHoverNode: (...node: Id[]) => void;
   onFocusNode: (focused?: Id) => void;
   onContextMenuOpen: (open: boolean) => void;
   onSelectNode: OnSelectNode;
@@ -71,6 +79,12 @@ export type UIActions = {
   setVisualiserWidth: (width: number) => void;
   onSetFilterMainThreadMonitoring: (toggled: boolean) => void;
   onSetViewMode: (viewMode: ViewMode) => void;
+  onSetFrameworkEventMonitored: (
+    eventType: FrameworkEventType,
+    monitored: boolean,
+  ) => void;
+  onPlayPauseToggled: () => void;
+  onSearchTermUpdated: (searchTerm: string) => void;
 };
 
 export type SelectionSource = 'visualiser' | 'tree' | 'keyboard';
diff --git a/desktop/plugins/public/ui-debugger/components/Controls.tsx b/desktop/plugins/public/ui-debugger/components/Controls.tsx
index 3a37664ca..3085a4553 100644
--- a/desktop/plugins/public/ui-debugger/components/Controls.tsx
+++ b/desktop/plugins/public/ui-debugger/components/Controls.tsx
@@ -27,12 +27,6 @@ import {
 } from '@ant-design/icons';
 import {usePlugin, useValue, Layout} from 'flipper-plugin';
 import {FrameworkEventType} from '../ClientTypes';
-import {tracker} from '../utils/tracker';
-import {debounce} from 'lodash';
-
-const searchTermUpdated = debounce((searchTerm: string) => {
-  tracker.track('search-term-updated', {searchTerm});
-}, 250);
 
 export const Controls: React.FC = () => {
   const instance = usePlugin(plugin);
@@ -49,23 +43,12 @@ export const Controls: React.FC = () => {
   const [showFrameworkEventsModal, setShowFrameworkEventsModal] =
     useState(false);
 
-  const onSetEventMonitored: (
-    eventType: FrameworkEventType,
-    monitored: boolean,
-  ) => void = (eventType: FrameworkEventType, monitored: boolean) => {
-    tracker.track('framework-event-monitored', {eventType, monitored});
-    instance.uiState.frameworkEventMonitoring.update((draft) =>
-      draft.set(eventType, monitored),
-    );
-  };
-
   return (
     <Layout.Horizontal pad="small" gap="small">
       <Input
         value={searchTerm}
         onChange={(e) => {
-          instance.uiState.searchTerm.set(e.target.value);
-          searchTermUpdated(e.target.value);
+          instance.uiActions.onSearchTermUpdated(e.target.value);
         }}
         prefix={<SearchOutlined />}
         placeholder="Search"
@@ -73,11 +56,7 @@ export const Controls: React.FC = () => {
       <Button
         type="default"
         shape="circle"
-        onClick={() => {
-          const isPaused = !instance.uiState.isPaused.get();
-          tracker.track('play-pause-toggled', {paused: isPaused});
-          instance.setPlayPause(isPaused);
-        }}
+        onClick={instance.uiActions.onPlayPauseToggled}
         icon={
           <Tooltip
             title={isPaused ? 'Resume live updates' : 'Pause incoming updates'}>
@@ -102,7 +81,7 @@ export const Controls: React.FC = () => {
             instance.uiActions.onSetFilterMainThreadMonitoring
           }
           frameworkEventTypes={[...frameworkEventMonitoring.entries()]}
-          onSetEventMonitored={onSetEventMonitored}
+          onSetEventMonitored={instance.uiActions.onSetFrameworkEventMonitored}
           visible={showFrameworkEventsModal}
           onCancel={() => setShowFrameworkEventsModal(false)}
         />
diff --git a/desktop/plugins/public/ui-debugger/components/PerfStats.tsx b/desktop/plugins/public/ui-debugger/components/PerfStats.tsx
index 5224772be..270cd4d5e 100644
--- a/desktop/plugins/public/ui-debugger/components/PerfStats.tsx
+++ b/desktop/plugins/public/ui-debugger/components/PerfStats.tsx
@@ -14,7 +14,7 @@ import {
   ClientNode,
   FrameworkEvent,
 } from '../ClientTypes';
-import {UIState} from '../DesktopTypes';
+import {ReadOnlyUIState} from '../DesktopTypes';
 import React, {useMemo} from 'react';
 import {
   DataInspector,
@@ -26,7 +26,7 @@ import {
 } from 'flipper-plugin';
 
 export function PerfStats(props: {
-  uiState: UIState;
+  uiState: ReadOnlyUIState;
   nodes: Map<Id, ClientNode>;
   rootId?: Id;
   events: DataSource<DynamicPerformanceStatsEvent, number>;
diff --git a/desktop/plugins/public/ui-debugger/components/Tree.tsx b/desktop/plugins/public/ui-debugger/components/Tree.tsx
index 80b97e264..1154036ac 100644
--- a/desktop/plugins/public/ui-debugger/components/Tree.tsx
+++ b/desktop/plugins/public/ui-debugger/components/Tree.tsx
@@ -253,7 +253,7 @@ export function Tree2({
               }}
               onMouseLeave={() => {
                 if (isContextMenuOpen === false) {
-                  instance.uiState.hoveredNodes.set([]);
+                  instance.uiActions.onHoverNode();
                 }
               }}>
               {rowVirtualizer.getVirtualItems().map((virtualRow) => (
@@ -756,7 +756,7 @@ function useKeyboardShortcuts(
   selectedNode: Id | undefined,
   hoveredNodeId: Id | undefined,
   onSelectNode: OnSelectNode,
-  onHoverNode: (id?: Id) => void,
+  onHoverNode: (...id: Id[]) => void,
   onExpandNode: (id: Id) => void,
   onCollapseNode: (id: Id) => void,
   isUsingKBToScrollUntill: React.MutableRefObject<number>,
@@ -861,7 +861,7 @@ function moveSelectedNodeUpOrDown(
   hoveredNode: Id | undefined,
   selectedNode: Id | undefined,
   onSelectNode: OnSelectNode,
-  onHoverNode: (id?: Id) => void,
+  onHoverNode: (...id: Id[]) => void,
   isUsingKBToScrollUntill: React.MutableRefObject<MillisSinceEpoch>,
 ) {
   const nodeToUse = selectedNode != null ? selectedNode : hoveredNode;
@@ -886,7 +886,7 @@ function moveSelectedNodeViaKeyBoard(
   treeNodes: TreeNode[],
   rowVirtualizer: Virtualizer<HTMLDivElement, Element>,
   onSelectNode: OnSelectNode,
-  onHoverNode: (id?: Id) => void,
+  onHoverNode: (...id: Id[]) => void,
   isUsingKBToScrollUntil: React.MutableRefObject<number>,
 ) {
   if (newIdx >= 0 && newIdx < treeNodes.length) {
diff --git a/desktop/plugins/public/ui-debugger/components/Visualization2D.tsx b/desktop/plugins/public/ui-debugger/components/Visualization2D.tsx
index de77162fe..eba541e99 100644
--- a/desktop/plugins/public/ui-debugger/components/Visualization2D.tsx
+++ b/desktop/plugins/public/ui-debugger/components/Visualization2D.tsx
@@ -80,7 +80,7 @@ export const Visualization2D: React.FC<
         hitNodes.length > 0 &&
         !isEqual(hitNodes, instance.uiState.hoveredNodes.get())
       ) {
-        instance.uiState.hoveredNodes.set(hitNodes);
+        instance.uiActions.onHoverNode(...hitNodes);
       }
     }, MouseThrottle);
     window.addEventListener('mousemove', mouseListener);
@@ -95,6 +95,7 @@ export const Visualization2D: React.FC<
     instance.uiState.isContextMenuOpen,
     width,
     snapshotNode,
+    instance.uiActions,
   ]);
 
   if (!focusState || !snapshotNode) {
@@ -110,7 +111,7 @@ export const Visualization2D: React.FC<
           e.stopPropagation();
           //the context menu triggers this callback but we dont want to remove hover effect
           if (!instance.uiState.isContextMenuOpen.get()) {
-            instance.uiState.hoveredNodes.set([]);
+            instance.uiActions.onHoverNode();
           }
 
           mouseInVisualiserRef.current = false;
@@ -338,7 +339,7 @@ const ContextMenu: React.FC<{nodes: Map<Id, ClientNode>}> = ({children}) => {
   return (
     <Dropdown
       onVisibleChange={(open) => {
-        instance.uiState.isContextMenuOpen.set(open);
+        instance.uiActions.onContextMenuOpen(open);
       }}
       trigger={['contextMenu']}
       overlay={() => {
@@ -358,7 +359,7 @@ const ContextMenu: React.FC<{nodes: Map<Id, ClientNode>}> = ({children}) => {
                 key="remove-focus"
                 text="Remove focus"
                 onClick={() => {
-                  instance.uiState.focusedNode.set(undefined);
+                  instance.uiActions.onFocusNode(undefined);
                 }}
               />
             )}
diff --git a/desktop/plugins/public/ui-debugger/components/util/UIDebuggerMenuItem.tsx b/desktop/plugins/public/ui-debugger/components/util/UIDebuggerMenuItem.tsx
index f2d397bbf..9292b1055 100644
--- a/desktop/plugins/public/ui-debugger/components/util/UIDebuggerMenuItem.tsx
+++ b/desktop/plugins/public/ui-debugger/components/util/UIDebuggerMenuItem.tsx
@@ -39,7 +39,7 @@ export const UIDebuggerMenuItem: React.FC<{
       disabled={onClick == null}
       onClick={() => {
         onClick?.();
-        instance.uiState.isContextMenuOpen.set(false);
+        instance.uiActions.onContextMenuOpen(false);
       }}>
       <Layout.Horizontal center gap="small">
         {icon}
diff --git a/desktop/plugins/public/ui-debugger/index.tsx b/desktop/plugins/public/ui-debugger/index.tsx
index 89b8d8b3e..f5ce12fcd 100644
--- a/desktop/plugins/public/ui-debugger/index.tsx
+++ b/desktop/plugins/public/ui-debugger/index.tsx
@@ -12,7 +12,6 @@ import {
   createDataSource,
   createState,
   PluginClient,
-  produce,
 } from 'flipper-plugin';
 import {
   Events,
@@ -34,11 +33,13 @@ import {
   StreamState,
   UIActions,
   ViewMode,
+  ReadOnlyUIState,
 } from './DesktopTypes';
 import {Draft} from 'immer';
 import {tracker} from './utils/tracker';
 import {getStreamInterceptor} from './fb-stubs/StreamInterceptor';
 import {prefetchSourceFileLocation} from './components/fb-stubs/IDEContextMenu';
+import {debounce} from 'lodash';
 
 type LiveClientState = {
   snapshotInfo: SnapshotInfo | null;
@@ -234,25 +235,9 @@ export function plugin(client: PluginClient<Events>) {
     expandedNodes: createState<Set<Id>>(new Set()),
   };
 
-  const setPlayPause = (isPaused: boolean) => {
-    uiState.isPaused.set(isPaused);
-    if (!isPaused) {
-      //When going back to play mode then set the atoms to the live state to rerender the latest
-      //Also need to fixed expanded state for any change in active child state
-      uiState.expandedNodes.update((draft) => {
-        liveClientData.nodes.forEach((node) => {
-          collapseinActiveChildren(node, draft);
-        });
-      });
-      nodesAtom.set(liveClientData.nodes);
-      snapshot.set(liveClientData.snapshotInfo);
-      checkFocusedNodeStillActive(uiState, nodesAtom.get());
-    }
-  };
-
   //this is the client data is what drives all of desktop UI
   //it is always up-to-date with the client regardless of whether we are paused or not
-  let liveClientData: LiveClientState = {
+  const mutableLiveClientData: LiveClientState = {
     snapshotInfo: null,
     nodes: new Map(),
   };
@@ -330,13 +315,10 @@ export function plugin(client: PluginClient<Events>) {
     nodes: Map<Id, ClientNode>,
     snapshotInfo: SnapshotInfo | undefined,
   ) {
-    liveClientData = produce(liveClientData, (draft) => {
-      if (snapshotInfo) {
-        draft.snapshotInfo = snapshotInfo;
-      }
-
-      draft.nodes = nodes;
-    });
+    if (snapshotInfo) {
+      mutableLiveClientData.snapshotInfo = snapshotInfo;
+    }
+    mutableLiveClientData.nodes = nodes;
 
     uiState.expandedNodes.update((draft) => {
       for (const node of nodes.values()) {
@@ -354,8 +336,8 @@ export function plugin(client: PluginClient<Events>) {
     });
 
     if (!uiState.isPaused.get()) {
-      nodesAtom.set(liveClientData.nodes);
-      snapshot.set(liveClientData.snapshotInfo);
+      nodesAtom.set(mutableLiveClientData.nodes);
+      snapshot.set(mutableLiveClientData.snapshotInfo);
 
       checkFocusedNodeStillActive(uiState, nodesAtom.get());
     }
@@ -378,14 +360,13 @@ export function plugin(client: PluginClient<Events>) {
 
   return {
     rootId,
-    uiState,
-    uiActions: uiActions(uiState, nodesAtom),
+    uiState: uiState as ReadOnlyUIState,
+    uiActions: uiActions(uiState, nodesAtom, snapshot, mutableLiveClientData),
     nodes: nodesAtom,
     frameworkEvents,
     snapshot,
     metadata,
     perfEvents,
-    setPlayPause,
     os,
   };
 }
@@ -393,6 +374,8 @@ export function plugin(client: PluginClient<Events>) {
 function uiActions(
   uiState: UIState,
   nodes: Atom<Map<Id, ClientNode>>,
+  snapshot: Atom<SnapshotInfo | null>,
+  liveClientData: LiveClientState,
 ): UIActions {
   const onExpandNode = (node: Id) => {
     uiState.expandedNodes.update((draft) => {
@@ -434,9 +417,9 @@ function uiActions(
     });
   };
 
-  const onHoverNode = (node?: Id) => {
+  const onHoverNode = (...node: Id[]) => {
     if (node != null) {
-      uiState.hoveredNodes.set([node]);
+      uiState.hoveredNodes.set(node);
     } else {
       uiState.hoveredNodes.set([]);
     }
@@ -473,6 +456,43 @@ function uiActions(
     uiState.viewMode.set(viewMode);
   };
 
+  const onSetFrameworkEventMonitored = (
+    eventType: FrameworkEventType,
+    monitored: boolean,
+  ) => {
+    tracker.track('framework-event-monitored', {eventType, monitored});
+    uiState.frameworkEventMonitoring.update((draft) =>
+      draft.set(eventType, monitored),
+    );
+  };
+
+  const onPlayPauseToggled = () => {
+    const isPaused = !uiState.isPaused.get();
+    tracker.track('play-pause-toggled', {paused: isPaused});
+    uiState.isPaused.set(isPaused);
+    if (!isPaused) {
+      //When going back to play mode then set the atoms to the live state to rerender the latest
+      //Also need to fixed expanded state for any change in active child state
+      uiState.expandedNodes.update((draft) => {
+        liveClientData.nodes.forEach((node) => {
+          collapseinActiveChildren(node, draft);
+        });
+      });
+      nodes.set(liveClientData.nodes);
+      snapshot.set(liveClientData.snapshotInfo);
+      checkFocusedNodeStillActive(uiState, nodes.get());
+    }
+  };
+
+  const searchTermUpdatedDebounced = debounce((searchTerm: string) => {
+    tracker.track('search-term-updated', {searchTerm});
+  }, 250);
+
+  const onSearchTermUpdated = (searchTerm: string) => {
+    uiState.searchTerm.set(searchTerm);
+    searchTermUpdatedDebounced(searchTerm);
+  };
+
   return {
     onExpandNode,
     onCollapseNode,
@@ -483,6 +503,9 @@ function uiActions(
     setVisualiserWidth,
     onSetFilterMainThreadMonitoring,
     onSetViewMode,
+    onSetFrameworkEventMonitored,
+    onPlayPauseToggled,
+    onSearchTermUpdated,
   };
 }