From 7a94ad4e8a071ebaf8adb329cb4939be07b8b269 Mon Sep 17 00:00:00 2001
From: Brian Vaughn <bvaughn@fb.com>
Date: Mon, 17 Jun 2019 11:30:53 -0700
Subject: [PATCH] Added props/state/context inspection to KeyValue

---
 .../dev/app/InspectableElements/Hydration.js  |   2 +
 src/__tests__/inspectedElementContext-test.js |  10 +-
 src/__tests__/legacy/inspectElement-test.js   |   4 +-
 src/backend/renderer.js                       |   2 +-
 .../views/Components/ExpandCollapseToggle.js  |   8 +-
 .../views/Components/InspectedElementTree.js  |   6 +-
 src/devtools/views/Components/KeyValue.js     |  47 ++++++--
 .../views/Components/SelectedElement.js       |  33 +++++-
 src/devtools/views/utils.js                   |   2 +-
 src/hydration.js                              | 101 ++++++++++--------
 10 files changed, 150 insertions(+), 65 deletions(-)

diff --git a/shells/dev/app/InspectableElements/Hydration.js b/shells/dev/app/InspectableElements/Hydration.js
index 04cdc50164..a51db3e2eb 100644
--- a/shells/dev/app/InspectableElements/Hydration.js
+++ b/shells/dev/app/InspectableElements/Hydration.js
@@ -13,6 +13,7 @@ const arrayOfArrays = [
   [['a', 'b', 'c'], ['d', 'e', 'f'], ['h', 'i', 'j']],
   [['k', 'l', 'm'], ['n', 'o', 'p'], ['q', 'r', 's']],
   [['t', 'u', 'v'], ['w', 'x', 'y'], ['z']],
+  [],
 ];
 
 const objectOfObjects = {
@@ -31,6 +32,7 @@ const objectOfObjects = {
     i: 8,
     j: 9,
   },
+  qux: {},
 };
 
 export default function Hydration() {
diff --git a/src/__tests__/inspectedElementContext-test.js b/src/__tests__/inspectedElementContext-test.js
index 64ddb50fb5..c8ffcaeb48 100644
--- a/src/__tests__/inspectedElementContext-test.js
+++ b/src/__tests__/inspectedElementContext-test.js
@@ -315,18 +315,24 @@ describe('InspectedElementContext', () => {
       typed_array,
       date,
     } = (inspectedElement: any).props;
+    expect(html_element[meta.inspectable]).toBe(false);
     expect(html_element[meta.name]).toBe('DIV');
     expect(html_element[meta.type]).toBe('html_element');
+    expect(fn[meta.inspectable]).toBe(false);
     expect(fn[meta.name]).toBe('exmapleFunction');
     expect(fn[meta.type]).toBe('function');
+    expect(symbol[meta.inspectable]).toBe(false);
     expect(symbol[meta.name]).toBe('Symbol(symbol)');
     expect(symbol[meta.type]).toBe('symbol');
+    expect(react_element[meta.inspectable]).toBe(false);
     expect(react_element[meta.name]).toBe('span');
     expect(react_element[meta.type]).toBe('react_element');
-    expect(array_buffer[meta.meta].length).toBe(3);
+    expect(array_buffer[meta.size]).toBe(3);
+    expect(array_buffer[meta.inspectable]).toBe(false);
     expect(array_buffer[meta.name]).toBe('ArrayBuffer');
     expect(array_buffer[meta.type]).toBe('array_buffer');
-    expect(typed_array[meta.meta].length).toBe(3);
+    expect(typed_array[meta.size]).toBe(3);
+    expect(typed_array[meta.inspectable]).toBe(false);
     expect(typed_array[meta.name]).toBe('Uint8Array');
     expect(typed_array[meta.type]).toBe('typed_array');
     expect(date[meta.type]).toBe('date');
diff --git a/src/__tests__/legacy/inspectElement-test.js b/src/__tests__/legacy/inspectElement-test.js
index 7e8eb91bc9..49b15c6a44 100644
--- a/src/__tests__/legacy/inspectElement-test.js
+++ b/src/__tests__/legacy/inspectElement-test.js
@@ -132,10 +132,10 @@ describe('InspectedElementContext', () => {
     expect(symbol[meta.type]).toBe('symbol');
     expect(react_element[meta.name]).toBe('span');
     expect(react_element[meta.type]).toBe('react_element');
-    expect(array_buffer[meta.meta].length).toBe(3);
+    expect(array_buffer[meta.size]).toBe(3);
     expect(array_buffer[meta.name]).toBe('ArrayBuffer');
     expect(array_buffer[meta.type]).toBe('array_buffer');
-    expect(typed_array[meta.meta].length).toBe(3);
+    expect(typed_array[meta.size]).toBe(3);
     expect(typed_array[meta.name]).toBe('Uint8Array');
     expect(typed_array[meta.type]).toBe('typed_array');
     expect(date[meta.type]).toBe('date');
diff --git a/src/backend/renderer.js b/src/backend/renderer.js
index 75c76d7ae2..4cd29d69f6 100644
--- a/src/backend/renderer.js
+++ b/src/backend/renderer.js
@@ -2168,7 +2168,7 @@ export function attach(
               ((mostRecentlyInspectedElement: any): InspectedElement),
               path
             ),
-            mergeInspectedPaths,
+            currentlyInspectedPaths,
             path
           ),
         };
diff --git a/src/devtools/views/Components/ExpandCollapseToggle.js b/src/devtools/views/Components/ExpandCollapseToggle.js
index b5425c8129..61fdca7f33 100644
--- a/src/devtools/views/Components/ExpandCollapseToggle.js
+++ b/src/devtools/views/Components/ExpandCollapseToggle.js
@@ -1,6 +1,6 @@
 // @flow
 
-import React, { useCallback } from 'react';
+import React from 'react';
 import Button from '../Button';
 import ButtonIcon from '../ButtonIcon';
 
@@ -15,14 +15,10 @@ export default function ExpandCollapseToggle({
   isOpen,
   setIsOpen,
 }: ExpandCollapseToggleProps) {
-  const handleClick = useCallback(() => {
-    setIsOpen(prevIsOpen => !prevIsOpen);
-  }, [setIsOpen]);
-
   return (
     <Button
       className={styles.ExpandCollapseToggle}
-      onClick={handleClick}
+      onClick={() => setIsOpen(prevIsOpen => !prevIsOpen)}
       title={`${isOpen ? 'Collapse' : 'Expand'} prop value`}
     >
       <ButtonIcon type={isOpen ? 'expanded' : 'collapsed'} />
diff --git a/src/devtools/views/Components/InspectedElementTree.js b/src/devtools/views/Components/InspectedElementTree.js
index aeab263fce..5475b236c8 100644
--- a/src/devtools/views/Components/InspectedElementTree.js
+++ b/src/devtools/views/Components/InspectedElementTree.js
@@ -8,10 +8,13 @@ import KeyValue from './KeyValue';
 import { serializeDataForCopy } from '../utils';
 import styles from './InspectedElementTree.css';
 
+import type { InspectPath } from './SelectedElement';
+
 type OverrideValueFn = (path: Array<string | number>, value: any) => void;
 
 type Props = {|
   data: Object | null,
+  inspectPath?: InspectPath,
   label: string,
   overrideValueFn?: ?OverrideValueFn,
   showWhenEmpty?: boolean,
@@ -19,6 +22,7 @@ type Props = {|
 
 export default function InspectedElementTree({
   data,
+  inspectPath,
   label,
   overrideValueFn,
   showWhenEmpty = false,
@@ -32,7 +36,6 @@ export default function InspectedElementTree({
   if (isEmpty && !showWhenEmpty) {
     return null;
   } else {
-    // TODO Add click and key handlers for toggling element open/close state.
     return (
       <div className={styles.InspectedElementTree}>
         <div className={styles.HeaderRow}>
@@ -49,6 +52,7 @@ export default function InspectedElementTree({
             <KeyValue
               key={name}
               depth={1}
+              inspectPath={inspectPath}
               name={name}
               overrideValueFn={overrideValueFn}
               path={[name]}
diff --git a/src/devtools/views/Components/KeyValue.js b/src/devtools/views/Components/KeyValue.js
index 119a9a9bd5..2bc78e7c91 100644
--- a/src/devtools/views/Components/KeyValue.js
+++ b/src/devtools/views/Components/KeyValue.js
@@ -1,6 +1,6 @@
 // @flow
 
-import React, { useCallback, useState } from 'react';
+import React, { useEffect, useRef, useState } from 'react';
 import type { Element } from 'react';
 import EditableValue from './EditableValue';
 import ExpandCollapseToggle from './ExpandCollapseToggle';
@@ -8,11 +8,14 @@ import { getMetaValueLabel } from '../utils';
 import { meta } from '../../../hydration';
 import styles from './KeyValue.css';
 
+import type { InspectPath } from './SelectedElement';
+
 type OverrideValueFn = (path: Array<string | number>, value: any) => void;
 
 type KeyValueProps = {|
   depth: number,
   hidden?: boolean,
+  inspectPath?: InspectPath,
   name: string,
   overrideValueFn?: ?OverrideValueFn,
   path?: Array<any>,
@@ -24,6 +27,7 @@ type KeyValueProps = {|
 
 export default function KeyValue({
   depth,
+  inspectPath,
   hidden,
   name,
   overrideValueFn,
@@ -31,11 +35,27 @@ export default function KeyValue({
   value,
 }: KeyValueProps) {
   const [isOpen, setIsOpen] = useState<boolean>(false);
+  const prevIsOpenRef = useRef(isOpen);
 
-  const toggleIsOpen = useCallback(
-    () => setIsOpen(prevIsOpen => !prevIsOpen),
-    []
-  );
+  const isInspectable =
+    value !== null &&
+    typeof value === 'object' &&
+    value[meta.inspectable] &&
+    value[meta.size] !== 0;
+
+  useEffect(() => {
+    if (
+      isInspectable &&
+      isOpen &&
+      !prevIsOpenRef.current &&
+      typeof inspectPath === 'function'
+    ) {
+      inspectPath(path);
+    }
+    prevIsOpenRef.current = isOpen;
+  }, [inspectPath, isInspectable, isOpen, path]);
+
+  const toggleIsOpen = () => setIsOpen(prevIsOpen => !prevIsOpen);
 
   const dataType = typeof value;
   const isSimpleType =
@@ -81,12 +101,19 @@ export default function KeyValue({
       </div>
     );
   } else if (value.hasOwnProperty(meta.type)) {
-    // TODO (hydration) show UI to load its data?
-    // TODO Is this type even necessary? Can we just drop it?
     children = (
       <div key="root" className={styles.Item} hidden={hidden} style={style}>
-        <div className={styles.ExpandCollapseToggleSpacer} />
-        <span className={styles.Name}>{name}</span>
+        {isInspectable ? (
+          <ExpandCollapseToggle isOpen={isOpen} setIsOpen={setIsOpen} />
+        ) : (
+          <div className={styles.ExpandCollapseToggleSpacer} />
+        )}
+        <span
+          className={styles.Name}
+          onClick={isInspectable ? toggleIsOpen : undefined}
+        >
+          {name}
+        </span>
         <span className={styles.Value}>{getMetaValueLabel(value)}</span>
       </div>
     );
@@ -98,6 +125,7 @@ export default function KeyValue({
         <KeyValue
           key={index}
           depth={depth + 1}
+          inspectPath={inspectPath}
           hidden={hidden || !isOpen}
           name={index}
           overrideValueFn={overrideValueFn}
@@ -133,6 +161,7 @@ export default function KeyValue({
         <KeyValue
           key={name}
           depth={depth + 1}
+          inspectPath={inspectPath}
           hidden={hidden || !isOpen}
           name={name}
           overrideValueFn={overrideValueFn}
diff --git a/src/devtools/views/Components/SelectedElement.js b/src/devtools/views/Components/SelectedElement.js
index e4250fbe03..97cd2d20db 100644
--- a/src/devtools/views/Components/SelectedElement.js
+++ b/src/devtools/views/Components/SelectedElement.js
@@ -25,6 +25,7 @@ import {
 
 import styles from './SelectedElement.css';
 
+import type { GetPath } from './InspectedElementContext';
 import type { Element, InspectedElement } from './types';
 import type { ElementType } from 'src/types';
 
@@ -38,7 +39,7 @@ export default function SelectedElement(_: Props) {
   const store = useContext(StoreContext);
   const { dispatch: modalDialogDispatch } = useContext(ModalDialogContext);
 
-  const { read } = useContext(InspectedElementContext);
+  const { getPath, read } = useContext(InspectedElementContext);
 
   const element =
     inspectedElementID !== null
@@ -200,7 +201,11 @@ export default function SelectedElement(_: Props) {
 
       {inspectedElement !== null && (
         <InspectedElementView
+          key={
+            inspectedElementID /* Ensure state resets between seleted Elements */
+          }
           element={element}
+          getPath={getPath}
           inspectedElement={inspectedElement}
         />
       )}
@@ -208,8 +213,11 @@ export default function SelectedElement(_: Props) {
   );
 }
 
+export type InspectPath = (path: Array<string | number>) => void;
+
 type InspectedElementViewProps = {|
   element: Element,
+  getPath: GetPath,
   inspectedElement: InspectedElement,
 |};
 
@@ -217,6 +225,7 @@ const IS_SUSPENDED = 'Suspended';
 
 function InspectedElementView({
   element,
+  getPath,
   inspectedElement,
 }: InspectedElementViewProps) {
   const { id, type } = element;
@@ -236,6 +245,25 @@ function InspectedElementView({
   const bridge = useContext(BridgeContext);
   const store = useContext(StoreContext);
 
+  const inspectContextPath = useCallback(
+    (path: Array<string | number>) => {
+      getPath(id, ['context', ...path]);
+    },
+    [getPath, id]
+  );
+  const inspectPropsPath = useCallback(
+    (path: Array<string | number>) => {
+      getPath(id, ['props', ...path]);
+    },
+    [getPath, id]
+  );
+  const inspectStatePath = useCallback(
+    (path: Array<string | number>) => {
+      getPath(id, ['state', ...path]);
+    },
+    [getPath, id]
+  );
+
   let overrideContextFn = null;
   let overridePropsFn = null;
   let overrideStateFn = null;
@@ -279,6 +307,7 @@ function InspectedElementView({
       <InspectedElementTree
         label="props"
         data={props}
+        inspectPath={inspectPropsPath}
         overrideValueFn={overridePropsFn}
         showWhenEmpty
       />
@@ -294,6 +323,7 @@ function InspectedElementView({
         <InspectedElementTree
           label="state"
           data={state}
+          inspectPath={inspectStatePath}
           overrideValueFn={overrideStateFn}
         />
       )}
@@ -301,6 +331,7 @@ function InspectedElementView({
       <InspectedElementTree
         label="context"
         data={context}
+        inspectPath={inspectContextPath}
         overrideValueFn={overrideContextFn}
       />
       {events !== null && events.length > 0 && <EventsTree events={events} />}
diff --git a/src/devtools/views/utils.js b/src/devtools/views/utils.js
index 96b808ad78..f5bf15371f 100644
--- a/src/devtools/views/utils.js
+++ b/src/devtools/views/utils.js
@@ -91,7 +91,7 @@ export function getMetaValueLabel(data: Object): string | null {
     case 'data_view':
     case 'array':
     case 'typed_array':
-      return `${name}[${data[meta.meta].length}]`;
+      return `${name}[${data[meta.size]}]`;
     default:
       return null;
   }
diff --git a/src/hydration.js b/src/hydration.js
index a586c8ea83..9bde94aca3 100644
--- a/src/hydration.js
+++ b/src/hydration.js
@@ -19,13 +19,22 @@ import {
 import { getDisplayName, getInObject, setInObject } from './utils';
 
 export const meta = {
-  name: Symbol('name'),
-  type: Symbol('type'),
+  inspectable: Symbol('inspectable'),
   inspected: Symbol('inspected'),
-  meta: Symbol('meta'),
-  proto: Symbol('proto'),
+  name: Symbol('name'),
+  readonly: Symbol('readonly'),
+  size: Symbol('size'),
+  type: Symbol('type'),
 };
 
+type Dehydrated = {|
+  inspectable: boolean,
+  name: string | null,
+  readonly?: boolean,
+  size?: number,
+  type: string,
+|};
+
 // This threshold determines the depth at which the bridge "dehydrates" nested data.
 // Dehydration means that we don't serialize the data for e.g. postMessage or stringify,
 // unless the frontend explicitly requests it (e.g. a user clicks to expand a props object).
@@ -82,29 +91,33 @@ function getPropType(data: Object): string | null {
  */
 function createDehydrated(
   type: string,
+  inspectable: boolean,
   data: Object,
   cleaned: Array<Array<string | number>>,
   path: Array<string | number>
-): Object {
-  const meta = {};
-
-  if (type === 'array' || type === 'typed_array') {
-    meta.length = data.length;
-  }
-  if (type === 'iterator' || type === 'typed_array') {
-    meta.readOnly = true;
-  }
-
+): Dehydrated {
   cleaned.push(path);
 
-  return {
+  const dehydrated: Dehydrated = {
+    inspectable,
     type,
-    meta,
     name:
       !data.constructor || data.constructor.name === 'Object'
         ? ''
         : data.constructor.name,
   };
+
+  if (type === 'array' || type === 'typed_array') {
+    dehydrated.size = data.length;
+  } else if (type === 'object') {
+    dehydrated.size = Object.keys(data).length;
+  }
+
+  if (type === 'iterator' || type === 'typed_array') {
+    dehydrated.readonly = true;
+  }
+
+  return dehydrated;
 }
 
 function isInspectedPath(
@@ -145,13 +158,14 @@ export function dehydrate(
   path: Array<string | number>,
   inspectedPaths: Object,
   level?: number = 0
-): string | Object {
+): string | Dehydrated | { [key: string]: string | Dehydrated } {
   const type = getPropType(data);
 
   switch (type) {
     case 'html_element':
       cleaned.push(path);
       return {
+        inspectable: false,
         name: data.tagName,
         type: 'html_element',
       };
@@ -159,6 +173,7 @@ export function dehydrate(
     case 'function':
       cleaned.push(path);
       return {
+        inspectable: false,
         name: data.name,
         type: 'function',
       };
@@ -171,8 +186,9 @@ export function dehydrate(
     case 'symbol':
       cleaned.push(path);
       return {
-        type: 'symbol',
+        inspectable: false,
         name: data.toString(),
+        type: 'symbol',
       };
 
     // React Elements aren't very inspector-friendly,
@@ -180,6 +196,7 @@ export function dehydrate(
     case 'react_element':
       cleaned.push(path);
       return {
+        inspectable: false,
         name: getDisplayNameForReactElement(data),
         type: 'react_element',
       };
@@ -189,18 +206,16 @@ export function dehydrate(
     case 'data_view':
       cleaned.push(path);
       return {
-        type,
+        inspectable: false,
         name: type === 'data_view' ? 'DataView' : 'ArrayBuffer',
-        meta: {
-          length: data.byteLength,
-          uninspectable: true,
-        },
+        size: data.byteLength,
+        type,
       };
 
     case 'array':
       const arrayPathCheck = isInspectedPath(path, inspectedPaths);
       if (level >= LEVEL_THRESHOLD && !arrayPathCheck) {
-        return createDehydrated(type, data, cleaned, path);
+        return createDehydrated(type, true, data, cleaned, path);
       }
       return data.map((item, i) =>
         dehydrate(
@@ -214,24 +229,24 @@ export function dehydrate(
 
     case 'typed_array':
     case 'iterator':
-      return createDehydrated(type, data, cleaned, path);
+      return createDehydrated(type, false, data, cleaned, path);
+
     case 'date':
       cleaned.push(path);
       return {
+        inspectable: false,
         name: data.toString(),
         type: 'date',
-        meta: {
-          uninspectable: true,
-        },
       };
+
     case 'object':
       const objectPathCheck = isInspectedPath(path, inspectedPaths);
       if (level >= LEVEL_THRESHOLD && !objectPathCheck) {
-        return createDehydrated(type, data, cleaned, path);
+        return createDehydrated(type, true, data, cleaned, path);
       } else {
-        const res = {};
+        const object = {};
         for (let name in data) {
-          res[name] = dehydrate(
+          object[name] = dehydrate(
             data[name],
             cleaned,
             path.concat([name]),
@@ -239,7 +254,7 @@ export function dehydrate(
             objectPathCheck ? 1 : level + 1
           );
         }
-        return res;
+        return object;
       }
 
     default:
@@ -252,16 +267,16 @@ export function fillInPath(
   path: Array<string | number>,
   value: any
 ) {
-  const length = path.length;
-  const parent = getInObject(object, path.slice(0, length - 1));
-  if (object != null) {
-    delete parent[meta.name];
-    delete parent[meta.type];
-    delete parent[meta.meta];
-    delete parent[meta.inspected];
-
-    setInObject(object, path, value);
+  const target = getInObject(object, path);
+  if (target != null) {
+    delete target[meta.inspectable];
+    delete target[meta.inspected];
+    delete target[meta.name];
+    delete target[meta.readonly];
+    delete target[meta.size];
+    delete target[meta.type];
   }
+  setInObject(object, path, value);
 }
 
 export function hydrate(
@@ -280,9 +295,11 @@ export function hydrate(
 
     // Replace the string keys with Symbols so they're non-enumerable.
     const replaced: { [key: Symbol]: boolean | string } = {};
+    replaced[meta.inspectable] = !!value.inspectable;
     replaced[meta.inspected] = false;
-    replaced[meta.meta] = value.meta;
     replaced[meta.name] = value.name;
+    replaced[meta.size] = value.size;
+    replaced[meta.readonly] = !!value.readonly;
     replaced[meta.type] = value.type;
 
     parent[last] = replaced;