expand components slowly

Summary:
This diff introduces the concept of `shouldExpand` in DataInspector. Rather than expanding components right away, we only expand if the CPU is idly, this makes sure our app remains interactive, rather than stalling for 15 seconds as shown in the example query.

In the future we could solve the blocking by using react suspense as well, but this solution has the nice benefit that it allows inspecting and interacting with the data right away.

Changelog: The JSON inspector in plugins like GraphQL no longer freezes Flipper temporarily when expanding large data sets and will remain interactive during

Reviewed By: jknoxville

Differential Revision: D21302821

fbshipit-source-id: 6a53858f9062175596dc695c4af172d60422abe7

desktop/app/src/ui/components/data-inspector/DataInspector.tsx

@@ -22,6 +22,7 @@ import {clipboard} from 'electron';
 import deepEqual from 'deep-equal';
 import React from 'react';
 import {TooltipOptions} from '../TooltipProvider';
+import {shallowEqual} from 'react-redux';
 
 export {DataValueExtractor} from './DataPreview';
 
@@ -266,16 +267,7 @@ const diffMetadataExtractor: DiffMetadataExtractor = (
   return Object.prototype.hasOwnProperty.call(data, key) ? [{data: val}] : [];
 };
 
-function isComponentExpanded(
-  data: any,
-  diffType: string,
-  diffValue: any,
-  isExpanded: boolean,
-) {
-  if (isExpanded) {
-    return true;
-  }
-
+function isComponentExpanded(data: any, diffType: string, diffValue: any) {
   if (diffValue == null) {
     return false;
   }
@@ -299,6 +291,7 @@ function isComponentExpanded(
 }
 
 type DataInspectorState = {
+  shouldExpand: boolean;
   isExpanded: boolean;
   isExpandable: boolean;
   res: any;
@@ -327,12 +320,14 @@ export default class DataInspector extends Component<
     ancestry: [],
   };
 
+  expandHandle: any = undefined;
   interaction: (name: string, data?: any) => void;
 
   constructor(props: DataInspectorProps) {
     super(props);
     this.interaction = reportInteraction('DataInspector', props.path.join(':'));
     this.state = {
+      shouldExpand: false,
       isExpandable: false,
       isExpanded: false,
       res: undefined,
@@ -346,7 +341,13 @@ export default class DataInspector extends Component<
     );
   }
 
-  shouldComponentUpdate(nextProps: DataInspectorProps) {
+  shouldComponentUpdate(
+    nextProps: DataInspectorProps,
+    nextState: DataInspectorState,
+  ) {
+    if (!shallowEqual(nextState, this.state)) {
+      return true;
+    }
     const {props} = this;
 
     // check if any expanded paths effect this subtree
@@ -382,7 +383,7 @@ export default class DataInspector extends Component<
 
   static getDerivedStateFromProps(
     nextProps: DataInspectorProps,
-    _currentState: DataInspectorState,
+    currentState: DataInspectorState,
   ): DataInspectorState {
     const {data, depth, diff, expandRoot, path} = nextProps;
 
@@ -391,6 +392,7 @@ export default class DataInspector extends Component<
 
     if (!res) {
       return {
+        shouldExpand: false,
         isExpanded: false,
         isExpandable: false,
         res,
@@ -399,29 +401,35 @@ export default class DataInspector extends Component<
     }
 
     const isExpandable = DataInspector.isExpandable(res.value);
-    const isExpanded =
-      isExpandable &&
-      (resDiff != null
-        ? isComponentExpanded(
-            res.value,
-            resDiff.type,
-            resDiff.value,
-            expandRoot === true || DataInspector.isExpanded(nextProps, path),
-          )
-        : expandRoot === true || DataInspector.isExpanded(nextProps, path));
+    let shouldExpand = false;
+    if (isExpandable) {
+      if (
+        expandRoot === true ||
+        DataInspector.shouldBeExpanded(nextProps, path)
+      ) {
+        shouldExpand = true;
+      } else if (resDiff) {
+        shouldExpand = isComponentExpanded(
+          res.value,
+          resDiff.type,
+          resDiff.value,
+        );
+      }
+    }
 
     return {
-      isExpanded,
+      isExpanded: currentState.isExpanded,
+      shouldExpand,
       isExpandable,
       res,
       resDiff,
     };
   }
 
-  static isExpanded(props: DataInspectorProps, pathParts: Array<string>) {
+  static shouldBeExpanded(props: DataInspectorProps, pathParts: Array<string>) {
     const {expanded} = props;
 
-    // if we no expanded object then expand everything
+    // if we have no expanded object then expand everything
     if (expanded == null) {
       return true;
     }
@@ -442,6 +450,33 @@ export default class DataInspector extends Component<
     return true;
   }
 
+  componentDidMount() {
+    this.expandIfNeeded();
+  }
+
+  componentDidUpdate() {
+    this.expandIfNeeded();
+  }
+
+  componentWillUnmount() {
+    cancelIdleCallback(this.expandHandle);
+  }
+
+  expandIfNeeded() {
+    if (this.state.isExpanded !== this.state.shouldExpand) {
+      cancelIdleCallback(this.expandHandle);
+      if (!this.state.shouldExpand) {
+        this.setState({isExpanded: false});
+      } else {
+        this.expandHandle = requestIdleCallback(() => {
+          this.setState({
+            isExpanded: true,
+          });
+        });
+      }
+    }
+  }
+
   setExpanded(pathParts: Array<string>, isExpanded: boolean) {
     const {expanded, onExpanded} = this.props;
     if (!onExpanded || !expanded) {
@@ -457,7 +492,11 @@ export default class DataInspector extends Component<
   }
 
   handleClick = () => {
-    const isExpanded = DataInspector.isExpanded(this.props, this.props.path);
+    cancelIdleCallback(this.expandHandle);
+    const isExpanded = DataInspector.shouldBeExpanded(
+      this.props,
+      this.props.path,
+    );
     this.interaction(isExpanded ? 'collapsed' : 'expanded');
     this.setExpanded(this.props.path, !isExpanded);
   };

desktop/app/src/ui/components/data-inspector/__tests__/DataInspector.node.tsx

@@ -8,11 +8,40 @@
  */
 
 import * as React from 'react';
-import {render, fireEvent} from '@testing-library/react';
+import {render, fireEvent, waitFor} from '@testing-library/react';
 
 jest.mock('../../../../fb/Logger');
 import ManagedDataInspector from '../ManagedDataInspector';
 
+const mocks = {
+  requestIdleCallback(fn: Function) {
+    return setTimeout(fn, 1);
+  },
+  cancelIdleCallback(handle: any) {
+    clearTimeout(handle);
+  },
+};
+
+beforeAll(() => {
+  Object.keys(mocks).forEach((key) => {
+    // @ts-ignore
+    if (!global[key]) {
+      // @ts-ignore
+      global[key] = mocks[key];
+    }
+  });
+});
+
+afterAll(() => {
+  Object.keys(mocks).forEach((key) => {
+    // @ts-ignore
+    if (global[key] === mocks[key]) {
+      // @ts-ignore
+      delete global[key];
+    }
+  });
+});
+
 const json = {
   data: {
     is: {
@@ -26,13 +55,13 @@ const json = {
 
 test('changing collapsed property works', async () => {
   const res = render(<ManagedDataInspector data={json} collapsed expandRoot />);
-  expect((await res.queryAllByText(/is/)).length).toBe(1); // from expandRoot
+  expect(await res.findByText(/is/)).toBeTruthy(); // from expandRoot
   expect((await res.queryAllByText(/cool/)).length).toBe(0);
 
   res.rerender(
     <ManagedDataInspector data={json} collapsed={false} expandRoot />,
   );
-  expect((await res.queryAllByText(/cool/)).length).toBe(1);
+  await waitFor(() => res.findByText(/cool/));
 
   res.rerender(
     <ManagedDataInspector data={json} collapsed={true} expandRoot />,
@@ -42,24 +71,28 @@ test('changing collapsed property works', async () => {
 
 test('can manually collapse properties', async () => {
   const res = render(<ManagedDataInspector data={json} collapsed expandRoot />);
+
+  await res.findByText(/is/); // previewed as key, like: "data: {is, and}"
   expect((await res.queryAllByText(/awesomely/)).length).toBe(0);
-  expect((await res.queryAllByText(/is/)).length).toBe(1); // previewed as key, like: "data: {is, and}"
 
   // expand twice
   fireEvent.click(await res.findByText(/data/));
-  expect((await res.queryAllByText(/awesomely/)).length).toBe(1);
+  await res.findByText(/awesomely/);
   expect((await res.queryAllByText(/cool/)).length).toBe(0);
+
   fireEvent.click(await res.findByText(/is/));
-  expect((await res.queryAllByText(/cool/)).length).toBe(1);
+  await res.findByText(/cool/);
   expect((await res.queryAllByText(/json/)).length).toBe(0); // this node is not shown
 
   // collapsing everything again
   fireEvent.click(await res.findByText(/data/));
-  expect((await res.queryAllByText(/awesomely/)).length).toBe(0);
+  await waitFor(() => {
+    expect(res.queryByText(/awesomely/)).toBeNull();
+  });
 
   // expand everything again, expanded paths will have been remembered
   fireEvent.click(await res.findByText(/data/));
-  expect((await res.queryAllByText(/is/)).length).toBe(1);
-  expect((await res.queryAllByText(/awesomely/)).length).toBe(1);
+  await res.findByText(/is/);
+  await res.findByText(/awesomely/);
   expect((await res.queryAllByText(/json/)).length).toBe(0);
 });