[Debug Tools] Always use includeHooksSource option (#28309)

This option was added defensively but it's not needed. There's no cost to including it always. I suspect this optional was added mainly to avoid needing to update tests. That's not a reason to have an unnecessary public API though. We have a praxis for dealing with source location in tests to avoid them failing tests. I also ported them to inline snapshots so that additions to the protocol isn't such a pain.
facebook · Feb 14, 2024 · f0e808e · f0e808e
1 parent dc31781
commit f0e808e
Show file tree

Hide file tree

Showing 5 changed files with 2,096 additions and 979 deletions.
diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js
@@ -569,7 +569,7 @@ export type HooksNode = {
   value: mixed,
   subHooks: Array<HooksNode>,
   debugInfo: null | ReactDebugInfo,
-  hookSource?: HookSource,
+  hookSource: null | HookSource,
 };
 export type HooksTree = Array<HooksNode>;
 
@@ -716,7 +716,6 @@ function parseCustomHookName(functionName: void | string): string {
 function buildTree(
   rootStack: any,
   readHookLog: Array<HookLogEntry>,
-  includeHooksSource: boolean,
 ): HooksTree {
   const rootChildren: Array<HooksNode> = [];
   let prevStack = null;
@@ -760,16 +759,13 @@ function buildTree(
           value: undefined,
           subHooks: children,
           debugInfo: null,
-        };
-
-        if (includeHooksSource) {
-          levelChild.hookSource = {
+          hookSource: {
             lineNumber: stackFrame.lineNumber,
             columnNumber: stackFrame.columnNumber,
             functionName: stackFrame.functionName,
             fileName: stackFrame.fileName,
-          };
-        }
+          },
+        };
 
         levelChildren.push(levelChild);
         stackOfChildren.push(levelChildren);
@@ -800,26 +796,25 @@ function buildTree(
       value: hook.value,
       subHooks: [],
       debugInfo: debugInfo,
+      hookSource: null,
     };
 
-    if (includeHooksSource) {
-      const hookSource: HookSource = {
-        lineNumber: null,
-        functionName: null,
-        fileName: null,
-        columnNumber: null,
-      };
-      if (stack && stack.length >= 1) {
-        const stackFrame = stack[0];
-        hookSource.lineNumber = stackFrame.lineNumber;
-        hookSource.functionName = stackFrame.functionName;
-        hookSource.fileName = stackFrame.fileName;
-        hookSource.columnNumber = stackFrame.columnNumber;
-      }
-
-      levelChild.hookSource = hookSource;
+    const hookSource: HookSource = {
+      lineNumber: null,
+      functionName: null,
+      fileName: null,
+      columnNumber: null,
+    };
+    if (stack && stack.length >= 1) {
+      const stackFrame = stack[0];
+      hookSource.lineNumber = stackFrame.lineNumber;
+      hookSource.functionName = stackFrame.functionName;
+      hookSource.fileName = stackFrame.fileName;
+      hookSource.columnNumber = stackFrame.columnNumber;
     }
 
+    levelChild.hookSource = hookSource;
+
     levelChildren.push(levelChild);
   }
 
@@ -897,7 +892,6 @@ export function inspectHooks<Props>(
   renderFunction: Props => React$Node,
   props: Props,
   currentDispatcher: ?CurrentDispatcherRef,
-  includeHooksSource: boolean = false,
 ): HooksTree {
   // DevTools will pass the current renderer's injected dispatcher.
   // Other apps might compile debug hooks as part of their app though.
@@ -923,7 +917,7 @@ export function inspectHooks<Props>(
     currentDispatcher.current = previousDispatcher;
   }
   const rootStack = ErrorStackParser.parse(ancestorStackError);
-  return buildTree(rootStack, readHookLog, includeHooksSource);
+  return buildTree(rootStack, readHookLog);
 }
 
 function setupContexts(contextMap: Map<ReactContext<any>, any>, fiber: Fiber) {
@@ -955,7 +949,6 @@ function inspectHooksOfForwardRef<Props, Ref>(
   props: Props,
   ref: Ref,
   currentDispatcher: CurrentDispatcherRef,
-  includeHooksSource: boolean,
 ): HooksTree {
   const previousDispatcher = currentDispatcher.current;
   let readHookLog;
@@ -972,7 +965,7 @@ function inspectHooksOfForwardRef<Props, Ref>(
     currentDispatcher.current = previousDispatcher;
   }
   const rootStack = ErrorStackParser.parse(ancestorStackError);
-  return buildTree(rootStack, readHookLog, includeHooksSource);
+  return buildTree(rootStack, readHookLog);
 }
 
 function resolveDefaultProps(Component: any, baseProps: any) {
@@ -993,7 +986,6 @@ function resolveDefaultProps(Component: any, baseProps: any) {
 export function inspectHooksOfFiber(
   fiber: Fiber,
   currentDispatcher: ?CurrentDispatcherRef,
-  includeHooksSource: boolean = false,
 ): HooksTree {
   // DevTools will pass the current renderer's injected dispatcher.
   // Other apps might compile debug hooks as part of their app though.
@@ -1035,11 +1027,10 @@ export function inspectHooksOfFiber(
         props,
         fiber.ref,
         currentDispatcher,
-        includeHooksSource,
       );
     }
 
-    return inspectHooks(type, props, currentDispatcher, includeHooksSource);
+    return inspectHooks(type, props, currentDispatcher);
   } finally {
     currentFiber = null;
     currentHook = null;