Add Debug Tools Package for Introspection of Hooks #14085

sebmarkbage · 2018-11-03T03:57:22Z

Adds introspection of primitive and custom hooks in a single function component. If you feed it a standalone component it just gives you the initial/default values as the current value of each primitive. If you pass it a Fiber, it gives you the current values by reading the state from the Fiber.

Ideally we want this in DevTools but it requires so much implementation details that I don't think we'll want to maintain this with multi-version support. We'll probably have to move to snapshot versions of this package in DevTools, or just snapshoting all of DevTools. It's probably useful debug info for others too. So I decided to just make this a new separate package.

The algorithm goes something like this:

If running in Fiber mode, set up contexts of all parent providers and set the current hook to the first one of the Fiber. If running in standalone mode, skip this step.
Compute the stack trace from the root of the inspection function.
Set up a custom Dispatcher.
Compute the stack traces to all primitive hooks, using the custom Dispatcher, when used outside a component.
Execute the render function of the inspected component. This will call all hooks in order since they have to be unconditional. It's safe because the render function as no side-effects on its own.
Inside each primitive hook in the custom Dispatcher, log each primitive, it's current stateful value, and the stack trace at the time it was invoked. T
his will give us a list of all primitive hooks and their stacks.
For each primitive hook in the log, process the stack trace by removing any shared ancestors with the root of the inspection function. This ensures that the bottom of the stack is the render function. Also, for that particular primitive, remove the top of the stack that is shared with the stack when that primitive is called stand alone from a function component. This removes the call to the primitive itself. If the top most function call is called "useState" or whatever the primitive is called, remove that too since it's the React package wrapper around the dispatcher. The result should be a stack of only the custom hooks.
Loop through the list of primitive hooks, use the stack of custom hooks to form a tree structure. Shared parent paths means that they go into the same subnode.
...
Profit.

Currently custom hooks doesn't have a "value" associated with them so you can only introspect the current values of primitives. In a follow up, we can add a DEV only API to share more information from custom hooks with the devtools. E.g. useInspect(value)

sizebot · 2018-11-03T04:04:13Z

Details of bundled changes.

Comparing: 8eca0ef...7006c4b

scheduler

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
scheduler.development.js	n/a	n/a	0 B	19.17 KB	0 B	5.74 KB	UMD_DEV
scheduler.production.min.js	n/a	n/a	0 B	3.16 KB	0 B	1.53 KB	UMD_PROD

react-debug-tools

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
react-debug-tools.development.js	n/a	n/a	0 B	16.33 KB	0 B	4.85 KB	NODE_DEV
react-debug-tools.production.min.js	n/a	n/a	0 B	5.28 KB	0 B	2.14 KB	NODE_PROD

Generated by 🚫 dangerJS

sophiebits

nice

sophiebits · 2018-11-03T04:03:25Z

packages/react-debug-tools/README.md

+
+This is an experimental package for debugging React renderers.
+
+**Its API is not as stable as that of React, React Native, or React DOM, and does not follow the common versioning scheme.**


sophiebits · 2018-11-03T04:04:31Z

packages/react-debug-tools/index.js

+ * LICENSE file in the root directory of this source tree.
+ */
+
+// This entry point is intentionally not typed. It exists only for third-party


copy pasta?

sophiebits · 2018-11-03T04:09:59Z

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

