PR feedback:

1. Fixed some minor typos 2. Added inline comment explaining the purpose of rollupDebugValues() 3. Refactored rollupDebugValues() to use a for loop rather than filter() 4. Improve check for useDebugValue hook to lessen the chance of a false positive 5. Added optional formatter function param to useDebugValue
facebook · Jan 10, 2019 · 683907b · 683907b
1 parent 5d7b4be
commit 683907b
Show file tree

Hide file tree

Showing 5 changed files with 104 additions and 24 deletions.
diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js
@@ -181,11 +181,11 @@ function useImperativeHandle<T>(
   });
 }
 
-function useDebugValue(valueLabel: any) {
+function useDebugValue(value: any, formatterFn: ?(value: any) => any) {
   hookLog.push({
     primitive: 'DebugValue',
     stackError: new Error(),
-    value: valueLabel,
+    value: typeof formatterFn === 'function' ? formatterFn(value) : value,
   });
 }
 
@@ -413,27 +413,42 @@ function buildTree(rootStack, readHookLog): HooksTree {
     });
   }
 
-  // Associate custom hook values (useInpect() hook entries) with the correct hooks
-  rootChildren.forEach(hooksNode => rollupDebugValues(hooksNode));
+  // Associate custom hook values (useDebugValue() hook entries) with the correct hooks.
+  rollupDebugValues(rootChildren, null);
 
   return rootChildren;
 }
 
