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.

Sign up for GitHub

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

DevTools: Rely on sourcemaps to compute hook name of built-in hooks in newer versions #28593

Merged
merged 4 commits into from
Apr 11, 2024
Merged

DevTools: Rely on sourcemaps to compute hook name of built-in hooks in newer versions #28593

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2a28644
DevTools: Rely on sourcemaps to compute hook name of built-in hooks
eps1lon Mar 13, 2024
81f4373
Fix "Unknown" on deep hook trees
eps1lon Mar 27, 2024
62d0cba
Fallback to primitive in case of emergency
eps1lon Mar 27, 2024
9688f14
Fix parsing for minified prod bundles
eps1lon Apr 3, 2024
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
106 changes: 71 additions & 35 deletions packages/react-debug-tools/src/ReactDebugHooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ type HookLogEntry = {
stackError: Error,
value: mixed,
debugInfo: ReactDebugInfo | null,
dispatcherHookName: string,
};

let hookLog: Array<HookLogEntry> = [];
Expand Down Expand Up @@ -131,6 +132,8 @@ function getPrimitiveStackCache(): Map<string, Array<any>> {
);
} catch (x) {}
}

Dispatcher.useId();
} finally {
readHookLog = hookLog;
hookLog = [];
Expand Down Expand Up @@ -207,6 +210,7 @@ function use<T>(usable: Usable<T>): T {
value: fulfilledValue,
debugInfo:
thenable._debugInfo === undefined ? null : thenable._debugInfo,
dispatcherHookName: 'Use',
});
return fulfilledValue;
}
Expand All @@ -224,6 +228,7 @@ function use<T>(usable: Usable<T>): T {
value: thenable,
debugInfo:
thenable._debugInfo === undefined ? null : thenable._debugInfo,
dispatcherHookName: 'Use',
});
throw SuspenseException;
} else if (usable.$$typeof === REACT_CONTEXT_TYPE) {
Expand All @@ -236,6 +241,7 @@ function use<T>(usable: Usable<T>): T {
stackError: new Error(),
value,
debugInfo: null,
dispatcherHookName: 'Use',
});

return value;
Expand All @@ -254,6 +260,7 @@ function useContext<T>(context: ReactContext<T>): T {
stackError: new Error(),
value: value,
debugInfo: null,
dispatcherHookName: 'Context',
});
return value;
}
Expand All @@ -275,6 +282,7 @@ function useState<S>(
stackError: new Error(),
value: state,
debugInfo: null,
dispatcherHookName: 'State',
});
return [state, (action: BasicStateAction<S>) => {}];
}
Expand All @@ -297,6 +305,7 @@ function useReducer<S, I, A>(
stackError: new Error(),
value: state,
debugInfo: null,
dispatcherHookName: 'Reducer',
});
return [state, (action: A) => {}];
}
Expand All @@ -310,6 +319,7 @@ function useRef<T>(initialValue: T): {current: T} {
stackError: new Error(),
value: ref.current,
debugInfo: null,
dispatcherHookName: 'Ref',
});
return ref;
}
Expand All @@ -322,6 +332,7 @@ function useCacheRefresh(): () => void {
stackError: new Error(),
value: hook !== null ? hook.memoizedState : function refresh() {},
debugInfo: null,
dispatcherHookName: 'CacheRefresh',
});
return () => {};
}
Expand All @@ -337,6 +348,7 @@ function useLayoutEffect(
stackError: new Error(),
value: create,
debugInfo: null,
dispatcherHookName: 'LayoutEffect',
});
}

Expand All @@ -351,6 +363,7 @@ function useInsertionEffect(
stackError: new Error(),
value: create,
debugInfo: null,
dispatcherHookName: 'InsertionEffect',
});
}

Expand All @@ -365,6 +378,7 @@ function useEffect(
stackError: new Error(),
value: create,
debugInfo: null,
dispatcherHookName: 'Effect',
});
}

Expand All @@ -388,6 +402,7 @@ function useImperativeHandle<T>(
stackError: new Error(),
value: instance,
debugInfo: null,
dispatcherHookName: 'ImperativeHandle',
});
}

Expand All @@ -398,6 +413,7 @@ function useDebugValue(value: any, formatterFn: ?(value: any) => any) {
stackError: new Error(),
value: typeof formatterFn === 'function' ? formatterFn(value) : value,
debugInfo: null,
dispatcherHookName: 'DebugValue',
});
}

