From 87f6eb84e9ea263748b261f6e1a255e1a55e5cf4 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 14 Jan 2019 14:53:22 -0800 Subject: [PATCH] Support configurable labels for custom hooks (#14559) * react-debug-tools accepts currentDispatcher ref as param * ReactDebugHooks injected dispatcher ref is optional * Support custom values for custom hooks * PR feedback: 1. Renamed useDebugValueLabel hook to useDebugValue 2. Wrapped useDebugValue internals in if-DEV so that it could be removed from production builds. * 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 * Nitpick renamed a method --- .../react-debug-tools/src/ReactDebugHooks.js | 50 +++++- .../ReactHooksInspection-test.internal.js | 33 +++- ...ooksInspectionIntegration-test.internal.js | 148 ++++++++++++++++++ .../src/ReactFiberDispatcher.js | 2 + .../react-reconciler/src/ReactFiberHooks.js | 9 ++ packages/react/src/React.js | 2 + packages/react/src/ReactHooks.js | 7 + 7 files changed, 249 insertions(+), 2 deletions(-) diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js index b00df7e46d761..1256c8fa8330f 100644 --- a/packages/react-debug-tools/src/ReactDebugHooks.js +++ b/packages/react-debug-tools/src/ReactDebugHooks.js @@ -55,6 +55,7 @@ function getPrimitiveStackCache(): Map> { Dispatcher.useLayoutEffect(() => {}); Dispatcher.useEffect(() => {}); Dispatcher.useImperativeHandle(undefined, () => null); + Dispatcher.useDebugValue(null); Dispatcher.useCallback(() => {}); Dispatcher.useMemo(() => null); } finally { @@ -180,6 +181,14 @@ function useImperativeHandle( }); } +function useDebugValue(value: any, formatterFn: ?(value: any) => any) { + hookLog.push({ + primitive: 'DebugValue', + stackError: new Error(), + value: typeof formatterFn === 'function' ? formatterFn(value) : value, + }); +} + function useCallback(callback: T, inputs: Array | void | null): T { let hook = nextHook(); hookLog.push({ @@ -206,6 +215,7 @@ const Dispatcher = { useContext, useEffect, useImperativeHandle, + useDebugValue, useLayoutEffect, useMemo, useReducer, @@ -388,7 +398,7 @@ function buildTree(rootStack, readHookLog): HooksTree { let children = []; levelChildren.push({ name: parseCustomHookName(stack[j - 1].functionName), - value: undefined, // TODO: Support custom inspectable values. + value: undefined, subHooks: children, }); stackOfChildren.push(levelChildren); @@ -402,9 +412,47 @@ function buildTree(rootStack, readHookLog): HooksTree { subHooks: [], }); } + + // Associate custom hook values (useDebugValue() hook entries) with the correct hooks. + processDebugValues(rootChildren, null); + return rootChildren; } +// Custom hooks support user-configurable labels (via the special useDebugValue() hook). +// That hook adds user-provided values to the hooks tree, +// but these values aren't intended to appear alongside of the other hooks. +// Instead they should be attributed to their parent custom hook. +// This method walks the tree and assigns debug values to their custom hook owners. +function processDebugValues( + hooksTree: HooksTree, + parentHooksNode: HooksNode | null, +): void { + let debugValueHooksNodes: Array = []; + + 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 { + processDebugValues(hooksNode.subHooks, hooksNode); + } + } + + // Bubble debug value labels to their custom hook owner. + // If there is no parent hook, just ignore them for now. + // (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); + } + } +} + export function inspectHooks( renderFunction: Props => React$Node, props: Props, diff --git a/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.internal.js b/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.internal.js index cb26bfc55636d..d84bd82fb2780 100644 --- a/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.internal.js +++ b/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.internal.js @@ -41,6 +41,7 @@ describe('ReactHooksInspection', () => { it('should inspect a simple custom hook', () => { function useCustom(value) { let [state] = React.useState(value); + React.useDebugValue('custom hook label'); return state; } function Foo(props) { @@ -51,7 +52,7 @@ describe('ReactHooksInspection', () => { expect(tree).toEqual([ { name: 'Custom', - value: undefined, + value: __DEV__ ? 'custom hook label' : undefined, subHooks: [ { name: 'State', @@ -249,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 index b242c4adc5cc0..703db2af538d8 100644 --- a/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.internal.js +++ b/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.internal.js @@ -212,6 +212,154 @@ describe('ReactHooksInspectionIntergration', () => { ]); }); + describe('useDebugValue', () => { + it('should support inspectable values for multiple custom hooks', () => { + function useLabeledValue(label) { + let [value] = React.useState(label); + React.useDebugValue(`custom label ${label}`); + return value; + } + function useAnonymous(label) { + let [value] = React.useState(label); + return value; + } + function Example() { + useLabeledValue('a'); + React.useState('b'); + useAnonymous('c'); + useLabeledValue('d'); + return null; + } + let renderer = ReactTestRenderer.create(); + let childFiber = renderer.root.findByType(Example)._currentFiber(); + let tree = ReactDebugTools.inspectHooksOfFiber(childFiber); + expect(tree).toEqual([ + { + name: 'LabeledValue', + value: __DEV__ ? 'custom label a' : undefined, + subHooks: [{name: 'State', value: 'a', subHooks: []}], + }, + { + name: 'State', + value: 'b', + subHooks: [], + }, + { + name: 'Anonymous', + value: undefined, + subHooks: [{name: 'State', value: 'c', subHooks: []}], + }, + { + name: 'LabeledValue', + value: __DEV__ ? 'custom label d' : undefined, + subHooks: [{name: 'State', value: 'd', subHooks: []}], + }, + ]); + }); + + it('should support inspectable values for nested custom hooks', () => { + function useInner() { + React.useDebugValue('inner'); + React.useState(0); + } + function useOuter() { + React.useDebugValue('outer'); + useInner(); + } + function Example() { + useOuter(); + return null; + } + let renderer = ReactTestRenderer.create(); + let childFiber = renderer.root.findByType(Example)._currentFiber(); + let tree = ReactDebugTools.inspectHooksOfFiber(childFiber); + expect(tree).toEqual([ + { + name: 'Outer', + value: __DEV__ ? 'outer' : undefined, + subHooks: [ + { + name: 'Inner', + value: __DEV__ ? 'inner' : undefined, + subHooks: [{name: 'State', value: 0, subHooks: []}], + }, + ], + }, + ]); + }); + + it('should support multiple inspectable values per custom hooks', () => { + function useMultiLabelCustom() { + React.useDebugValue('one'); + React.useDebugValue('two'); + React.useDebugValue('three'); + React.useState(0); + } + function useSingleLabelCustom(value) { + React.useDebugValue(`single ${value}`); + React.useState(0); + } + function Example() { + useSingleLabelCustom('one'); + useMultiLabelCustom(); + useSingleLabelCustom('two'); + return null; + } + let renderer = ReactTestRenderer.create(); + let childFiber = renderer.root.findByType(Example)._currentFiber(); + let tree = ReactDebugTools.inspectHooksOfFiber(childFiber); + expect(tree).toEqual([ + { + name: 'SingleLabelCustom', + value: __DEV__ ? 'single one' : undefined, + subHooks: [{name: 'State', value: 0, subHooks: []}], + }, + { + name: 'MultiLabelCustom', + value: __DEV__ ? ['one', 'two', 'three'] : undefined, + subHooks: [{name: 'State', value: 0, subHooks: []}], + }, + { + name: 'SingleLabelCustom', + value: __DEV__ ? 'single two' : undefined, + subHooks: [{name: 'State', value: 0, subHooks: []}], + }, + ]); + }); + + it('should ignore useDebugValue() made outside of a custom hook', () => { + function Example() { + React.useDebugValue('this is invalid'); + return null; + } + let renderer = ReactTestRenderer.create(); + 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(); + 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 () => { let Suspense = React.Suspense; diff --git a/packages/react-reconciler/src/ReactFiberDispatcher.js b/packages/react-reconciler/src/ReactFiberDispatcher.js index d29267f84b295..ed2ed3b2f315b 100644 --- a/packages/react-reconciler/src/ReactFiberDispatcher.js +++ b/packages/react-reconciler/src/ReactFiberDispatcher.js @@ -13,6 +13,7 @@ import { useContext, useEffect, useImperativeHandle, + useDebugValue, useLayoutEffect, useMemo, useReducer, @@ -26,6 +27,7 @@ export const Dispatcher = { useContext, useEffect, useImperativeHandle, + useDebugValue, useLayoutEffect, useMemo, useReducer, diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index b859f4067b6a6..2f8d9a24a5658 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -588,6 +588,15 @@ export function useImperativeHandle( }, nextInputs); } +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 custom hook values. +} + export function useCallback( callback: T, inputs: Array | void | null, diff --git a/packages/react/src/React.js b/packages/react/src/React.js index 9544e21ad99e9..4b868feca9c88 100644 --- a/packages/react/src/React.js +++ b/packages/react/src/React.js @@ -33,6 +33,7 @@ import { useContext, useEffect, useImperativeHandle, + useDebugValue, useLayoutEffect, useMemo, useReducer, @@ -99,6 +100,7 @@ if (enableHooks) { React.useContext = useContext; React.useEffect = useEffect; React.useImperativeHandle = useImperativeHandle; + React.useDebugValue = useDebugValue; React.useLayoutEffect = useLayoutEffect; React.useMemo = useMemo; React.useReducer = useReducer; diff --git a/packages/react/src/ReactHooks.js b/packages/react/src/ReactHooks.js index e5fa9d3a507c4..37f85cd5c3537 100644 --- a/packages/react/src/ReactHooks.js +++ b/packages/react/src/ReactHooks.js @@ -110,3 +110,10 @@ export function useImperativeHandle( const dispatcher = resolveDispatcher(); return dispatcher.useImperativeHandle(ref, create, inputs); } + +export function useDebugValue(value: any, formatterFn: ?(value: any) => any) { + if (__DEV__) { + const dispatcher = resolveDispatcher(); + return dispatcher.useDebugValue(value, formatterFn); + } +}