Refactor inspect/select logic so that $r contains hooks data (#364)

* Refactor inspect/select logic so that var contains hooks data * Legacy renderer resets $r to null when inspecting non class/function element
facebook · Aug 4, 2019 · db8542a · db8542a
1 parent 644c9c4
commit db8542a
Show file tree

Hide file tree

Showing 7 changed files with 105 additions and 110 deletions.
diff --git a/packages/react-devtools-core/src/backend.js b/packages/react-devtools-core/src/backend.js
@@ -112,8 +112,8 @@ export function connectToDevTools(options: ?ConnectOptions) {
       },
     });
     bridge.addListener(
-      'selectElement',
-      ({ id, rendererID }: {| id: number, rendererID: number |}) => {
+      'inspectElement',
+      ({ id, rendererID }: { id: number, rendererID: number }) => {
         const renderer = agent.rendererInterfaces[rendererID];
         if (renderer != null) {
           // Send event for RN to highlight.

diff --git a/src/backend/agent.js b/src/backend/agent.js
@@ -124,7 +124,6 @@ export default class Agent extends EventEmitter<{|
     bridge.addListener('overrideState', this.overrideState);
     bridge.addListener('overrideSuspense', this.overrideSuspense);
     bridge.addListener('reloadAndProfile', this.reloadAndProfile);
-    bridge.addListener('selectElement', this.selectElement);
     bridge.addListener('startProfiling', this.startProfiling);
     bridge.addListener('stopProfiling', this.stopProfiling);
     bridge.addListener(
@@ -219,6 +218,24 @@ export default class Agent extends EventEmitter<{|
       console.warn(`Invalid renderer id "${rendererID}" for element "${id}"`);
     } else {
       this._bridge.send('inspectedElement', renderer.inspectElement(id, path));
+
+      // When user selects an element, stop trying to restore the selection,
+      // and instead remember the current selection for the next reload.
+      if (
+        this._persistedSelectionMatch === null ||
+        this._persistedSelectionMatch.id !== id
+      ) {
+        this._persistedSelection = null;
+        this._persistedSelectionMatch = null;
+        renderer.setTrackedPath(null);
+        this._throttledPersistSelection(rendererID, id);
+      }
+
+      // TODO: If there was a way to change the selected DOM element
+      // in native Elements tab without forcing a switch to it, we'd do it here.
+      // For now, it doesn't seem like there is a way to do that:
+      // https://github.com/bvaughn/react-devtools-experimental/issues/102
+      // (Setting $0 doesn't work, and calling inspect() switches the tab.)
     }
   };
 
@@ -244,33 +261,6 @@ export default class Agent extends EventEmitter<{|
     this._bridge.send('reloadAppForProfiling');
   };
 
-  selectElement = ({ id, rendererID }: ElementAndRendererID) => {
-    const renderer = this._rendererInterfaces[rendererID];
-    if (renderer == null) {
-      console.warn(`Invalid renderer id "${rendererID}" for element "${id}"`);
-    } else {
-      renderer.selectElement(id);
-
-      // When user selects an element, stop trying to restore the selection,
-      // and instead remember the current selection for the next reload.
-      if (
-        this._persistedSelectionMatch === null ||
-        this._persistedSelectionMatch.id !== id
-      ) {
-        this._persistedSelection = null;
-        this._persistedSelectionMatch = null;
-        renderer.setTrackedPath(null);
-        this._throttledPersistSelection(rendererID, id);
-      }
-
-      // TODO: If there was a way to change the selected DOM element
-      // in native Elements tab without forcing a switch to it, we'd do it here.
-      // For now, it doesn't seem like there is a way to do that:
-      // https://github.com/bvaughn/react-devtools-experimental/issues/102
-      // (Setting $0 doesn't work, and calling inspect() switches the tab.)
-    }
-  };
-
   overrideContext = ({ id, path, rendererID, value }: SetInParams) => {
     const renderer = this._rendererInterfaces[rendererID];
     if (renderer == null) {

diff --git a/src/backend/legacy/renderer.js b/src/backend/legacy/renderer.js
@@ -609,6 +609,35 @@ export function attach(
     };
   }
 
+  function updateSelectedElement(id: number): void {
+    const internalInstance = idToInternalInstanceMap.get(id);
+    if (internalInstance == null) {
+      console.warn(`Could not find instance with id "${id}"`);
+      return;
+    }
+
+    switch (getElementType(internalInstance)) {
+      case ElementTypeClass:
+        global.$r = internalInstance._instance;
+        break;
+      case ElementTypeFunction:
+        const element = internalInstance._currentElement;
+        if (element == null) {
+          console.warn(`Could not find element with id "${id}"`);
+          return;
+        }
+
+        global.$r = {
+          props: element.props,
+          type: element.type,
+        };
+        break;
+      default:
+        global.$r = null;
+        break;
+    }
+  }
+
   function inspectElement(
     id: number,
     path?: Array<string | number>
@@ -630,6 +659,11 @@ export function attach(
       mergeInspectedPaths(path);
     }
 
+    // Any time an inspected element has an update,
+    // we should update the selected $r value as wel.
+    // Do this before dehyration (cleanForBridge).
+    updateSelectedElement(id);
+
     inspectedElement.context = cleanForBridge(
       inspectedElement.context,
       createIsPathWhitelisted('context')
@@ -777,34 +811,6 @@ export function attach(
     global.$type = element.type;
   }
 
-  function selectElement(id: number): void {
-    const internalInstance = idToInternalInstanceMap.get(id);
-    if (internalInstance == null) {
-      console.warn(`Could not find instance with id "${id}"`);
-      return;
-    }
-
-    switch (getElementType(internalInstance)) {
-      case ElementTypeClass:
-        global.$r = internalInstance._instance;
-        break;
-      case ElementTypeFunction:
-        const element = internalInstance._currentElement;
-        if (element == null) {
-          console.warn(`Could not find element with id "${id}"`);
-          return;
-        }
-
-        global.$r = {
-          props: element.props,
-          type: element.type,
-        };
-        break;
-      default:
-        break;
-    }
-  }
-
   function setInProps(id: number, path: Array<string | number>, value: any) {
     const internalInstance = idToInternalInstanceMap.get(id);
     if (internalInstance != null) {
@@ -918,7 +924,6 @@ export function attach(
     overrideSuspense,
     prepareViewElementSource,
     renderer,
-    selectElement,
     setInContext,
     setInHook,
     setInProps,

diff --git a/src/backend/renderer.js b/src/backend/renderer.js
@@ -1988,49 +1988,6 @@ export function attach(
   }
   // END copied code
 
-  function selectElement(id: number): void {
-    let fiber = idToFiberMap.get(id);
-    if (fiber == null) {
-      console.warn(`Could not find Fiber with id "${id}"`);
-      return;
-    }
-
-    const { elementType, memoizedProps, stateNode, tag, type } = fiber;
-
-    switch (tag) {
-      case ClassComponent:
-      case IncompleteClassComponent:
-      case IndeterminateComponent:
-        global.$r = stateNode;
-        break;
-      case FunctionComponent:
-        global.$r = {
-          props: memoizedProps,
-          type,
-        };
-        break;
-      case ForwardRef:
-        global.$r = {
-          props: memoizedProps,
-          type: type.render,
-        };
-        break;
-      case MemoComponent:
-      case SimpleMemoComponent:
-        global.$r = {
-          props: memoizedProps,
-          type:
-            elementType != null && elementType.type != null
-              ? elementType.type
-              : type,
-        };
-        break;
-      default:
-        global.$r = null;
-        break;
-    }
-  }
-
   function prepareViewElementSource(id: number): void {
     let fiber = idToFiberMap.get(id);
     if (fiber == null) {
@@ -2341,6 +2298,52 @@ export function attach(
     };
   }
 
+  function updateSelectedElement(inspectedElement: InspectedElement): void {
+    const { hooks, id, props } = inspectedElement;
+
+    let fiber = idToFiberMap.get(id);
+    if (fiber == null) {
+      console.warn(`Could not find Fiber with id "${id}"`);
+      return;
+    }
+
+    const { elementType, stateNode, tag, type } = fiber;
+
+    switch (tag) {
+      case ClassComponent:
+      case IncompleteClassComponent:
+      case IndeterminateComponent:
+        global.$r = stateNode;
+        break;
+      case FunctionComponent:
+        global.$r = {
+          hooks,
+          props,
+          type,
+        };
+        break;
+      case ForwardRef:
+        global.$r = {
+          props,
+          type: type.render,
+        };
+        break;
+      case MemoComponent:
+      case SimpleMemoComponent:
+        global.$r = {
+          props,
+          type:
+            elementType != null && elementType.type != null
+              ? elementType.type
+              : type,
+        };
+        break;
+      default:
+        global.$r = null;
+        break;
+    }
+  }
+
   function inspectElement(
     id: number,
     path?: Array<string | number>
@@ -2401,6 +2404,11 @@ export function attach(
         mergeInspectedPaths(path);
       }
 
+      // Any time an inspected element has an update,
+      // we should update the selected $r value as wel.
+      // Do this before dehyration (cleanForBridge).
+      updateSelectedElement(mostRecentlyInspectedElement);
+
       // Clone before cleaning so that we preserve the full data.
       // This will enable us to send patches without re-inspecting if hydrated paths are requested.
       // (Reducing how often we shallow-render is a better DX for function components that use hooks.)
@@ -2986,7 +2994,6 @@ export function attach(
     prepareViewElementSource,
     overrideSuspense,
     renderer,
-    selectElement,
     setInContext,
     setInHook,
     setInProps,

diff --git a/src/backend/types.js b/src/backend/types.js
@@ -299,7 +299,6 @@ export type RendererInterface = {
   overrideSuspense: (id: number, forceFallback: boolean) => void,
   prepareViewElementSource: (id: number) => void,
   renderer: ReactRenderer | null,
-  selectElement: (id: number) => void,
   setInContext: (id: number, path: Array<string | number>, value: any) => void,
   setInHook: (
     id: number,

diff --git a/src/bridge.js b/src/bridge.js
@@ -99,7 +99,6 @@ type FrontendEvents = {|
   overrideSuspense: [OverrideSuspense],
   profilingData: [ProfilingDataBackend],
   reloadAndProfile: [boolean],
-  selectElement: [ElementAndRendererID],
   selectFiber: [number],
   shutdown: [],
   startInspectingNative: [],

diff --git a/src/devtools/views/Components/InspectedElementContext.js b/src/devtools/views/Components/InspectedElementContext.js
@@ -241,11 +241,6 @@ function InspectedElementContextController({ children }: Props) {
     // We'll poll for an update in the response handler below.
     sendRequest();
 
-    // Update the $r variable.
-    if (rendererID !== null) {
-      bridge.send('selectElement', { id: selectedElementID, rendererID });
-    }
-
     const onInspectedElement = (data: InspectedElementPayload) => {
       // If this is the element we requested, wait a little bit and then ask for another update.
       if (data.id === selectedElementID) {