Expand All @@ -409,6 +425,7 @@ function useCallback<T>(callback: T, inputs: Array<mixed> | void | null): T {
stackError: new Error(),
value: hook !== null ? hook.memoizedState[0] : callback,
debugInfo: null,
dispatcherHookName: 'Callback',
});
return callback;
}
Expand All @@ -425,6 +442,7 @@ function useMemo<T>(
stackError: new Error(),
value,
debugInfo: null,
dispatcherHookName: 'Memo',
});
return value;
}
Expand All @@ -446,6 +464,7 @@ function useSyncExternalStore<T>(
stackError: new Error(),
value,
debugInfo: null,
dispatcherHookName: 'SyncExternalStore',
});
return value;
}
Expand All @@ -468,6 +487,7 @@ function useTransition(): [
stackError: new Error(),
value: isPending,
debugInfo: null,
dispatcherHookName: 'Transition',
});
return [isPending, () => {}];
}
Expand All @@ -481,6 +501,7 @@ function useDeferredValue<T>(value: T, initialValue?: T): T {
stackError: new Error(),
value: prevValue,
debugInfo: null,
dispatcherHookName: 'DeferredValue',
});
return prevValue;
}
Expand All @@ -494,6 +515,7 @@ function useId(): string {
stackError: new Error(),
value: id,
debugInfo: null,
dispatcherHookName: 'Id',
});
return id;
}
Expand Down Expand Up @@ -544,6 +566,7 @@ function useOptimistic<S, A>(
stackError: new Error(),
value: state,
debugInfo: null,
dispatcherHookName: 'Optimistic',
});
return [state, (action: A) => {}];
}
Expand Down Expand Up @@ -603,6 +626,7 @@ function useFormState<S, P>(
stackError: stackError,
value: value,
debugInfo: debugInfo,
dispatcherHookName: 'FormState',
});

if (error !== null) {
Expand Down Expand Up @@ -672,6 +696,7 @@ function useActionState<S, P>(
stackError: stackError,
value: value,
debugInfo: debugInfo,
dispatcherHookName: 'ActionState',
});

