Skip to content

Commit 4a2d812

Browse files
author
Brian Vaughn
committed
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
1 parent 5d7b4be commit 4a2d812

File tree

5 files changed

+102
-24
lines changed

5 files changed

+102
-24
lines changed

packages/react-debug-tools/src/ReactDebugHooks.js

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -181,11 +181,11 @@ function useImperativeHandle<T>(
181181
});
182182
}
183183

184-
function useDebugValue(valueLabel: any) {
184+
function useDebugValue(value: any, formatterFn: ?(value: any) => any) {
185185
hookLog.push({
186186
primitive: 'DebugValue',
187187
stackError: new Error(),
188-
value: valueLabel,
188+
value: typeof formatterFn === 'function' ? formatterFn(value) : value,
189189
});
190190
}
191191

@@ -413,27 +413,42 @@ function buildTree(rootStack, readHookLog): HooksTree {
413413
});
414414
}
415415

416-
// Associate custom hook values (useInpect() hook entries) with the correct hooks
417-
rootChildren.forEach(hooksNode => rollupDebugValues(hooksNode));
416+
// Associate custom hook values (useDebugValue() hook entries) with the correct hooks.
417+
rollupDebugValues(rootChildren, null);
418418

419419
return rootChildren;
420420
}
421421

422-
function rollupDebugValues(hooksNode: HooksNode): void {
423-
let useInpectHooksNodes: Array<HooksNode> = [];
424-
hooksNode.subHooks = hooksNode.subHooks.filter(subHooksNode => {
425-
if (subHooksNode.name === 'DebugValue') {
426-
useInpectHooksNodes.push(subHooksNode);
427-
return false;
422+
// Custom hooks support user-configurable labels (via the useDebugValue() hook).
423+
// That hook adds the user-provided values to the hooks tree.
424+
// This method removes those values (so they don't appear in DevTools),
425+
// and bubbles them up to the "value" attribute of their parent custom hook.
426+
function rollupDebugValues(
427+
hooksTree: HooksTree,
428+
parentHooksNode: HooksNode | null,
429+
): void {
430+
let debugValueHooksNodes: Array<HooksNode> = [];
431+
432+
for (let i = 0; i < hooksTree.length; i++) {
433+
const hooksNode = hooksTree[i];
434+
if (hooksNode.name === 'DebugValue' && hooksNode.subHooks.length === 0) {
435+
hooksTree.splice(i, 1);
436+
i--;
437+
debugValueHooksNodes.push(hooksNode);
428438
} else {
429-
rollupDebugValues(subHooksNode);
430-
return true;
439+
rollupDebugValues(hooksNode.subHooks, hooksNode);
440+
}
441+
}
442+
443+
// Bubble debug value labels to their parent custom hook.
444+
// If there is no parent hook, just ignore them.
445+
// (We may warn about this in the future.)
446+
if (parentHooksNode !== null) {
447+
if (debugValueHooksNodes.length === 1) {
448+
parentHooksNode.value = debugValueHooksNodes[0].value;
449+
} else if (debugValueHooksNodes.length > 1) {
450+
parentHooksNode.value = debugValueHooksNodes.map(({value}) => value);
431451
}
432-
});
433-
if (useInpectHooksNodes.length === 1) {
434-
hooksNode.value = useInpectHooksNodes[0].value;
435-
} else if (useInpectHooksNodes.length > 1) {
436-
hooksNode.value = useInpectHooksNodes.map(({value}) => value);
437452
}
438453
}
439454

packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.internal.js

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,4 +250,33 @@ describe('ReactHooksInspection', () => {
250250
expect(setterCalls[0]).not.toBe(initial);
251251
expect(setterCalls[1]).toBe(initial);
252252
});
253+
254+
describe('useDebugValue', () => {
255+
it('should be ignored when called outside of a custom hook', () => {
256+
function Foo(props) {
257+
React.useDebugValue('this is invalid');
258+
return null;
259+
}
260+
let tree = ReactDebugTools.inspectHooks(Foo, {});
261+
expect(tree).toHaveLength(0);
262+
});
263+
264+
it('should support an optional formatter function param', () => {
265+
function useCustom() {
266+
React.useDebugValue({bar: 123}, object => `bar:${object.bar}`);
267+
}
268+
function Foo(props) {
269+
useCustom();
270+
return null;
271+
}
272+
let tree = ReactDebugTools.inspectHooks(Foo, {});
273+
expect(tree).toEqual([
274+
{
275+
name: 'Custom',
276+
value: 'bar:123',
277+
subHooks: [],
278+
},
279+
]);
280+
});
281+
});
253282
});

packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.internal.js

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ describe('ReactHooksInspectionIntergration', () => {
223223
let [value] = React.useState(label);
224224
return value;
225225
}
226-
function Example(props) {
226+
function Example() {
227227
useLabeledValue('a');
228228
React.useState('b');
229229
useAnonymous('c');
@@ -266,7 +266,7 @@ describe('ReactHooksInspectionIntergration', () => {
266266
React.useDebugValue('outer');
267267
useInner();
268268
}
269-
function Example(props) {
269+
function Example() {
270270
useOuter();
271271
return null;
272272
}
@@ -299,7 +299,7 @@ describe('ReactHooksInspectionIntergration', () => {
299299
React.useDebugValue(`single ${value}`);
300300
React.useState(0);
301301
}
302-
function Example(props) {
302+
function Example() {
303303
useSingleLabelCustom('one');
304304
useMultiLabelCustom();
305305
useSingleLabelCustom('two');
@@ -326,6 +326,37 @@ describe('ReactHooksInspectionIntergration', () => {
326326
},
327327
]);
328328
});
329+
330+
it('should ignore useDebugValue() made outside of a custom hook', () => {
331+
function Example() {
332+
React.useDebugValue('this is invalid');
333+
return null;
334+
}
335+
let renderer = ReactTestRenderer.create(<Example />);
336+
let childFiber = renderer.root.findByType(Example)._currentFiber();
337+
let tree = ReactDebugTools.inspectHooksOfFiber(childFiber);
338+
expect(tree).toHaveLength(0);
339+
});
340+
341+
it('should support an optional formatter function param', () => {
342+
function useCustom() {
343+
React.useDebugValue({bar: 123}, object => `bar:${object.bar}`);
344+
}
345+
function Example() {
346+
useCustom();
347+
return null;
348+
}
349+
let renderer = ReactTestRenderer.create(<Example />);
350+
let childFiber = renderer.root.findByType(Example)._currentFiber();
351+
let tree = ReactDebugTools.inspectHooksOfFiber(childFiber);
352+
expect(tree).toEqual([
353+
{
354+
name: 'Custom',
355+
value: 'bar:123',
356+
subHooks: [],
357+
},
358+
]);
359+
});
329360
});
330361

331362
it('should support defaultProps and lazy', async () => {

packages/react-reconciler/src/ReactFiberHooks.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -588,10 +588,13 @@ export function useImperativeHandle<T>(
588588
}, nextInputs);
589589
}
590590

591-
export function useDebugValue(valueLabel: string): void {
591+
export function useDebugValue(
592+
value: any,
593+
formatterFn: ?(value: any) => any,
594+
): void {
592595
// This hook is normally a no-op.
593596
// The react-debug-hooks package injects its own implementation
594-
// so that e.g. DevTools can display customhook values.
597+
// so that e.g. DevTools can display custom hook values.
595598
}
596599

597600
export function useCallback<T>(

packages/react/src/ReactHooks.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,9 @@ export function useImperativeHandle<T>(
111111
return dispatcher.useImperativeHandle(ref, create, inputs);
112112
}
113113

114-
export function useDebugValue(valueLabel: string) {
114+
export function useDebugValue(value: any, formatterFn: ?(value: any) => any) {
115115
if (__DEV__) {
116116
const dispatcher = resolveDispatcher();
117-
return dispatcher.useDebugValue(valueLabel);
117+
return dispatcher.useDebugValue(value, formatterFn);
118118
}
119119
}

0 commit comments

Comments
 (0)