+}
+
+function parseCustomHookName(functionName: string): string {
+  let startIndex = functionName.indexOf('.');


lastIndexOf?

Good catch. Don’t know if any conditions report multiple levels but there is at least one.

sophiebits · 2018-11-03T04:11:10Z

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

+}
+
+export function inspectHooksOfFiber(fiber: Fiber) {
+  if (fiber.tag !== FunctionComponent && fiber.tag !== SimpleMemoComponent) {


Can’t ForwardRef have hooks?

useImperativeMethods specifically only works with ForwardRef too. I opened a related issue at facebook/react-devtools#1213.

sophiebits · 2018-11-03T20:57:15Z

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

+}
+
+function isReactWrapper(functionName, primitiveName) {
+  let expectedPrimitiveName = 'use' + primitiveName;


is this to handle dots? you could also do

let basename = functionName.split('.').pop(); return basename === primitiveName;

kinda hard to tell what names this is trying to catch

I figured it would be unlikely for a custom hook to be called useAbuseState or something like that.

Not too happy about this heuristic but not sure what’s better.

sophiebits · 2018-11-03T21:04:05Z

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

+let ReactTestRenderer;
+let ReactDebugTools;
+
+describe('ReactHooksInspectionIntergration', () => {


add a custom hook integration test?

Would be nice to have some basic testing of the stack parsing+intersection logic too.

gaearon · 2018-11-05T00:48:27Z

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

+  // Warm up the cache so that it doesn't consume the currentHook.
+  getPrimitiveStackCache();
+  let type = fiber.type;
+  let props = fiber.memoizedProps;


Does this have default props resolved?

I'm not sure this is safe because the return fibers may not be current but close enough and it's fast. We use this to set up the current values of the providers.

Just in case. I don't know of any scenario where this can happen.

bvaughn · 2018-11-05T17:02:57Z

Ideally we want this in DevTools but it requires so much implementation details that I don't think we'll want to maintain this with multi-version support. We'll probably have to move to snapshot versions of this package in DevTools, or just snapshoting all of DevTools.

Related issue: facebook/react-devtools/issues/1214

If we were to rearchitect this package in the way I propose on that issue, would there still be value in releasing react-debug-tools as its own package?

For each primitive hook in the log, process the stack trace by removing any shared ancestors with the root of the inspection function. This ensures that the bottom of the stack is the render function. Also, for that particular primitive, remove the top of the stack that is shared with the stack when that primitive is called stand alone from a function component. This removes the call to the primitive itself.

Nice! 👏

bvaughn · 2018-11-05T17:08:04Z

scripts/rollup/results.json

-      "size": 95936,
-      "gzip": 25258
+      "size": 98406,
+      "gzip": 25783


I don't think we normally include results.json in PRs (to avoid causing merge conflicts).

We should just delete it and stop generating it on local builds :P

bvaughn · 2018-11-05T17:11:44Z

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

+let ReactTestRenderer;
+let ReactDebugTools;
+
+describe('ReactHooksInspectionIntergration', () => {


Would be nice to have some basic testing of the stack parsing+intersection logic too.

bvaughn · 2018-11-05T17:14:30Z

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

+      Dispatcher.useEffect(() => {});
+      Dispatcher.useImperativeMethods(undefined, () => null);
+      Dispatcher.useCallback(() => {});
+      Dispatcher.useMemo(() => null);


Would be nice if we had an automated check to ensure that the set of fake hooks here stays in sync with the set of real ones.

We probably need a Flow type for this. There's not only one real one. There are at least three (Fiber, old SSR, new SSR).

And shallow one!

bvaughn · 2018-11-05T17:15:35Z

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

+
+function useState<S>(
+  initialState: (() => S) | S,
+): [S, Dispatch<BasicStateAction<S>>] {


Would be nice if we had an automated check to ensure these hook signatures stayed in-sync with the real ones.

The type should do that https://github.com/facebook/react/blob/master/packages/react/src/ReactCurrentOwner.js#L25

bvaughn · 2018-11-05T17:17:48Z

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

+      Dispatcher.useImperativeMethods(undefined, () => null);
+      Dispatcher.useCallback(() => {});
+      Dispatcher.useMemo(() => null);
+    } finally {


Why do we swallow errors in this way? An error in an given hook would block parsing of all subsequent hooks.

We don't swallow errors. This is a finally call.

Yeah, I chose my words poorly.

I was trying to point out that an error halfway through the hooks initialization would leave primitiveStackCache halfway populated with stack info, which seems weird. What's the point? It seems like we should just fail hard (no primitiveStackCache, future calls to this method also fail)

@bvaughn We only set primitiveStackCache if it doesn't throw.

🤪 Right.

bvaughn · 2018-11-05T17:19:58Z

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

+  if (hook !== null) {
+    currentHook = hook.next;
+  }
+  return hook;


Seems a little confusing that a function named nextHook returns the current hook 😄

bvaughn · 2018-11-05T17:26:21Z

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

+//
+// 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
+// truncated to a stack trace limit.


This comment is great. ^

Would be nice to have some tests for these cases.

bvaughn · 2018-11-05T17:41:48Z

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

+        // Store the current value that we're going to restore later.
+        contextMap.set(context, context._currentValue);
+        // Set the inner most provider value on the context.
+        context._currentValue = current.memoizedProps.value;


Why not just read the value from contextMap directly in readContext and useContext (rather than temporarily overriding context._currentValue)?

The contextMap stores the shadowed value, not the current one.

It's nice to use the same mechanism as Fiber because it allows the inspectHook thing to be called in a context where it context has already been set up with some other mechanism.

* Add debug tools package * Add basic implementation * Implement inspection of the current state of hooks using the fiber tree * Support useContext hooks inspection by backtracking from the Fiber I'm not sure this is safe because the return fibers may not be current but close enough and it's fast. We use this to set up the current values of the providers. * rm copypasta * Use lastIndexOf Just in case. I don't know of any scenario where this can happen. * Support ForwardRef * Add test for memo and custom hooks * Support defaultProps resolution

a-x- · 2019-08-09T08:58:16Z

Can you help me to clarify, what is useInspect? there is not any useInspect in react source, google also don't know anything about it

bvaughn · 2019-08-09T15:40:20Z

@a-x- The hook Sebastian was referring to was later added as useDebugValue (#14559)

You can read about it here: https://reactjs.org/docs/hooks-reference.html#usedebugvalue

* Add debug tools package * Add basic implementation * Implement inspection of the current state of hooks using the fiber tree * Support useContext hooks inspection by backtracking from the Fiber I'm not sure this is safe because the return fibers may not be current but close enough and it's fast. We use this to set up the current values of the providers. * rm copypasta * Use lastIndexOf Just in case. I don't know of any scenario where this can happen. * Support ForwardRef * Add test for memo and custom hooks * Support defaultProps resolution

sebmarkbage requested review from sophiebits, bvaughn, gaearon and acdlite November 3, 2018 03:57

facebook-github-bot added the CLA Signed label Nov 3, 2018

sophiebits approved these changes Nov 3, 2018

View reviewed changes

bvaughn mentioned this pull request Nov 4, 2018

☂️ Hooks umbrella task facebook/react-devtools#1215

Closed

4 tasks

gaearon reviewed Nov 5, 2018

View reviewed changes

sebmarkbage added 9 commits November 4, 2018 22:44

Add debug tools package

bff1863

Add basic implementation

209636b

Implement inspection of the current state of hooks using the fiber tree

79288cb

Support useContext hooks inspection by backtracking from the Fiber

6dcf6c8

I'm not sure this is safe because the return fibers may not be current but close enough and it's fast. We use this to set up the current values of the providers.

rm copypasta

7a9ae47

Use lastIndexOf

677f1db

Just in case. I don't know of any scenario where this can happen.

Support ForwardRef

3ec453a

Add test for memo and custom hooks

bd99c87

Support defaultProps resolution

7006c4b

sebmarkbage force-pushed the hooksreflection branch from 9e51849 to 7006c4b Compare November 5, 2018 08:04

bvaughn approved these changes Nov 5, 2018

View reviewed changes

sebmarkbage merged commit fd1256a into facebook:master Nov 5, 2018

sebmarkbage mentioned this pull request Nov 16, 2018

RFC: React Hooks reactjs/rfcs#68

Merged

gaearon mentioned this pull request Nov 20, 2018

Support: useEffect(..) hook getify/TNG-Hooks#3

Closed


		This is an experimental package for debugging React renderers.

		Its API is not as stable as that of React, React Native, or React DOM, and does not follow the common versioning scheme.

Add Debug Tools Package for Introspection of Hooks #14085

Add Debug Tools Package for Introspection of Hooks #14085

Conversation

sebmarkbage commented Nov 3, 2018 • edited Loading

sizebot commented Nov 3, 2018 • edited Loading

scheduler

react-debug-tools

sophiebits left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bvaughn commented Nov 5, 2018

bvaughn Nov 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

a-x- commented Aug 9, 2019 • edited Loading

bvaughn commented Aug 9, 2019 • edited Loading

sebmarkbage commented Nov 3, 2018 •

edited

Loading

sizebot commented Nov 3, 2018 •

edited

Loading

bvaughn Nov 5, 2018 •

edited

Loading

a-x- commented Aug 9, 2019 •

edited

Loading

bvaughn commented Aug 9, 2019 •

edited

Loading