if (error !== null) {
Expand Down Expand Up @@ -759,8 +784,7 @@ export type HooksTree = Array<HooksNode>;
// of a hook call. A simple way to demonstrate this is wrapping `new Error()`
// in a wrapper constructor like a polyfill. That'll add an extra frame.
// Similar things can happen with the call to the dispatcher. The top frame
// may not be the primitive. Likewise the primitive can have fewer stack frames
// such as when a call to useState got inlined to use dispatcher.useState.
// may not be the primitive.
Comment on lines -762 to +787
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't see how this can happen since the dispatcher is private. Without the "wrapper -> dispatcher" assumption I'm out of ideas on how to enable arbitrary wrapper names like useHostTransitionStatus is trying to enable.

It also sounds like it contradicts the current implementation of findPrimitiveIndex where we always assumed there are two stack frames for the dispatcher method and the public method right next to each other.

//
// We also can't assume that the last frame of the root call is the same
// frame as the last frame of the hook call because long stack traces can be
Expand Down Expand Up @@ -810,27 +834,8 @@ function findCommonAncestorIndex(rootStack: any, hookStack: any) {
return -1;
}

function isReactWrapper(functionName: any, primitiveName: string) {
if (!functionName) {
return false;
}
switch (primitiveName) {
case 'Context':
case 'Context (use)':
case 'Promise':
case 'Unresolved':
if (functionName.endsWith('use')) {
return true;
}
}
const expectedPrimitiveName = 'use' + primitiveName;
if (functionName.length < expectedPrimitiveName.length) {
return false;
}
return (
functionName.lastIndexOf(expectedPrimitiveName) ===
functionName.length - expectedPrimitiveName.length
);
function isReactWrapper(functionName: any, wrapperName: string) {
return parseHookName(functionName) === wrapperName;
}

function findPrimitiveIndex(hookStack: any, hook: HookLogEntry) {
Expand All @@ -841,17 +846,18 @@ function findPrimitiveIndex(hookStack: any, hook: HookLogEntry) {
}
for (let i = 0; i < primitiveStack.length && i < hookStack.length; i++) {
if (primitiveStack[i].source !== hookStack[i].source) {
// If the next two frames are functions called `useX` then we assume that they're part of the
// wrappers that the React packager or other packages adds around the dispatcher.
// If the next frame is a method from the dispatcher, we
// assume that the next frame after that is the actual public API call.
// This prohibits nesting dispatcher calls in hooks.
if (
i < hookStack.length - 1 &&
isReactWrapper(hookStack[i].functionName, hook.primitive)
isReactWrapper(hookStack[i].functionName, hook.dispatcherHookName)
) {
i++;
}
if (
i < hookStack.length - 1 &&
isReactWrapper(hookStack[i].functionName, hook.primitive)
isReactWrapper(hookStack[i].functionName, hook.dispatcherHookName)
) {
i++;
}
Expand All @@ -872,21 +878,41 @@ function parseTrimmedStack(rootStack: any, hook: HookLogEntry) {
primitiveIndex === -1 ||
rootIndex - primitiveIndex < 2
) {
// Something went wrong. Give up.
return null;
if (primitiveIndex === -1) {
// Something went wrong. Give up.
return [null, null];
} else {
return [hookStack[primitiveIndex - 1], null];
}
}
return hookStack.slice(primitiveIndex, rootIndex - 1);
return [
hookStack[primitiveIndex - 1],
hookStack.slice(primitiveIndex, rootIndex - 1),
];
Comment on lines +888 to +891
Copy link
Contributor

Choose a reason for hiding this comment

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

Where did this come from? Looking at the code, you are trying to use the functionName from the primitive frame, but we didn't have this before, is it a regression for some cases?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's the core of this change. Instead of computing the hook name from the hook log, we compute it from the parent of the dispatcher call since we always have PublicReactPackage.useSomeHook calling into dispatcher.useSomePotentiallyDifferentlyNamedHook.

This doesn't really do much in this commit since the name of the public hook always matches the dispatcher call. But for useFormStatus (which we support in #28413), we have ReactDOM.useFormStatus calling into useHostTransitionStatus. Since useHostTransitionStatus can be called by a hook from a different renderer that isn't called useFormStatus (see #26719), we have to compute the name for devtools from the parent frame i.e. hookStack[primitiveIndex - 1].

But we can only use the the frame from the public method with sourcemaps because public hooks get mangled e.g. exports.useFormStatus = ia. The stack frame of exports.useFormstatus() would read at Object.ia [as useFormStatus] which is what I saw in #28413 (comment) where sourcemaps were broken. When sourcemaps are not broken i.e. outside of Jest, we get the correct stackframe reading at Object.useFormStatus which we can parse and extract the expected hook name FormStatus.

}

function parseCustomHookName(functionName: void | string): string {
function parseHookName(functionName: void | string): string {
if (!functionName) {
return '';
}
let startIndex = functionName.lastIndexOf('.');
let startIndex = functionName.lastIndexOf('[as ');

if (startIndex !== -1) {
// Workaround for sourcemaps in Jest and Chrome.
// In `node --enable-source-maps`, we don't see "Object.useHostTransitionStatus [as useFormStatus]" but "Object.useFormStatus"
// "Object.useHostTransitionStatus [as useFormStatus]" -> "useFormStatus"
return parseHookName(functionName.slice(startIndex + '[as '.length, -1));
}
startIndex = functionName.lastIndexOf('.');
if (startIndex === -1) {
startIndex = 0;
} else {
startIndex += 1;
}
if (functionName.slice(startIndex, startIndex + 3) === 'use') {
if (functionName.length - startIndex === 3) {
return 'Use';
}
startIndex += 3;
}
return functionName.slice(startIndex);
Expand All @@ -903,7 +929,17 @@ function buildTree(
const stackOfChildren = [];
for (let i = 0; i < readHookLog.length; i++) {
const hook = readHookLog[i];
const stack = parseTrimmedStack(rootStack, hook);
const parseResult = parseTrimmedStack(rootStack, hook);
const primitiveFrame = parseResult[0];
const stack = parseResult[1];
let displayName = hook.displayName;
if (displayName === null && primitiveFrame !== null) {
displayName =
parseHookName(primitiveFrame.functionName) ||
// Older versions of React do not have sourcemaps.
// In those versions there was always a 1:1 mapping between wrapper and dispatcher method.
parseHookName(hook.dispatcherHookName);
}
if (stack !== null) {
// Note: The indices 0 <= n < length-1 will contain the names.
// The indices 1 <= n < length will contain the source locations.
Expand Down Expand Up @@ -934,7 +970,7 @@ function buildTree(
const levelChild: HooksNode = {
id: null,
isStateEditable: false,
name: parseCustomHookName(stack[j - 1].functionName),
name: parseHookName(stack[j - 1].functionName),
value: undefined,
subHooks: children,
debugInfo: null,
Expand All @@ -952,7 +988,7 @@ function buildTree(
}
prevStack = stack;
}
const {displayName, primitive, debugInfo} = hook;
const {primitive, debugInfo} = hook;

// For now, the "id" of stateful hooks is just the stateful hook index.
// Custom hooks have no ids, nor do non-stateful native hooks (e.g. Context, DebugValue).
Expand Down
4 changes: 2 additions & 2 deletions packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,7 @@ describe('ReactHooksInspection', () => {
},
"id": null,
"isStateEditable": false,
"name": "Promise",
"name": "Use",
"subHooks": [],
"value": "world",
},
Expand Down Expand Up @@ -601,7 +601,7 @@ describe('ReactHooksInspection', () => {
},
"id": null,
"isStateEditable": false,
"name": "Unresolved",
"name": "Use",
"subHooks": [],
"value": Any<Promise>,
}
Expand Down