Skip to content

Commit

Permalink
Always use includeHooksSource option
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sebmarkbage committed Feb 13, 2024
1 parent 32df74d commit 0e01326
Show file tree
Hide file tree
Showing 5 changed files with 1,822 additions and 990 deletions.
53 changes: 22 additions & 31 deletions packages/react-debug-tools/src/ReactDebugHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,7 @@ export type HooksNode = {
value: mixed,
subHooks: Array<HooksNode>,
debugInfo: null | ReactDebugInfo,
hookSource?: HookSource,
hookSource: null | HookSource,
};
export type HooksTree = Array<HooksNode>;

Expand Down Expand Up @@ -717,7 +717,6 @@ function parseCustomHookName(functionName: void | string): string {
function buildTree(
rootStack: any,
readHookLog: Array<HookLogEntry>,
includeHooksSource: boolean,
): HooksTree {
const rootChildren: Array<HooksNode> = [];
let prevStack = null;
Expand Down Expand Up @@ -761,16 +760,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);
Expand Down Expand Up @@ -801,26 +797,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);
}

Expand Down Expand Up @@ -898,7 +893,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.
Expand All @@ -924,7 +918,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) {
Expand Down Expand Up @@ -953,7 +947,6 @@ function inspectHooksOfForwardRef<Props, Ref>(
props: Props,
ref: Ref,
currentDispatcher: CurrentDispatcherRef,
includeHooksSource: boolean,
): HooksTree {
const previousDispatcher = currentDispatcher.current;
let readHookLog;
Expand All @@ -970,7 +963,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) {
Expand All @@ -991,7 +984,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.
Expand Down Expand Up @@ -1033,11 +1025,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;
Expand Down
Loading

0 comments on commit 0e01326

Please sign in to comment.