DevTools: Restore inspect-element bridge optimizations (facebook#20789)

* Restore inspect-element bridge optimizations

When the new Suspense cache was integrated (so that startTransition could be used) I removed a couple of optimizations between the backend and frontend that reduced bridge traffic when e.g. dehydrated paths were inspected for elements that had not rendered since previously inspected. This commit re-adds those optimizations as well as an additional test with a bug fix that I noticed while reading the backend code.

There are two remaining TODO items as of this commit:
- Make inspected element edits and deletes also use transition API
- Don't over-eagerly refresh the cache in our ping-for-updates handler

I will addres both in subsequent commits.

* Poll for update only refreshes cache when there's an update

* Added inline comment

packages/react-devtools-shared/src/__tests__/inspectedElement-test.js

@@ -65,7 +65,9 @@ describe('InspectedElement', () => {
       .TreeContextController;
 
     // Used by inspectElementAtIndex() helper function
-    testRendererInstance = TestRenderer.create(null);
+    testRendererInstance = TestRenderer.create(null, {
+      unstable_isConcurrent: true,
+    });
   });
 
   afterEach(() => {
@@ -259,7 +261,9 @@ describe('InspectedElement', () => {
       // from props like defaultSelectedElementID and it's easier to reset here than
       // to read the TreeDispatcherContext and update the selected ID that way.
       // We're testing the inspected values here, not the context wiring, so that's ok.
-      testRendererInstance = TestRenderer.create(null);
+      testRendererInstance = TestRenderer.create(null, {
+        unstable_isConcurrent: true,
+      });
 
       const inspectedElement = await inspectElementAtIndex(index);
 
@@ -1197,10 +1201,9 @@ describe('InspectedElement', () => {
             "1": 2,
             "2": 3,
           },
-          "1": Object {
-            "0": "a",
-            "1": "b",
-            "2": "c",
+          "1": Dehydrated {
+            "preview_short": Set(3),
+            "preview_long": Set(3) {"a", "b", "c"},
           },
         },
       }
@@ -1283,12 +1286,9 @@ describe('InspectedElement', () => {
             },
             "value": 1,
           },
-          "c": Object {
-            "d": Dehydrated {
-              "preview_short": {…},
-              "preview_long": {e: {…}, value: 1},
-            },
-            "value": 1,
+          "c": Dehydrated {
+            "preview_short": {…},
+            "preview_long": {d: {…}, value: 1},
           },
         },
       }
@@ -1377,6 +1377,143 @@ describe('InspectedElement', () => {
     done();
   });
 
+  it('should return a full update if a path is inspected for an object that has other pending changes', async done => {
+    const Example = () => null;
+
+    const container = document.createElement('div');
+    await utils.actAsync(() =>
+      ReactDOM.render(
+        <Example
+          nestedObject={{
+            a: {
+              value: 1,
+              b: {
+                value: 1,
+              },
+            },
+            c: {
+              value: 1,
+              d: {
+                value: 1,
+                e: {
+                  value: 1,
+                },
+              },
+            },
+          }}
+        />,
+        container,
+      ),
+    );
+
+    let inspectedElement = null;
+    let inspectElementPath = null;
+
+    // Render once to get a handle on inspectElementPath()
+    inspectedElement = await inspectElementAtIndex(0, () => {
+      inspectElementPath = useInspectElementPath();
+    });
+
+    async function loadPath(path) {
+      TestUtilsAct(() => {
+        TestRendererAct(() => {
+          inspectElementPath(path);
+          jest.runOnlyPendingTimers();
+        });
+      });
+
+      inspectedElement = await inspectElementAtIndex(0);
+    }
+
+    expect(inspectedElement.props).toMatchInlineSnapshot(`
+      Object {
+        "nestedObject": Object {
+          "a": Dehydrated {
+            "preview_short": {…},
+            "preview_long": {b: {…}, value: 1},
+          },
+          "c": Dehydrated {
+            "preview_short": {…},
+            "preview_long": {d: {…}, value: 1},
+          },
+        },
+      }
+    `);
+
+    await loadPath(['props', 'nestedObject', 'a']);
+
+    expect(inspectedElement.props).toMatchInlineSnapshot(`
+      Object {
+        "nestedObject": Object {
+          "a": Object {
+            "b": Object {
+              "value": 1,
+            },
+            "value": 1,
+          },
+          "c": Dehydrated {
+            "preview_short": {…},
+            "preview_long": {d: {…}, value: 1},
+          },
+        },
+      }
+    `);
+
+    TestRendererAct(() => {
+      TestUtilsAct(() => {
+        ReactDOM.render(
+          <Example
+            nestedObject={{
+              a: {
+                value: 2,
+                b: {
+                  value: 2,
+                },
+              },
+              c: {
+                value: 2,
+                d: {
+                  value: 2,
+                  e: {
+                    value: 2,
+                  },
+                },
+              },
+            }}
+          />,
+          container,
+        );
+      });
+    });
+
+    await loadPath(['props', 'nestedObject', 'c']);
+
+    expect(inspectedElement.props).toMatchInlineSnapshot(`
+      Object {
+        "nestedObject": Object {
+          "a": Object {
+            "b": Object {
+              "value": 2,
+            },
+            "value": 2,
+          },
+          "c": Object {
+            "d": Object {
+              "e": Dehydrated {
+                "preview_short": {…},
+                "preview_long": {value: 2},
+              },
+              "value": 2,
+            },
+            "value": 2,
+          },
+        },
+      }
+    `);
+
+    done();
+  });
+
   it('should not tear if hydration is requested after an update', async done => {
     const Example = () => null;
 
@@ -1936,6 +2073,7 @@ describe('InspectedElement', () => {
               <Suspender target={id} />
             </React.Suspense>
           </Contexts>,
+          {unstable_isConcurrent: true},
         );
       }, false);
       await utils.actAsync(() => {

packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js

@@ -26,15 +26,14 @@ describe('InspectedElementContext', () => {
 
   async function read(
     id: number,
-    inspectedPaths?: Object = {},
+    path?: Array<string | number> = null,
   ): Promise<Object> {
     const rendererID = ((store.getRendererIDForElement(id): any): number);
     const promise = backendAPI
       .inspectElement({
         bridge,
-        forceUpdate: true,
         id,
-        inspectedPaths,
+        path,
         rendererID,
       })
       .then(data =>
@@ -686,7 +685,7 @@ describe('InspectedElementContext', () => {
       }
     `);
 
-    inspectedElement = await read(id, {props: {nestedObject: {a: {}}}});
+    inspectedElement = await read(id, ['props', 'nestedObject', 'a']);
     expect(inspectedElement.props).toMatchInlineSnapshot(`
       Object {
         "nestedObject": Object {
@@ -702,9 +701,7 @@ describe('InspectedElementContext', () => {
       }
     `);
 
-    inspectedElement = await read(id, {
-      props: {nestedObject: {a: {b: {c: {}}}}},
-    });
+    inspectedElement = await read(id, ['props', 'nestedObject', 'a', 'b', 'c']);
     expect(inspectedElement.props).toMatchInlineSnapshot(`
       Object {
         "nestedObject": Object {
@@ -724,9 +721,15 @@ describe('InspectedElementContext', () => {
       }
     `);
 
-    inspectedElement = await read(id, {
-      props: {nestedObject: {a: {b: {c: {0: {d: {}}}}}}},
-    });
+    inspectedElement = await read(id, [
+      'props',
+      'nestedObject',
+      'a',
+      'b',
+      'c',
+      0,
+      'd',
+    ]);
     expect(inspectedElement.props).toMatchInlineSnapshot(`
       Object {
         "nestedObject": Object {

packages/react-devtools-shared/src/backend/agent.js

@@ -70,8 +70,7 @@ type CopyElementParams = {|
 
 type InspectElementParams = {|
   id: number,
-  inspectedPaths: Object,
-  forceUpdate: boolean,
+  path: Array<string | number> | null,
   rendererID: number,
   requestID: number,
 |};
@@ -332,8 +331,7 @@ export default class Agent extends EventEmitter<{|
 
   inspectElement = ({
     id,
-    inspectedPaths,
-    forceUpdate,
+    path,
     rendererID,
     requestID,
   }: InspectElementParams) => {
@@ -343,7 +341,7 @@ export default class Agent extends EventEmitter<{|
     } else {
       this._bridge.send(
         'inspectedElement',
-        renderer.inspectElement(requestID, id, inspectedPaths, forceUpdate),
+        renderer.inspectElement(requestID, id, path),
       );
 
       // When user selects an element, stop trying to restore the selection,

packages/react-devtools-shared/src/backend/legacy/renderer.js

@@ -584,12 +584,25 @@ export function attach(
   }
 
   let currentlyInspectedElementID: number | null = null;
+  let currentlyInspectedPaths: Object = {};
+
+  // Track the intersection of currently inspected paths,
+  // so that we can send their data along if the element is re-rendered.
+  function mergeInspectedPaths(path: Array<string | number>) {
+    let current = currentlyInspectedPaths;
+    path.forEach(key => {
+      if (!current[key]) {
+        current[key] = {};
+      }
+      current = current[key];
+    });
+  }
 
-  function createIsPathAllowed(key: string, inspectedPaths: Object) {
+  function createIsPathAllowed(key: string) {
     // This function helps prevent previously-inspected paths from being dehydrated in updates.
     // This is important to avoid a bad user experience where expanded toggles collapse on update.
     return function isPathAllowed(path: Array<string | number>): boolean {
-      let current = inspectedPaths[key];
+      let current = currentlyInspectedPaths[key];
       if (!current) {
         return false;
       }
@@ -680,10 +693,11 @@ export function attach(
   function inspectElement(
     requestID: number,
     id: number,
-    inspectedPaths: Object,
+    path: Array<string | number> | null,
   ): InspectedElementPayload {
     if (currentlyInspectedElementID !== id) {
       currentlyInspectedElementID = id;
+      currentlyInspectedPaths = {};
     }
 
     const inspectedElement = inspectElementRaw(id);
@@ -695,22 +709,26 @@ export function attach(
       };
     }
 
+    if (path !== null) {
+      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,
-      createIsPathAllowed('context', inspectedPaths),
+      createIsPathAllowed('context'),
     );
     inspectedElement.props = cleanForBridge(
       inspectedElement.props,
-      createIsPathAllowed('props', inspectedPaths),
+      createIsPathAllowed('props'),
     );
     inspectedElement.state = cleanForBridge(
       inspectedElement.state,
-      createIsPathAllowed('state', inspectedPaths),
+      createIsPathAllowed('state'),
     );
 
     return {