Move the logic to load properties of a node to util file.

This will allow a consumer (e.g. the debugger) to preload properties of a given root
and use them as it wishes (e.g., for the debugger, use all the direct properties as
root for the ObjectInspector of the Popup preview).

Author: nchevobbe

packages/devtools-reps/src/object-inspector/index.js

@@ -39,21 +39,9 @@ const {
   nodeIsPrototype,
   nodeIsSetter,
   nodeIsWindow,
-  shouldLoadItemEntries,
-  shouldLoadItemIndexedProperties,
-  shouldLoadItemNonIndexedProperties,
-  shouldLoadItemPrototype,
-  shouldLoadItemSymbols,
+  loadItemProperties,
 } = require("./utils/node");
 
-const {
-  enumEntries,
-  enumIndexedProperties,
-  enumNonIndexedProperties,
-  getPrototype,
-  enumSymbols,
-} = require("./utils/client");
-
 import type {
   CachedNodes,
   LoadedProperties,
@@ -244,68 +232,20 @@ class ObjectInspector extends Component {
     if (expand === true) {
       const gripItem = getClosestGripNode(item);
       const value = getValue(gripItem);
-
       const path = item.path;
-      const [start, end] = item.meta
-        ? [item.meta.startIndex, item.meta.endIndex]
-        : [];
-
-      let promises = [];
-      let objectClient;
-      const getObjectClient = () => {
-        if (objectClient) {
-          return objectClient;
-        }
-        return this.props.createObjectClient(value);
-      };
 
-      if (shouldLoadItemIndexedProperties(item, loadedProperties)) {
-        promises.push(enumIndexedProperties(getObjectClient(), start, end));
-      }
-
-      if (shouldLoadItemNonIndexedProperties(item, loadedProperties)) {
-        promises.push(enumNonIndexedProperties(getObjectClient(), start, end));
-      }
-
-      if (shouldLoadItemEntries(item, loadedProperties)) {
-        promises.push(enumEntries(getObjectClient(), start, end));
-      }
-
-      if (shouldLoadItemPrototype(item, loadedProperties)) {
-        promises.push(getPrototype(getObjectClient()));
-      }
-
-      if (shouldLoadItemSymbols(item, loadedProperties)) {
-        promises.push(enumSymbols(getObjectClient(), start, end));
-      }
-
-      if (promises.length > 0) {
-        // Set the loading state with the pending promises.
+      const onItemPropertiesLoaded = loadItemProperties(
+        item, this.props.createObjectClient, loadedProperties);
+      if (onItemPropertiesLoaded !== null) {
         this.setState((prevState, props) => {
           const nextLoading = new Map(prevState.loading);
-          nextLoading.set(path, promises);
+          nextLoading.set(path, onItemPropertiesLoaded);
           return {
             loading: nextLoading
           };
         });
 
-        const responses = await Promise.all(promises);
-
-        // Let's loop through the responses to build a single response object.
-        const response = responses.reduce((accumulator, res) => {
-          Object.entries(res).forEach(([k, v]) => {
-            if (accumulator.hasOwnProperty(k)) {
-              if (Array.isArray(accumulator[k])) {
-                accumulator[k].push(...v);
-              } else if (typeof accumulator[k] === "object") {
-                accumulator[k] = Object.assign({}, accumulator[k], v);
-              }
-            } else {
-              accumulator[k] = v;
-            }
-          });
-          return accumulator;
-        }, {});
+        const properties = await onItemPropertiesLoaded;
 
         this.setState((prevState, props) => {
           const nextLoading = new Map(prevState.loading);
@@ -320,7 +260,7 @@ class ObjectInspector extends Component {
             actors: isRoot
               ? prevState.actors
               : (new Set(prevState.actors)).add(value.actor),
-            loadedProperties: (new Map(prevState.loadedProperties)).set(path, response),
+            loadedProperties: (new Map(prevState.loadedProperties)).set(path, properties),
             loading: nextLoading,
           };
         });

packages/devtools-reps/src/object-inspector/tests/component/state.js

@@ -111,7 +111,7 @@ describe("ObjectInspector - state", () => {
     // Once all the loading promises are resolved, the loading
     // state property should be cleaned up, and actors and loadedProperties
     // should have the expected values.
-    await Promise.all(state.loading.get("root-1"));
+    await state.loading.get("root-1");
 
     state = wrapper.state();
     expect(state.loading.has("root-1")).toBeFalsy();
@@ -131,7 +131,7 @@ describe("ObjectInspector - state", () => {
     // Once all the loading promises are resolved, the loading
     // state property should be cleaned up, and actors and loadedProperties
     // should have the expected values.
-    await Promise.all(state.loading.get("root-1/__proto__"));
+    await state.loading.get("root-1/__proto__");
     expect(formatObjectInspector(wrapper)).toMatchSnapshot();
     state = wrapper.state();
 
@@ -168,7 +168,7 @@ describe("ObjectInspector - state", () => {
     // Once all the loading promises are resolved, the loading
     // state property should be cleaned up, and actors and loadedProperties
     // should have the expected values.
-    await Promise.all(state.loading.get("root-2"));
+    await state.loading.get("root-2");
     expect(formatObjectInspector(wrapper)).toMatchSnapshot();
 
     state = wrapper.state();
@@ -187,7 +187,7 @@ describe("ObjectInspector - state", () => {
     // Once all the loading promises are resolved, the loading
     // state property should be cleaned up, and actors and loadedProperties
     // should have the expected values.
-    await Promise.all(state.loading.get("root-2/__proto__"));
+    await state.loading.get("root-2/__proto__");
     expect(formatObjectInspector(wrapper)).toMatchSnapshot();
     state = wrapper.state();
 

packages/devtools-reps/src/object-inspector/utils/node.js

@@ -29,12 +29,21 @@ const NODE_TYPES = {
   PROTOTYPE: Symbol("__proto__"),
 };
 
+const {
+  enumEntries,
+  enumIndexedProperties,
+  enumNonIndexedProperties,
+  getPrototype,
+  enumSymbols,
+} = require("./client");
+
 import type {
   CachedNodes,
   GripProperties,
   LoadedProperties,
   Node,
   NodeContents,
+  ObjectClient,
   RdpGrip,
 } from "../types";
 
@@ -762,6 +771,69 @@ function getClosestNonBucketNode(item: Node) : Node | null {
   return getClosestNonBucketNode(parent);
 }
 
+function loadItemProperties(
+  item: Node,
+  createObjectClient: (RdpGrip | NodeContents) => ObjectClient,
+  loadedProperties: LoadedProperties
+) : Promise<Object> | null {
+  const [start, end] = item.meta
+    ? [item.meta.startIndex, item.meta.endIndex]
+    : [];
+
+  let objectClient;
+  const getObjectClient = () => {
+    if (objectClient) {
+      return objectClient;
+    }
+
+    const gripItem = getClosestGripNode(item);
+    const value = getValue(gripItem);
+    return createObjectClient(value);
+  };
+
+  let loadingPromises = [];
+  if (shouldLoadItemIndexedProperties(item, loadedProperties)) {
+    loadingPromises.push(enumIndexedProperties(getObjectClient(), start, end));
+  }
+
+  if (shouldLoadItemNonIndexedProperties(item, loadedProperties)) {
+    loadingPromises.push(enumNonIndexedProperties(getObjectClient(), start, end));
+  }
+
+  if (shouldLoadItemEntries(item, loadedProperties)) {
+    loadingPromises.push(enumEntries(getObjectClient(), start, end));
+  }
+
+  if (shouldLoadItemPrototype(item, loadedProperties)) {
+    loadingPromises.push(getPrototype(getObjectClient()));
+  }
+
+  if (shouldLoadItemSymbols(item, loadedProperties)) {
+    loadingPromises.push(enumSymbols(getObjectClient(), start, end));
+  }
+
+  if (loadingPromises.length === 0) {
+    return null;
+  }
+
+  return Promise.all(loadingPromises)
+    .then(responses => responses.reduce((accumulator, res) => {
+      // Let's loop through the responses to build a single response object.
+      Object.entries(res).forEach(([k, v]) => {
+        if (accumulator.hasOwnProperty(k)) {
+          if (Array.isArray(accumulator[k])) {
+            accumulator[k].push(...v);
+          } else if (typeof accumulator[k] === "object") {
+            accumulator[k] = Object.assign({}, accumulator[k], v);
+          }
+        } else {
+          accumulator[k] = v;
+        }
+      });
+      return accumulator;
+    }, {}));
+}
+
 function shouldLoadItemIndexedProperties(
   item: Node,
   loadedProperties: LoadedProperties = new Map()
@@ -878,6 +950,7 @@ module.exports = {
   nodeNeedsNumericalBuckets,
   nodeSupportsNumericalBucketing,
   setNodeChildren,
+  loadItemProperties,
   shouldLoadItemEntries,
   shouldLoadItemIndexedProperties,
   shouldLoadItemNonIndexedProperties,