Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support configurable labels for custom hooks #14559

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 49 additions & 1 deletion packages/react-debug-tools/src/ReactDebugHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ function getPrimitiveStackCache(): Map<string, Array<any>> {
Dispatcher.useLayoutEffect(() => {});
Dispatcher.useEffect(() => {});
Dispatcher.useImperativeHandle(undefined, () => null);
Dispatcher.useDebugValue(null);
Dispatcher.useCallback(() => {});
Dispatcher.useMemo(() => null);
} finally {
Expand Down Expand Up @@ -180,6 +181,14 @@ function useImperativeHandle<T>(
});
}

function useDebugValue(value: any, formatterFn: ?(value: any) => any) {
hookLog.push({
primitive: 'DebugValue',
stackError: new Error(),
value: typeof formatterFn === 'function' ? formatterFn(value) : value,
});
}

function useCallback<T>(callback: T, inputs: Array<mixed> | void | null): T {
let hook = nextHook();
hookLog.push({
Expand All @@ -206,6 +215,7 @@ const Dispatcher = {
useContext,
useEffect,
useImperativeHandle,
useDebugValue,
useLayoutEffect,
useMemo,
useReducer,
Expand Down Expand Up @@ -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);
Expand All @@ -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<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 {
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<Props>(
renderFunction: Props => React$Node,
props: Props,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -51,7 +52,7 @@ describe('ReactHooksInspection', () => {
expect(tree).toEqual([
{
name: 'Custom',
value: undefined,
value: __DEV__ ? 'custom hook label' : undefined,
subHooks: [
{
name: 'State',
Expand Down Expand Up @@ -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}],
},
]);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,154 @@ describe('ReactHooksInspectionIntergration', () => {
]);
});

describe('useDebugValue', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also have a test for useDebugValue on top level of component? Outside a Hook.

Copy link
Collaborator

@gaearon gaearon Jan 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also should we add tests for updates? To verify inspectHooksOfFiber can find latest version. (In case it matters — I don't understand this enough to tell yet.)

Ah nvm, I see you pass current fiber explicitly.

Copy link
Contributor Author

@bvaughn bvaughn Jan 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I can add a test for that. I'm not sure what the expected behavior should be. A warning? Just ignore it entirely?

For now I'll just remove them.

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(<Example />);
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(<Example />);
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(<Example />);
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(<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 () => {
let Suspense = React.Suspense;

Expand Down
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactFiberDispatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
useContext,
useEffect,
useImperativeHandle,
useDebugValue,
useLayoutEffect,
useMemo,
useReducer,
Expand All @@ -26,6 +27,7 @@ export const Dispatcher = {
useContext,
useEffect,
useImperativeHandle,
useDebugValue,
useLayoutEffect,
useMemo,
useReducer,
Expand Down
9 changes: 9 additions & 0 deletions packages/react-reconciler/src/ReactFiberHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,15 @@ export function useImperativeHandle<T>(
}, 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<T>(
callback: T,
inputs: Array<mixed> | void | null,
Expand Down
2 changes: 2 additions & 0 deletions packages/react/src/React.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
useContext,
useEffect,
useImperativeHandle,
useDebugValue,
useLayoutEffect,
useMemo,
useReducer,
Expand Down Expand Up @@ -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;
Expand Down
7 changes: 7 additions & 0 deletions packages/react/src/ReactHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,3 +110,10 @@ export function useImperativeHandle<T>(
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);
}
}