From 6f332dcc1d8b948c5ba2d333498e72a8935b7803 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 9 Jan 2019 11:27:15 -0800 Subject: [PATCH 1/6] react-debug-tools accepts currentDispatcher ref as param --- .../react-debug-tools/src/ReactDebugHooks.js | 30 ++++++++---- .../ReactHooksInspection-test.internal.js | 15 ++++-- ...ooksInspectionIntegration-test.internal.js | 46 +++++++++++++++---- 3 files changed, 66 insertions(+), 25 deletions(-) diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js index 9b5a3b4994781..819e6e1a3addc 100644 --- a/packages/react-debug-tools/src/ReactDebugHooks.js +++ b/packages/react-debug-tools/src/ReactDebugHooks.js @@ -20,7 +20,7 @@ import { ForwardRef, } from 'shared/ReactWorkTags'; -const ReactCurrentDispatcher = ReactSharedInternals.ReactCurrentDispatcher; +type CurrentDispatcherRef = typeof ReactSharedInternals.ReactCurrentDispatcher; // Used to track hooks called during a render @@ -406,12 +406,13 @@ function buildTree(rootStack, readHookLog): HooksTree { } export function inspectHooks( + currentDispatcher: CurrentDispatcherRef, renderFunction: Props => React$Node, props: Props, ): HooksTree { - let previousDispatcher = ReactCurrentDispatcher.current; + let previousDispatcher = currentDispatcher.current; let readHookLog; - ReactCurrentDispatcher.current = Dispatcher; + currentDispatcher.current = Dispatcher; let ancestorStackError; try { ancestorStackError = new Error(); @@ -419,7 +420,7 @@ export function inspectHooks( } finally { readHookLog = hookLog; hookLog = []; - ReactCurrentDispatcher.current = previousDispatcher; + currentDispatcher.current = previousDispatcher; } let rootStack = ErrorStackParser.parse(ancestorStackError); return buildTree(rootStack, readHookLog); @@ -447,13 +448,14 @@ function restoreContexts(contextMap: Map, any>) { } function inspectHooksOfForwardRef( + currentDispatcher: CurrentDispatcherRef, renderFunction: (Props, Ref) => React$Node, props: Props, ref: Ref, ): HooksTree { - let previousDispatcher = ReactCurrentDispatcher.current; + let previousDispatcher = currentDispatcher.current; let readHookLog; - ReactCurrentDispatcher.current = Dispatcher; + currentDispatcher.current = Dispatcher; let ancestorStackError; try { ancestorStackError = new Error(); @@ -461,7 +463,7 @@ function inspectHooksOfForwardRef( } finally { readHookLog = hookLog; hookLog = []; - ReactCurrentDispatcher.current = previousDispatcher; + currentDispatcher.current = previousDispatcher; } let rootStack = ErrorStackParser.parse(ancestorStackError); return buildTree(rootStack, readHookLog); @@ -482,7 +484,10 @@ function resolveDefaultProps(Component, baseProps) { return baseProps; } -export function inspectHooksOfFiber(fiber: Fiber) { +export function inspectHooksOfFiber( + currentDispatcher: CurrentDispatcherRef, + fiber: Fiber, +) { if ( fiber.tag !== FunctionComponent && fiber.tag !== SimpleMemoComponent && @@ -506,9 +511,14 @@ export function inspectHooksOfFiber(fiber: Fiber) { try { setupContexts(contextMap, fiber); if (fiber.tag === ForwardRef) { - return inspectHooksOfForwardRef(type.render, props, fiber.ref); + return inspectHooksOfForwardRef( + currentDispatcher, + type.render, + props, + fiber.ref, + ); } - return inspectHooks(type, props); + return inspectHooks(currentDispatcher, type, props); } finally { currentHook = null; restoreContexts(contextMap); 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 326c1581fee99..4611bd7d3fb8b 100644 --- a/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.internal.js +++ b/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.internal.js @@ -12,6 +12,7 @@ let React; let ReactDebugTools; +let currentDispatcher; describe('ReactHooksInspection', () => { beforeEach(() => { @@ -21,6 +22,10 @@ describe('ReactHooksInspection', () => { ReactFeatureFlags.enableHooks = true; React = require('react'); ReactDebugTools = require('react-debug-tools'); + + currentDispatcher = + React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED + .ReactCurrentDispatcher; }); it('should inspect a simple useState hook', () => { @@ -28,7 +33,7 @@ describe('ReactHooksInspection', () => { let [state] = React.useState('hello world'); return
{state}
; } - let tree = ReactDebugTools.inspectHooks(Foo, {}); + let tree = ReactDebugTools.inspectHooks(currentDispatcher, Foo, {}); expect(tree).toEqual([ { name: 'State', @@ -47,7 +52,7 @@ describe('ReactHooksInspection', () => { let value = useCustom('hello world'); return
{value}
; } - let tree = ReactDebugTools.inspectHooks(Foo, {}); + let tree = ReactDebugTools.inspectHooks(currentDispatcher, Foo, {}); expect(tree).toEqual([ { name: 'Custom', @@ -79,7 +84,7 @@ describe('ReactHooksInspection', () => { ); } - let tree = ReactDebugTools.inspectHooks(Foo, {}); + let tree = ReactDebugTools.inspectHooks(currentDispatcher, Foo, {}); expect(tree).toEqual([ { name: 'Custom', @@ -142,7 +147,7 @@ describe('ReactHooksInspection', () => { ); } - let tree = ReactDebugTools.inspectHooks(Foo, {}); + let tree = ReactDebugTools.inspectHooks(currentDispatcher, Foo, {}); expect(tree).toEqual([ { name: 'Bar', @@ -207,7 +212,7 @@ describe('ReactHooksInspection', () => { let value = React.useContext(MyContext); return
{value}
; } - let tree = ReactDebugTools.inspectHooks(Foo, {}); + let tree = ReactDebugTools.inspectHooks(currentDispatcher, Foo, {}); expect(tree).toEqual([ { name: 'Context', 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 d740bb4fdc79f..3746f5d357384 100644 --- a/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.internal.js +++ b/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.internal.js @@ -13,6 +13,7 @@ let React; let ReactTestRenderer; let ReactDebugTools; +let currentDispatcher; describe('ReactHooksInspectionIntergration', () => { beforeEach(() => { @@ -23,6 +24,10 @@ describe('ReactHooksInspectionIntergration', () => { React = require('react'); ReactTestRenderer = require('react-test-renderer'); ReactDebugTools = require('react-debug-tools'); + + currentDispatcher = + React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED + .ReactCurrentDispatcher; }); it('should inspect the current state of useState hooks', () => { @@ -39,7 +44,10 @@ describe('ReactHooksInspectionIntergration', () => { let renderer = ReactTestRenderer.create(); let childFiber = renderer.root.findByType(Foo)._currentFiber(); - let tree = ReactDebugTools.inspectHooksOfFiber(childFiber); + let tree = ReactDebugTools.inspectHooksOfFiber( + currentDispatcher, + childFiber, + ); expect(tree).toEqual([ {name: 'State', value: 'hello', subHooks: []}, {name: 'State', value: 'world', subHooks: []}, @@ -53,7 +61,7 @@ describe('ReactHooksInspectionIntergration', () => { setStateA('Hi'); childFiber = renderer.root.findByType(Foo)._currentFiber(); - tree = ReactDebugTools.inspectHooksOfFiber(childFiber); + tree = ReactDebugTools.inspectHooksOfFiber(currentDispatcher, childFiber); expect(tree).toEqual([ {name: 'State', value: 'Hi', subHooks: []}, @@ -63,7 +71,7 @@ describe('ReactHooksInspectionIntergration', () => { setStateB('world!'); childFiber = renderer.root.findByType(Foo)._currentFiber(); - tree = ReactDebugTools.inspectHooksOfFiber(childFiber); + tree = ReactDebugTools.inspectHooksOfFiber(currentDispatcher, childFiber); expect(tree).toEqual([ {name: 'State', value: 'Hi', subHooks: []}, @@ -111,7 +119,10 @@ describe('ReactHooksInspectionIntergration', () => { let {onClick: updateStates} = renderer.root.findByType('div').props; - let tree = ReactDebugTools.inspectHooksOfFiber(childFiber); + let tree = ReactDebugTools.inspectHooksOfFiber( + currentDispatcher, + childFiber, + ); expect(tree).toEqual([ {name: 'State', value: 'a', subHooks: []}, {name: 'Reducer', value: 'b', subHooks: []}, @@ -126,7 +137,7 @@ describe('ReactHooksInspectionIntergration', () => { updateStates(); childFiber = renderer.root.findByType(Foo)._currentFiber(); - tree = ReactDebugTools.inspectHooksOfFiber(childFiber); + tree = ReactDebugTools.inspectHooksOfFiber(currentDispatcher, childFiber); expect(tree).toEqual([ {name: 'State', value: 'A', subHooks: []}, @@ -152,7 +163,10 @@ describe('ReactHooksInspectionIntergration', () => { , ); let childFiber = renderer.root.findByType(Foo)._currentFiber(); - let tree = ReactDebugTools.inspectHooksOfFiber(childFiber); + let tree = ReactDebugTools.inspectHooksOfFiber( + currentDispatcher, + childFiber, + ); expect(tree).toEqual([ { name: 'Context', @@ -172,7 +186,10 @@ describe('ReactHooksInspectionIntergration', () => { let renderer = ReactTestRenderer.create(); let childFiber = renderer.root.findByType(Foo)._currentFiber(); - let tree = ReactDebugTools.inspectHooksOfFiber(childFiber); + let tree = ReactDebugTools.inspectHooksOfFiber( + currentDispatcher, + childFiber, + ); expect(tree).toEqual([ {name: 'ImperativeHandle', value: obj, subHooks: []}, ]); @@ -187,7 +204,10 @@ describe('ReactHooksInspectionIntergration', () => { let renderer = ReactTestRenderer.create(); // TODO: Test renderer findByType is broken for memo. Have to search for the inner. let childFiber = renderer.root.findByType(InnerFoo)._currentFiber(); - let tree = ReactDebugTools.inspectHooksOfFiber(childFiber); + let tree = ReactDebugTools.inspectHooksOfFiber( + currentDispatcher, + childFiber, + ); expect(tree).toEqual([{name: 'State', value: 'hello', subHooks: []}]); }); @@ -202,7 +222,10 @@ describe('ReactHooksInspectionIntergration', () => { } let renderer = ReactTestRenderer.create(); let childFiber = renderer.root.findByType(Foo)._currentFiber(); - let tree = ReactDebugTools.inspectHooksOfFiber(childFiber); + let tree = ReactDebugTools.inspectHooksOfFiber( + currentDispatcher, + childFiber, + ); expect(tree).toEqual([ { name: 'Custom', @@ -238,7 +261,10 @@ describe('ReactHooksInspectionIntergration', () => { await LazyFoo; let childFiber = renderer.root._currentFiber(); - let tree = ReactDebugTools.inspectHooksOfFiber(childFiber); + let tree = ReactDebugTools.inspectHooksOfFiber( + currentDispatcher, + childFiber, + ); expect(tree).toEqual([{name: 'State', value: 'def', subHooks: []}]); }); }); From 721f16c9626ba630d50c603d4ff12155e22dcd41 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 10 Jan 2019 08:43:27 -0800 Subject: [PATCH 2/6] ReactDebugHooks injected dispatcher ref is optional --- .../react-debug-tools/src/ReactDebugHooks.js | 22 +++-- .../ReactHooksInspection-test.internal.js | 48 ++++++++--- ...ooksInspectionIntegration-test.internal.js | 81 ++++++++++--------- 3 files changed, 100 insertions(+), 51 deletions(-) diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js index 819e6e1a3addc..b00df7e46d761 100644 --- a/packages/react-debug-tools/src/ReactDebugHooks.js +++ b/packages/react-debug-tools/src/ReactDebugHooks.js @@ -406,10 +406,16 @@ function buildTree(rootStack, readHookLog): HooksTree { } export function inspectHooks( - currentDispatcher: CurrentDispatcherRef, renderFunction: Props => React$Node, props: Props, + currentDispatcher: ?CurrentDispatcherRef, ): HooksTree { + // DevTools will pass the current renderer's injected dispatcher. + // Other apps might compile debug hooks as part of their app though. + if (currentDispatcher == null) { + currentDispatcher = ReactSharedInternals.ReactCurrentDispatcher; + } + let previousDispatcher = currentDispatcher.current; let readHookLog; currentDispatcher.current = Dispatcher; @@ -448,10 +454,10 @@ function restoreContexts(contextMap: Map, any>) { } function inspectHooksOfForwardRef( - currentDispatcher: CurrentDispatcherRef, renderFunction: (Props, Ref) => React$Node, props: Props, ref: Ref, + currentDispatcher: CurrentDispatcherRef, ): HooksTree { let previousDispatcher = currentDispatcher.current; let readHookLog; @@ -485,9 +491,15 @@ function resolveDefaultProps(Component, baseProps) { } export function inspectHooksOfFiber( - currentDispatcher: CurrentDispatcherRef, fiber: Fiber, + currentDispatcher: ?CurrentDispatcherRef, ) { + // DevTools will pass the current renderer's injected dispatcher. + // Other apps might compile debug hooks as part of their app though. + if (currentDispatcher == null) { + currentDispatcher = ReactSharedInternals.ReactCurrentDispatcher; + } + if ( fiber.tag !== FunctionComponent && fiber.tag !== SimpleMemoComponent && @@ -512,13 +524,13 @@ export function inspectHooksOfFiber( setupContexts(contextMap, fiber); if (fiber.tag === ForwardRef) { return inspectHooksOfForwardRef( - currentDispatcher, type.render, props, fiber.ref, + currentDispatcher, ); } - return inspectHooks(currentDispatcher, type, props); + return inspectHooks(type, props, currentDispatcher); } finally { currentHook = null; restoreContexts(contextMap); 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 4611bd7d3fb8b..cb26bfc55636d 100644 --- a/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.internal.js +++ b/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.internal.js @@ -12,7 +12,6 @@ let React; let ReactDebugTools; -let currentDispatcher; describe('ReactHooksInspection', () => { beforeEach(() => { @@ -22,10 +21,6 @@ describe('ReactHooksInspection', () => { ReactFeatureFlags.enableHooks = true; React = require('react'); ReactDebugTools = require('react-debug-tools'); - - currentDispatcher = - React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED - .ReactCurrentDispatcher; }); it('should inspect a simple useState hook', () => { @@ -33,7 +28,7 @@ describe('ReactHooksInspection', () => { let [state] = React.useState('hello world'); return
{state}
; } - let tree = ReactDebugTools.inspectHooks(currentDispatcher, Foo, {}); + let tree = ReactDebugTools.inspectHooks(Foo, {}); expect(tree).toEqual([ { name: 'State', @@ -52,7 +47,7 @@ describe('ReactHooksInspection', () => { let value = useCustom('hello world'); return
{value}
; } - let tree = ReactDebugTools.inspectHooks(currentDispatcher, Foo, {}); + let tree = ReactDebugTools.inspectHooks(Foo, {}); expect(tree).toEqual([ { name: 'Custom', @@ -84,7 +79,7 @@ describe('ReactHooksInspection', () => { ); } - let tree = ReactDebugTools.inspectHooks(currentDispatcher, Foo, {}); + let tree = ReactDebugTools.inspectHooks(Foo, {}); expect(tree).toEqual([ { name: 'Custom', @@ -147,7 +142,7 @@ describe('ReactHooksInspection', () => { ); } - let tree = ReactDebugTools.inspectHooks(currentDispatcher, Foo, {}); + let tree = ReactDebugTools.inspectHooks(Foo, {}); expect(tree).toEqual([ { name: 'Bar', @@ -212,7 +207,7 @@ describe('ReactHooksInspection', () => { let value = React.useContext(MyContext); return
{value}
; } - let tree = ReactDebugTools.inspectHooks(currentDispatcher, Foo, {}); + let tree = ReactDebugTools.inspectHooks(Foo, {}); expect(tree).toEqual([ { name: 'Context', @@ -221,4 +216,37 @@ describe('ReactHooksInspection', () => { }, ]); }); + + it('should support an injected dispatcher', () => { + function Foo(props) { + let [state] = React.useState('hello world'); + return
{state}
; + } + + let initial = {}; + let current = initial; + let getterCalls = 0; + let setterCalls = []; + let FakeDispatcherRef = { + get current() { + getterCalls++; + return current; + }, + set current(value) { + setterCalls.push(value); + current = value; + }, + }; + + expect(() => { + ReactDebugTools.inspectHooks(Foo, {}, FakeDispatcherRef); + }).toThrow( + 'Hooks can only be called inside the body of a function component.', + ); + + expect(getterCalls).toBe(1); + expect(setterCalls).toHaveLength(2); + expect(setterCalls[0]).not.toBe(initial); + expect(setterCalls[1]).toBe(initial); + }); }); 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 3746f5d357384..b242c4adc5cc0 100644 --- a/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.internal.js +++ b/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.internal.js @@ -13,7 +13,6 @@ let React; let ReactTestRenderer; let ReactDebugTools; -let currentDispatcher; describe('ReactHooksInspectionIntergration', () => { beforeEach(() => { @@ -24,10 +23,6 @@ describe('ReactHooksInspectionIntergration', () => { React = require('react'); ReactTestRenderer = require('react-test-renderer'); ReactDebugTools = require('react-debug-tools'); - - currentDispatcher = - React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED - .ReactCurrentDispatcher; }); it('should inspect the current state of useState hooks', () => { @@ -44,10 +39,7 @@ describe('ReactHooksInspectionIntergration', () => { let renderer = ReactTestRenderer.create(); let childFiber = renderer.root.findByType(Foo)._currentFiber(); - let tree = ReactDebugTools.inspectHooksOfFiber( - currentDispatcher, - childFiber, - ); + let tree = ReactDebugTools.inspectHooksOfFiber(childFiber); expect(tree).toEqual([ {name: 'State', value: 'hello', subHooks: []}, {name: 'State', value: 'world', subHooks: []}, @@ -61,7 +53,7 @@ describe('ReactHooksInspectionIntergration', () => { setStateA('Hi'); childFiber = renderer.root.findByType(Foo)._currentFiber(); - tree = ReactDebugTools.inspectHooksOfFiber(currentDispatcher, childFiber); + tree = ReactDebugTools.inspectHooksOfFiber(childFiber); expect(tree).toEqual([ {name: 'State', value: 'Hi', subHooks: []}, @@ -71,7 +63,7 @@ describe('ReactHooksInspectionIntergration', () => { setStateB('world!'); childFiber = renderer.root.findByType(Foo)._currentFiber(); - tree = ReactDebugTools.inspectHooksOfFiber(currentDispatcher, childFiber); + tree = ReactDebugTools.inspectHooksOfFiber(childFiber); expect(tree).toEqual([ {name: 'State', value: 'Hi', subHooks: []}, @@ -119,10 +111,7 @@ describe('ReactHooksInspectionIntergration', () => { let {onClick: updateStates} = renderer.root.findByType('div').props; - let tree = ReactDebugTools.inspectHooksOfFiber( - currentDispatcher, - childFiber, - ); + let tree = ReactDebugTools.inspectHooksOfFiber(childFiber); expect(tree).toEqual([ {name: 'State', value: 'a', subHooks: []}, {name: 'Reducer', value: 'b', subHooks: []}, @@ -137,7 +126,7 @@ describe('ReactHooksInspectionIntergration', () => { updateStates(); childFiber = renderer.root.findByType(Foo)._currentFiber(); - tree = ReactDebugTools.inspectHooksOfFiber(currentDispatcher, childFiber); + tree = ReactDebugTools.inspectHooksOfFiber(childFiber); expect(tree).toEqual([ {name: 'State', value: 'A', subHooks: []}, @@ -163,10 +152,7 @@ describe('ReactHooksInspectionIntergration', () => { , ); let childFiber = renderer.root.findByType(Foo)._currentFiber(); - let tree = ReactDebugTools.inspectHooksOfFiber( - currentDispatcher, - childFiber, - ); + let tree = ReactDebugTools.inspectHooksOfFiber(childFiber); expect(tree).toEqual([ { name: 'Context', @@ -186,10 +172,7 @@ describe('ReactHooksInspectionIntergration', () => { let renderer = ReactTestRenderer.create(); let childFiber = renderer.root.findByType(Foo)._currentFiber(); - let tree = ReactDebugTools.inspectHooksOfFiber( - currentDispatcher, - childFiber, - ); + let tree = ReactDebugTools.inspectHooksOfFiber(childFiber); expect(tree).toEqual([ {name: 'ImperativeHandle', value: obj, subHooks: []}, ]); @@ -204,10 +187,7 @@ describe('ReactHooksInspectionIntergration', () => { let renderer = ReactTestRenderer.create(); // TODO: Test renderer findByType is broken for memo. Have to search for the inner. let childFiber = renderer.root.findByType(InnerFoo)._currentFiber(); - let tree = ReactDebugTools.inspectHooksOfFiber( - currentDispatcher, - childFiber, - ); + let tree = ReactDebugTools.inspectHooksOfFiber(childFiber); expect(tree).toEqual([{name: 'State', value: 'hello', subHooks: []}]); }); @@ -222,10 +202,7 @@ describe('ReactHooksInspectionIntergration', () => { } let renderer = ReactTestRenderer.create(); let childFiber = renderer.root.findByType(Foo)._currentFiber(); - let tree = ReactDebugTools.inspectHooksOfFiber( - currentDispatcher, - childFiber, - ); + let tree = ReactDebugTools.inspectHooksOfFiber(childFiber); expect(tree).toEqual([ { name: 'Custom', @@ -261,10 +238,42 @@ describe('ReactHooksInspectionIntergration', () => { await LazyFoo; let childFiber = renderer.root._currentFiber(); - let tree = ReactDebugTools.inspectHooksOfFiber( - currentDispatcher, - childFiber, - ); + let tree = ReactDebugTools.inspectHooksOfFiber(childFiber); expect(tree).toEqual([{name: 'State', value: 'def', subHooks: []}]); }); + + it('should support an injected dispatcher', () => { + function Foo(props) { + let [state] = React.useState('hello world'); + return
{state}
; + } + + let initial = {}; + let current = initial; + let getterCalls = 0; + let setterCalls = []; + let FakeDispatcherRef = { + get current() { + getterCalls++; + return current; + }, + set current(value) { + setterCalls.push(value); + current = value; + }, + }; + + let renderer = ReactTestRenderer.create(); + let childFiber = renderer.root._currentFiber(); + expect(() => { + ReactDebugTools.inspectHooksOfFiber(childFiber, FakeDispatcherRef); + }).toThrow( + 'Hooks can only be called inside the body of a function component.', + ); + + expect(getterCalls).toBe(1); + expect(setterCalls).toHaveLength(2); + expect(setterCalls[0]).not.toBe(initial); + expect(setterCalls[1]).toBe(initial); + }); }); From 9d30fb25fb5c164651d79794e97f7e7fd0b5563e Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 9 Jan 2019 21:13:52 -0800 Subject: [PATCH 3/6] Support custom values for custom hooks --- .../react-debug-tools/src/ReactDebugHooks.js | 38 +++++- .../ReactHooksInspection-test.internal.js | 3 +- ...ooksInspectionIntegration-test.internal.js | 116 ++++++++++++++++++ .../src/ReactFiberDispatcher.js | 2 + .../react-reconciler/src/ReactFiberHooks.js | 6 + packages/react/src/React.js | 6 + packages/react/src/ReactHooks.js | 5 + 7 files changed, 174 insertions(+), 2 deletions(-) diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js index b00df7e46d761..2d8f8647ad65c 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.useDebugValueLabel(null); Dispatcher.useCallback(() => {}); Dispatcher.useMemo(() => null); } finally { @@ -180,6 +181,14 @@ function useImperativeHandle( }); } +function useDebugValueLabel(valueLabel: any) { + hookLog.push({ + primitive: 'DebugValueLabel', + stackError: new Error(), + value: valueLabel, + }); +} + function useCallback(callback: T, inputs: Array | void | null): T { let hook = nextHook(); hookLog.push({ @@ -205,7 +214,12 @@ const Dispatcher = { useCallback, useContext, useEffect, +<<<<<<< HEAD useImperativeHandle, +======= + useImperativeMethods, + useDebugValueLabel, +>>>>>>> Support custom values for custom hooks useLayoutEffect, useMemo, useReducer, @@ -388,7 +402,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 +416,31 @@ function buildTree(rootStack, readHookLog): HooksTree { subHooks: [], }); } + + // Associate custom hook values (useInpect() hook entries) with the correct hooks + rootChildren.forEach(hooksNode => rollupDebugValueLabels(hooksNode)); + return rootChildren; } +function rollupDebugValueLabels(hooksNode: HooksNode): void { + let useInpectHooksNodes: Array = []; + hooksNode.subHooks = hooksNode.subHooks.filter(subHooksNode => { + if (subHooksNode.name === 'DebugValueLabel') { + useInpectHooksNodes.push(subHooksNode); + return false; + } else { + rollupDebugValueLabels(subHooksNode); + return true; + } + }); + if (useInpectHooksNodes.length === 1) { + hooksNode.value = useInpectHooksNodes[0].value; + } else if (useInpectHooksNodes.length > 1) { + hooksNode.value = useInpectHooksNodes.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..47e8fc0a3b5ab 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.useDebugValueLabel('custom hook label'); return state; } function Foo(props) { @@ -51,7 +52,7 @@ describe('ReactHooksInspection', () => { expect(tree).toEqual([ { name: 'Custom', - value: undefined, + value: 'custom hook label', subHooks: [ { name: 'State', 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..a117b1e269b33 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,122 @@ describe('ReactHooksInspectionIntergration', () => { ]); }); + describe('useDebugValueLabel', () => { + it('should support inspectable values for multiple custom hooks', () => { + function useLabeledValue(label) { + let [value] = React.useState(label); + React.useDebugValueLabel(`custom label ${label}`); + return value; + } + function useAnonymous(label) { + let [value] = React.useState(label); + return value; + } + function Example(props) { + 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( + currentDispatcher, + childFiber, + ); + expect(tree).toEqual([ + { + name: 'LabeledValue', + value: 'custom label a', + subHooks: [{name: 'State', value: 'a', subHooks: []}], + }, + { + name: 'State', + value: 'b', + subHooks: [], + }, + { + name: 'Anonymous', + value: undefined, + subHooks: [{name: 'State', value: 'c', subHooks: []}], + }, + { + name: 'LabeledValue', + value: 'custom label d', + subHooks: [{name: 'State', value: 'd', subHooks: []}], + }, + ]); + }); + + it('should support inspectable values for nested custom hooks', () => { + function useInner() { + React.useDebugValueLabel('inner'); + } + function useOuter() { + React.useDebugValueLabel('outer'); + useInner(); + } + function Example(props) { + useOuter(); + return null; + } + let renderer = ReactTestRenderer.create(); + let childFiber = renderer.root.findByType(Example)._currentFiber(); + let tree = ReactDebugTools.inspectHooksOfFiber( + currentDispatcher, + childFiber, + ); + expect(tree).toEqual([ + { + name: 'Outer', + value: 'outer', + subHooks: [{name: 'Inner', value: 'inner', subHooks: []}], + }, + ]); + }); + + it('should support multiple inspectable values per custom hooks', () => { + function useMultiLabelCustom() { + React.useDebugValueLabel('one'); + React.useDebugValueLabel('two'); + React.useDebugValueLabel('three'); + } + function useSingleLabelCustom(value) { + React.useDebugValueLabel(`single ${value}`); + } + function Example(props) { + useSingleLabelCustom('one'); + useMultiLabelCustom(); + useSingleLabelCustom('two'); + return null; + } + let renderer = ReactTestRenderer.create(); + let childFiber = renderer.root.findByType(Example)._currentFiber(); + let tree = ReactDebugTools.inspectHooksOfFiber( + currentDispatcher, + childFiber, + ); + expect(tree).toEqual([ + { + name: 'SingleLabelCustom', + value: 'single one', + subHooks: [], + }, + { + name: 'MultiLabelCustom', + value: ['one', 'two', 'three'], + subHooks: [], + }, + { + name: 'SingleLabelCustom', + value: 'single two', + subHooks: [], + }, + ]); + }); + }); + 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..806c543fd0e23 100644 --- a/packages/react-reconciler/src/ReactFiberDispatcher.js +++ b/packages/react-reconciler/src/ReactFiberDispatcher.js @@ -13,6 +13,7 @@ import { useContext, useEffect, useImperativeHandle, + useDebugValueLabel, useLayoutEffect, useMemo, useReducer, @@ -26,6 +27,7 @@ export const Dispatcher = { useContext, useEffect, useImperativeHandle, + useDebugValueLabel, useLayoutEffect, useMemo, useReducer, diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index b859f4067b6a6..71346a3b25cea 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -588,6 +588,12 @@ export function useImperativeHandle( }, nextInputs); } +export function useDebugValueLabel(valueLabel: string): 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. +} + 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..11821bcb10c73 100644 --- a/packages/react/src/React.js +++ b/packages/react/src/React.js @@ -33,6 +33,7 @@ import { useContext, useEffect, useImperativeHandle, + useDebugValueLabel, useLayoutEffect, useMemo, useReducer, @@ -98,7 +99,12 @@ if (enableHooks) { React.useCallback = useCallback; React.useContext = useContext; React.useEffect = useEffect; +<<<<<<< HEAD React.useImperativeHandle = useImperativeHandle; +======= + React.useImperativeMethods = useImperativeMethods; + React.useDebugValueLabel = useDebugValueLabel; +>>>>>>> Support custom values for custom hooks 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..6f15eb682fea1 100644 --- a/packages/react/src/ReactHooks.js +++ b/packages/react/src/ReactHooks.js @@ -110,3 +110,8 @@ export function useImperativeHandle( const dispatcher = resolveDispatcher(); return dispatcher.useImperativeHandle(ref, create, inputs); } + +export function useDebugValueLabel(valueLabel: string) { + const dispatcher = resolveDispatcher(); + return dispatcher.useDebugValueLabel(valueLabel); +} From 5d7b4bea8b8c8bbb1b8ea03a3faec072f01a15f4 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 10 Jan 2019 08:56:25 -0800 Subject: [PATCH 4/6] PR feedback: 1. Renamed useDebugValueLabel hook to useDebugValue 2. Wrapped useDebugValue internals in if-DEV so that it could be removed from production builds. --- .../react-debug-tools/src/ReactDebugHooks.js | 20 +++---- .../ReactHooksInspection-test.internal.js | 4 +- ...ooksInspectionIntegration-test.internal.js | 60 +++++++++---------- .../src/ReactFiberDispatcher.js | 4 +- .../react-reconciler/src/ReactFiberHooks.js | 2 +- packages/react/src/React.js | 8 +-- packages/react/src/ReactHooks.js | 8 ++- 7 files changed, 50 insertions(+), 56 deletions(-) diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js index 2d8f8647ad65c..fbff47bb722fe 100644 --- a/packages/react-debug-tools/src/ReactDebugHooks.js +++ b/packages/react-debug-tools/src/ReactDebugHooks.js @@ -55,7 +55,7 @@ function getPrimitiveStackCache(): Map> { Dispatcher.useLayoutEffect(() => {}); Dispatcher.useEffect(() => {}); Dispatcher.useImperativeHandle(undefined, () => null); - Dispatcher.useDebugValueLabel(null); + Dispatcher.useDebugValue(null); Dispatcher.useCallback(() => {}); Dispatcher.useMemo(() => null); } finally { @@ -181,9 +181,9 @@ function useImperativeHandle( }); } -function useDebugValueLabel(valueLabel: any) { +function useDebugValue(valueLabel: any) { hookLog.push({ - primitive: 'DebugValueLabel', + primitive: 'DebugValue', stackError: new Error(), value: valueLabel, }); @@ -214,12 +214,8 @@ const Dispatcher = { useCallback, useContext, useEffect, -<<<<<<< HEAD useImperativeHandle, -======= - useImperativeMethods, - useDebugValueLabel, ->>>>>>> Support custom values for custom hooks + useDebugValue, useLayoutEffect, useMemo, useReducer, @@ -418,19 +414,19 @@ function buildTree(rootStack, readHookLog): HooksTree { } // Associate custom hook values (useInpect() hook entries) with the correct hooks - rootChildren.forEach(hooksNode => rollupDebugValueLabels(hooksNode)); + rootChildren.forEach(hooksNode => rollupDebugValues(hooksNode)); return rootChildren; } -function rollupDebugValueLabels(hooksNode: HooksNode): void { +function rollupDebugValues(hooksNode: HooksNode): void { let useInpectHooksNodes: Array = []; hooksNode.subHooks = hooksNode.subHooks.filter(subHooksNode => { - if (subHooksNode.name === 'DebugValueLabel') { + if (subHooksNode.name === 'DebugValue') { useInpectHooksNodes.push(subHooksNode); return false; } else { - rollupDebugValueLabels(subHooksNode); + rollupDebugValues(subHooksNode); return true; } }); 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 47e8fc0a3b5ab..6b61fcf9a4c6e 100644 --- a/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.internal.js +++ b/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.internal.js @@ -41,7 +41,7 @@ describe('ReactHooksInspection', () => { it('should inspect a simple custom hook', () => { function useCustom(value) { let [state] = React.useState(value); - React.useDebugValueLabel('custom hook label'); + React.useDebugValue('custom hook label'); return state; } function Foo(props) { @@ -52,7 +52,7 @@ describe('ReactHooksInspection', () => { expect(tree).toEqual([ { name: 'Custom', - value: 'custom hook label', + value: __DEV__ ? 'custom hook label' : undefined, subHooks: [ { name: 'State', 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 a117b1e269b33..2ecb64a4e8255 100644 --- a/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.internal.js +++ b/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.internal.js @@ -212,11 +212,11 @@ describe('ReactHooksInspectionIntergration', () => { ]); }); - describe('useDebugValueLabel', () => { + describe('useDebugValue', () => { it('should support inspectable values for multiple custom hooks', () => { function useLabeledValue(label) { let [value] = React.useState(label); - React.useDebugValueLabel(`custom label ${label}`); + React.useDebugValue(`custom label ${label}`); return value; } function useAnonymous(label) { @@ -232,14 +232,11 @@ describe('ReactHooksInspectionIntergration', () => { } let renderer = ReactTestRenderer.create(); let childFiber = renderer.root.findByType(Example)._currentFiber(); - let tree = ReactDebugTools.inspectHooksOfFiber( - currentDispatcher, - childFiber, - ); + let tree = ReactDebugTools.inspectHooksOfFiber(childFiber); expect(tree).toEqual([ { name: 'LabeledValue', - value: 'custom label a', + value: __DEV__ ? 'custom label a' : undefined, subHooks: [{name: 'State', value: 'a', subHooks: []}], }, { @@ -254,7 +251,7 @@ describe('ReactHooksInspectionIntergration', () => { }, { name: 'LabeledValue', - value: 'custom label d', + value: __DEV__ ? 'custom label d' : undefined, subHooks: [{name: 'State', value: 'd', subHooks: []}], }, ]); @@ -262,10 +259,11 @@ describe('ReactHooksInspectionIntergration', () => { it('should support inspectable values for nested custom hooks', () => { function useInner() { - React.useDebugValueLabel('inner'); + React.useDebugValue('inner'); + React.useState(0); } function useOuter() { - React.useDebugValueLabel('outer'); + React.useDebugValue('outer'); useInner(); } function Example(props) { @@ -274,27 +272,32 @@ describe('ReactHooksInspectionIntergration', () => { } let renderer = ReactTestRenderer.create(); let childFiber = renderer.root.findByType(Example)._currentFiber(); - let tree = ReactDebugTools.inspectHooksOfFiber( - currentDispatcher, - childFiber, - ); + let tree = ReactDebugTools.inspectHooksOfFiber(childFiber); expect(tree).toEqual([ { name: 'Outer', - value: 'outer', - subHooks: [{name: 'Inner', value: 'inner', subHooks: []}], + 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.useDebugValueLabel('one'); - React.useDebugValueLabel('two'); - React.useDebugValueLabel('three'); + React.useDebugValue('one'); + React.useDebugValue('two'); + React.useDebugValue('three'); + React.useState(0); } function useSingleLabelCustom(value) { - React.useDebugValueLabel(`single ${value}`); + React.useDebugValue(`single ${value}`); + React.useState(0); } function Example(props) { useSingleLabelCustom('one'); @@ -304,25 +307,22 @@ describe('ReactHooksInspectionIntergration', () => { } let renderer = ReactTestRenderer.create(); let childFiber = renderer.root.findByType(Example)._currentFiber(); - let tree = ReactDebugTools.inspectHooksOfFiber( - currentDispatcher, - childFiber, - ); + let tree = ReactDebugTools.inspectHooksOfFiber(childFiber); expect(tree).toEqual([ { name: 'SingleLabelCustom', - value: 'single one', - subHooks: [], + value: __DEV__ ? 'single one' : undefined, + subHooks: [{name: 'State', value: 0, subHooks: []}], }, { name: 'MultiLabelCustom', - value: ['one', 'two', 'three'], - subHooks: [], + value: __DEV__ ? ['one', 'two', 'three'] : undefined, + subHooks: [{name: 'State', value: 0, subHooks: []}], }, { name: 'SingleLabelCustom', - value: 'single two', - subHooks: [], + value: __DEV__ ? 'single two' : undefined, + subHooks: [{name: 'State', value: 0, subHooks: []}], }, ]); }); diff --git a/packages/react-reconciler/src/ReactFiberDispatcher.js b/packages/react-reconciler/src/ReactFiberDispatcher.js index 806c543fd0e23..ed2ed3b2f315b 100644 --- a/packages/react-reconciler/src/ReactFiberDispatcher.js +++ b/packages/react-reconciler/src/ReactFiberDispatcher.js @@ -13,7 +13,7 @@ import { useContext, useEffect, useImperativeHandle, - useDebugValueLabel, + useDebugValue, useLayoutEffect, useMemo, useReducer, @@ -27,7 +27,7 @@ export const Dispatcher = { useContext, useEffect, useImperativeHandle, - useDebugValueLabel, + useDebugValue, useLayoutEffect, useMemo, useReducer, diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 71346a3b25cea..f2be782798fc1 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -588,7 +588,7 @@ export function useImperativeHandle( }, nextInputs); } -export function useDebugValueLabel(valueLabel: string): void { +export function useDebugValue(valueLabel: string): 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. diff --git a/packages/react/src/React.js b/packages/react/src/React.js index 11821bcb10c73..4b868feca9c88 100644 --- a/packages/react/src/React.js +++ b/packages/react/src/React.js @@ -33,7 +33,7 @@ import { useContext, useEffect, useImperativeHandle, - useDebugValueLabel, + useDebugValue, useLayoutEffect, useMemo, useReducer, @@ -99,12 +99,8 @@ if (enableHooks) { React.useCallback = useCallback; React.useContext = useContext; React.useEffect = useEffect; -<<<<<<< HEAD React.useImperativeHandle = useImperativeHandle; -======= - React.useImperativeMethods = useImperativeMethods; - React.useDebugValueLabel = useDebugValueLabel; ->>>>>>> Support custom values for custom hooks + 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 6f15eb682fea1..bd4f9c5dfc320 100644 --- a/packages/react/src/ReactHooks.js +++ b/packages/react/src/ReactHooks.js @@ -111,7 +111,9 @@ export function useImperativeHandle( return dispatcher.useImperativeHandle(ref, create, inputs); } -export function useDebugValueLabel(valueLabel: string) { - const dispatcher = resolveDispatcher(); - return dispatcher.useDebugValueLabel(valueLabel); +export function useDebugValue(valueLabel: string) { + if (__DEV__) { + const dispatcher = resolveDispatcher(); + return dispatcher.useDebugValue(valueLabel); + } } From 683907baf7b9886417459f32faef462cb3f5a12e Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 10 Jan 2019 15:31:36 -0800 Subject: [PATCH 5/6] 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 --- .../react-debug-tools/src/ReactDebugHooks.js | 49 ++++++++++++------- .../ReactHooksInspection-test.internal.js | 30 ++++++++++++ ...ooksInspectionIntegration-test.internal.js | 38 ++++++++++++-- .../react-reconciler/src/ReactFiberHooks.js | 7 ++- packages/react/src/ReactHooks.js | 4 +- 5 files changed, 104 insertions(+), 24 deletions(-) diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js index fbff47bb722fe..565a1ccfb74db 100644 --- a/packages/react-debug-tools/src/ReactDebugHooks.js +++ b/packages/react-debug-tools/src/ReactDebugHooks.js @@ -181,11 +181,11 @@ function useImperativeHandle( }); } -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.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 = []; + + 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 index 6b61fcf9a4c6e..d84bd82fb2780 100644 --- 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 index 2ecb64a4e8255..703db2af538d8 100644 --- 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(); + 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 () => { diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index f2be782798fc1..2f8d9a24a5658 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -588,10 +588,13 @@ export function useImperativeHandle( }, 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( diff --git a/packages/react/src/ReactHooks.js b/packages/react/src/ReactHooks.js index bd4f9c5dfc320..37f85cd5c3537 100644 --- a/packages/react/src/ReactHooks.js +++ b/packages/react/src/ReactHooks.js @@ -111,9 +111,9 @@ export function useImperativeHandle( 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); } } From fa8d3d193068c728b5afa0f6321fcc204a223e18 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 11 Jan 2019 09:07:18 -0800 Subject: [PATCH 6/6] Nitpick renamed a method --- .../react-debug-tools/src/ReactDebugHooks.js | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js index 565a1ccfb74db..1256c8fa8330f 100644 --- a/packages/react-debug-tools/src/ReactDebugHooks.js +++ b/packages/react-debug-tools/src/ReactDebugHooks.js @@ -414,16 +414,17 @@ function buildTree(rootStack, readHookLog): HooksTree { } // Associate custom hook values (useDebugValue() hook entries) with the correct hooks. - rollupDebugValues(rootChildren, null); + processDebugValues(rootChildren, null); return rootChildren; } -// 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( +// 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 { @@ -436,12 +437,12 @@ function rollupDebugValues( i--; debugValueHooksNodes.push(hooksNode); } else { - rollupDebugValues(hooksNode.subHooks, hooksNode); + processDebugValues(hooksNode.subHooks, hooksNode); } } - // Bubble debug value labels to their parent custom hook. - // If there is no parent hook, just ignore them. + // 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) {