Skip to content

Commit

Permalink
PR feedback:
Browse files Browse the repository at this point in the history 
1. Renamed useDebugValueLabel hook to useDebugValue
2. Wrapped useDebugValue internals in if-DEV so that it could be removed from production builds.
  • Loading branch information
Brian Vaughn committed Jan 10, 2019
1 parent 9d30fb2 commit 5d7b4be
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 56 deletions.
20 changes: 8 additions & 12 deletions packages/react-debug-tools/src/ReactDebugHooks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ function getPrimitiveStackCache(): Map<string, Array<any>> {
Dispatcher.useLayoutEffect(() => {});
Dispatcher.useEffect(() => {});
Dispatcher.useImperativeHandle(undefined, () => null);
Dispatcher.useDebugValueLabel(null);
Dispatcher.useDebugValue(null);
Dispatcher.useCallback(() => {});
Dispatcher.useMemo(() => null);
} finally {
Expand Down Expand Up @@ -181,9 +181,9 @@ function useImperativeHandle<T>(
});
}

function useDebugValueLabel(valueLabel: any) {
function useDebugValue(valueLabel: any) {
hookLog.push({
primitive: 'DebugValueLabel',
primitive: 'DebugValue',
stackError: new Error(),
value: valueLabel,
});
Expand Down Expand Up @@ -214,12 +214,8 @@ const Dispatcher = {
useCallback,
useContext,
useEffect,
<<<<<<< HEAD
useImperativeHandle,
=======
useImperativeMethods,
useDebugValueLabel,
>>>>>>> Support custom values for custom hooks
useDebugValue,
useLayoutEffect,
useMemo,
useReducer,
Expand Down Expand Up @@ -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> = [];
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;
}
});
Expand Down
4 changes: 2 additions & 2 deletions packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -52,7 +52,7 @@ describe('ReactHooksInspection', () => {
expect(tree).toEqual([
{
name: 'Custom',
value: 'custom hook label',
value: __DEV__ ? 'custom hook label' : undefined,
subHooks: [
{
name: 'State',
Expand Down
60 changes: 30 additions & 30 deletions packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -232,14 +232,11 @@ describe('ReactHooksInspectionIntergration', () => {
}
let renderer = ReactTestRenderer.create(<Example />);
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: []}],
},
{
Expand All @@ -254,18 +251,19 @@ describe('ReactHooksInspectionIntergration', () => {
},
{
name: 'LabeledValue',
value: 'custom label d',
value: __DEV__ ? 'custom label d' : undefined,
subHooks: [{name: 'State', value: 'd', subHooks: []}],
},
]);
});

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) {
Expand All @@ -274,27 +272,32 @@ describe('ReactHooksInspectionIntergration', () => {
}
let renderer = ReactTestRenderer.create(<Example />);
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');
Expand All @@ -304,25 +307,22 @@ describe('ReactHooksInspectionIntergration', () => {
}
let renderer = ReactTestRenderer.create(<Example />);
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: []}],
},
]);
});
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberDispatcher.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
useContext,
useEffect,
useImperativeHandle,
useDebugValueLabel,
useDebugValue,
useLayoutEffect,
useMemo,
useReducer,
Expand All @@ -27,7 +27,7 @@ export const Dispatcher = {
useContext,
useEffect,
useImperativeHandle,
useDebugValueLabel,
useDebugValue,
useLayoutEffect,
useMemo,
useReducer,
Expand Down
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiberHooks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,7 @@ export function useImperativeHandle<T>(
}, 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.
Expand Down
8 changes: 2 additions & 6 deletions packages/react/src/React.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import {
useContext,
useEffect,
useImperativeHandle,
useDebugValueLabel,
useDebugValue,
useLayoutEffect,
useMemo,
useReducer,
Expand Down Expand Up @@ -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;
Expand Down
8 changes: 5 additions & 3 deletions packages/react/src/ReactHooks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,9 @@ export function useImperativeHandle<T>(
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);
}
}

0 comments on commit 5d7b4be

Please sign in to comment.