-function rollupDebugValues(hooksNode: HooksNode): void {
-  let useInpectHooksNodes: Array<HooksNode> = [];
-  hooksNode.subHooks = hooksNode.subHooks.filter(subHooksNode => {
-    if (subHooksNode.name === 'DebugValue') {
-      useInpectHooksNodes.push(subHooksNode);
-      return false;
+// Custom hooks support user-configurable labels (via the useDebugValue() hook).
+// That hook adds the user-provided values to the hooks tree.
+// This method removes those values (so they don't appear in DevTools),
+// and bubbles them up to the "value" attribute of their parent custom hook.
+function rollupDebugValues(
+  hooksTree: HooksTree,
+  parentHooksNode: HooksNode | null,
+): void {
+  let debugValueHooksNodes: Array<HooksNode> = [];
+
+  for (let i = 0; i < hooksTree.length; i++) {
+    const hooksNode = hooksTree[i];
+    if (hooksNode.name === 'DebugValue' && hooksNode.subHooks.length === 0) {
+      hooksTree.splice(i, 1);
+      i--;
+      debugValueHooksNodes.push(hooksNode);
     } else {
-      rollupDebugValues(subHooksNode);
-      return true;
+      rollupDebugValues(hooksNode.subHooks, hooksNode);
+    }
+  }
+
+  // Bubble debug value labels to their parent custom hook.
+  // If there is no parent hook, just ignore them.
+  // (We may warn about this in the future.)
+  if (parentHooksNode !== null) {
+    if (debugValueHooksNodes.length === 1) {
+      parentHooksNode.value = debugValueHooksNodes[0].value;
+    } else if (debugValueHooksNodes.length > 1) {
+      parentHooksNode.value = debugValueHooksNodes.map(({value}) => value);
     }
-  });
-  if (useInpectHooksNodes.length === 1) {
-    hooksNode.value = useInpectHooksNodes[0].value;
-  } else if (useInpectHooksNodes.length > 1) {
-    hooksNode.value = useInpectHooksNodes.map(({value}) => value);
   }
 }
 

diff --git a/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.internal.js b/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.internal.js
@@ -250,4 +250,34 @@ describe('ReactHooksInspection', () => {
     expect(setterCalls[0]).not.toBe(initial);
     expect(setterCalls[1]).toBe(initial);
   });
+
+  describe('useDebugValue', () => {
+    it('should be ignored when called outside of a custom hook', () => {
+      function Foo(props) {
+        React.useDebugValue('this is invalid');
+        return null;
+      }
+      let tree = ReactDebugTools.inspectHooks(Foo, {});
+      expect(tree).toHaveLength(0);
+    });
+
+    it('should support an optional formatter function param', () => {
+      function useCustom() {
+        React.useDebugValue({bar: 123}, object => `bar:${object.bar}`);
+        React.useState(0);
+      }
+      function Foo(props) {
+        useCustom();
+        return null;
+      }
+      let tree = ReactDebugTools.inspectHooks(Foo, {});
+      expect(tree).toEqual([
+        {
+          name: 'Custom',
+          value: __DEV__ ? 'bar:123' : undefined,
+          subHooks: [{name: 'State', subHooks: [], value: 0}],
+        },
+      ]);
+    });
+  });
 });
diff --git a/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.internal.js b/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.internal.js
@@ -223,7 +223,7 @@ describe('ReactHooksInspectionIntergration', () => {
         let [value] = React.useState(label);
         return value;
       }
-      function Example(props) {
+      function Example() {
         useLabeledValue('a');
         React.useState('b');
         useAnonymous('c');
@@ -266,7 +266,7 @@ describe('ReactHooksInspectionIntergration', () => {
         React.useDebugValue('outer');
         useInner();
       }
-      function Example(props) {
+      function Example() {
         useOuter();
         return null;
       }
@@ -299,7 +299,7 @@ describe('ReactHooksInspectionIntergration', () => {
         React.useDebugValue(`single ${value}`);
         React.useState(0);
       }
-      function Example(props) {
+      function Example() {
         useSingleLabelCustom('one');
         useMultiLabelCustom();
         useSingleLabelCustom('two');
@@ -326,6 +326,38 @@ describe('ReactHooksInspectionIntergration', () => {
         },
       ]);
     });
+
+    it('should ignore useDebugValue() made outside of a custom hook', () => {
+      function Example() {
+        React.useDebugValue('this is invalid');
+        return null;
+      }
+      let renderer = ReactTestRenderer.create(<Example />);
+      let childFiber = renderer.root.findByType(Example)._currentFiber();
+      let tree = ReactDebugTools.inspectHooksOfFiber(childFiber);
+      expect(tree).toHaveLength(0);
+    });
+
+    it('should support an optional formatter function param', () => {
+      function useCustom() {
+        React.useDebugValue({bar: 123}, object => `bar:${object.bar}`);
+        React.useState(0);
+      }
+      function Example() {
+        useCustom();
+        return null;
+      }
+      let renderer = ReactTestRenderer.create(<Example />);
+      let childFiber = renderer.root.findByType(Example)._currentFiber();
+      let tree = ReactDebugTools.inspectHooksOfFiber(childFiber);
+      expect(tree).toEqual([
+        {
+          name: 'Custom',
+          value: __DEV__ ? 'bar:123' : undefined,
+          subHooks: [{name: 'State', subHooks: [], value: 0}],
+        },
+      ]);
+    });
   });
 
   it('should support defaultProps and lazy', async () => {

diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js
@@ -588,10 +588,13 @@ export function useImperativeHandle<T>(
   }, nextInputs);
 }
 
-export function useDebugValue(valueLabel: string): void {
+export function useDebugValue(
+  value: any,
+  formatterFn: ?(value: any) => any,
+): void {
   // This hook is normally a no-op.
   // The react-debug-hooks package injects its own implementation
-  // so that e.g. DevTools can display customhook values.
+  // so that e.g. DevTools can display custom hook values.
 }
 
 export function useCallback<T>(

diff --git a/packages/react/src/ReactHooks.js b/packages/react/src/ReactHooks.js
@@ -111,9 +111,9 @@ export function useImperativeHandle<T>(
   return dispatcher.useImperativeHandle(ref, create, inputs);
 }
 
-export function useDebugValue(valueLabel: string) {
+export function useDebugValue(value: any, formatterFn: ?(value: any) => any) {
   if (__DEV__) {
     const dispatcher = resolveDispatcher();
-    return dispatcher.useDebugValue(valueLabel);
+    return dispatcher.useDebugValue(value, formatterFn);
   }
 }