MVP hooks support #1272

bvaughn · 2019-01-09T19:01:25Z

Support (read-only) inspectable hooks.

Open questions

How should we handle production mode?

How should we handle hooks inspection in production mode? The react-debug-hooks package relies on parsing function names to identify custom hooks. If these names are mangled, they won't be identifiable, and as a result– the hooks tree will look strange.

Development:

Production:

…ce "opaque node" just points to whichever fiber happened to be created first for the pair)

Jessidhia · 2019-01-09T23:28:31Z

Should the inputs array given to useCallback (2nd argument) also be part of the inspection? It is part of the hook node at least.

bvaughn · 2019-01-09T23:48:54Z

No, we aren't currently planning on surfacing that info in DevTools.

TryingToImprove · 2019-01-10T16:47:43Z

Given I have component like below:

const C = () => {
   const [person, setPerson] = useState({ name: 'test' });
   const [schools, setSchools] = useState(['test', 'test2]);

   return (<div>{person} went to {schools.join(',')}</div>)

In devtools it will display

- Hooks
   - State: person
   - State: schools

So there will be mutiple "state"-objects in the list if I do not use "custom hooks".

Is what I am describing correct?

Saifadin · 2019-01-10T17:34:28Z

@TryingToImprove most likely it will look like this

- Hooks
  - State: {
      name: 'test'
    }
  - State: [
      'test', 'test2'
    ]

bvaughn · 2019-01-10T17:46:07Z

I should clarify my previous comment.

Yes, you will see multiple "State" hooks– because you are calling useState multiple times in that example. They'll be listed in the order you've used them in, and they'll show the values currently returned.

So with the code example you provided, it would actually look like this:

Hooks
  State: {…}
  State: Array[2]

bvaughn · 2019-01-11T16:43:00Z

Okay, I think this is ready for review.

bvaughn · 2019-01-11T16:44:55Z

agent/Agent.js

+      }
+      if (this._prevInspectedHooks !== inspectedHooks) {
+        this._prevInspectedHooks = inspectedHooks;
+        this.emit('inspectedHooks', inspectedHooks);


Doing this (storing data, having Fiber-specific code) in the Agent sucks. I'm open to suggestions for a better place for this logic to live, but at the same time– I didn't stress about it too much because I think this version of DevTools is short lived (without either a rewrite or a refactor).

bvaughn · 2019-01-11T16:46:01Z

agent/Agent.js

+        if (internals && internals.currentDispatcherRef) {
+          // HACK: This leaks Fiber-specific logic into the Agent which is not ideal.
+          // $FlowFixMe
+          const currentFiber = data.state === internalInstance.memoizedState ? internalInstance : internalInstance.alternate;


This is gross for obvious reasons, but it needs to live somewhere. Happy to move it out of the Agent if anyone thinks it's too gross even for a temporary solution.

gaearon · 2019-01-11T18:07:03Z

Does it still show "undefined" for Hooks that don't have values? Is that necessary/useful? Can be just empty?

bvaughn · 2019-01-11T18:08:58Z

Does it still show "undefined" for Hooks that don't have values? Is that necessary/useful? Can be just empty?

Good call. I added a guard for this.

ksaldana1 · 2019-01-11T19:17:12Z

backend/attachRendererFiber.js

+    var containsHooks =
+      (tag === FunctionComponent ||
+        tag === SimpleMemoComponent ||
+        tag == ForwardRef) &&


Are you wanting to use == for the FowardRef comparison here?

Typo. Doesn't actually matter in this case but I'll fix it.

threepointone

I am a merciful god.

(I'll test this later tonight, but lgtm!)

threepointone · 2019-01-13T17:41:32Z

frontend/HooksTreeView.js

+  theme: Theme,
+};
+
+function HooksNodeView({ hooksNode, index, inspect, path, theme }: HooksNodeViewProps) {


yo dawg I heard you like hooks

threepointone · 2019-01-14T17:40:45Z

frontend/PropState.js

@@ -251,4 +270,9 @@ const noPropsStateStyle = (theme: Theme) => ({
  padding: '0.5rem',
 });

+const hooksTreeLoadingStyle = {


any reason this isn't in a stylesheet? (low pri, ignore)

Just following the (super gross) convention within the file. Old DevTools are pretty gross.

bvaughn · 2019-01-14T18:12:10Z

Updated Profiler UI to not show a "State" pane for Fibers using hooks. Unfortunately, we can't show the hooks either– because we can't inspect stale Fibers.

gaearon · 2019-01-14T18:32:38Z

Good call. I added a guard for this.

Just to confirm. If I specifically call useDebugValue(undefined) I'd want to see it though. There's a difference between wanting to set undefined as a value, and not having a value.

bvaughn · 2019-01-14T18:36:09Z

Just to confirm. If I specifically call useDebugValue(undefined) I'd want to see it though.

There is no way for DevTools to differentiate between the two states of "I explicitly called useDebugValue(undefined)" and "I didn't call useDebugValue" so I chose to optimize for the case I think is far more common.

We can't show hooks values either, unfortunately, since we can't use the ReactDebugHooks util for stale fibers because their props will have changed.

bvaughn · 2019-01-14T22:03:05Z

I moved the fiber+hooks specific hacks inside of Agent into a new HooksInspector plugin because they felt pretty dirty, but with that... I think we're good enough for an MVP. Anything else that crops up– I'll fix it in the future! :)

Support hooks

3e60854

facebook-github-bot added the CLA Signed label Jan 9, 2019

bvaughn mentioned this pull request Jan 9, 2019

react-debug-tools accepts currentDispatcher ref as param facebook/react#14556

Merged

Ensure that hooks inspection is being done for the current fiber (sin…

46f69d9

…ce "opaque node" just points to whichever fiber happened to be created first for the pair)

Support useDebugValueLabel() hook

b5ac442

bvaughn force-pushed the hooks-integration-experimental branch from 8ce4a2e to b5ac442 Compare January 10, 2019 06:03

Brian Vaughn added 2 commits January 10, 2019 07:58

Rename useImperativeMethods -> useImperativeHandle

7b07dae

Rename useDebugValueLabel -> useDebugValue

9c29ee0

Copied over changes from React PR

21ab2e8

bvaughn changed the title ~~Support hooks [WIP]~~ MVP hooks support Jan 11, 2019

Fixed some edge case UI bugs with hooks updates

f1a92de

bvaughn force-pushed the hooks-integration-experimental branch from 4cc286d to f1a92de Compare January 11, 2019 16:42

bvaughn requested review from gaearon and sebmarkbage January 11, 2019 16:43

bvaughn commented Jan 11, 2019

View reviewed changes

Don't show undefined value for unlabeled custom hooks

b3de585

ksaldana1 reviewed Jan 11, 2019

View reviewed changes

Fix typo (== -> ===)

a89784d

threepointone approved these changes Jan 14, 2019

View reviewed changes

Don't show state for Fibers using hooks in Profiler

f456a68

We can't show hooks values either, unfortunately, since we can't use the ReactDebugHooks util for stale fibers because their props will have changed.

bvaughn force-pushed the hooks-integration-experimental branch from 73ff54c to f456a68 Compare January 14, 2019 18:54

Moved hooks-specific hacks inside of Agent into a new plug-in

16f4fcc

bvaughn merged commit 83ce90d into facebook:master Jan 14, 2019

bvaughn deleted the hooks-integration-experimental branch January 14, 2019 22:03

chenesan mentioned this pull request May 10, 2019

New hooks api suggestion / discussion -- inspectHook() and setHookState() enzymejs/enzyme#2120

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MVP hooks support #1272

MVP hooks support #1272

bvaughn commented Jan 9, 2019 •

edited

Loading

Jessidhia commented Jan 9, 2019 •

edited

Loading

bvaughn commented Jan 9, 2019

TryingToImprove commented Jan 10, 2019

Saifadin commented Jan 10, 2019

bvaughn commented Jan 10, 2019

bvaughn commented Jan 11, 2019

bvaughn Jan 11, 2019

bvaughn Jan 11, 2019

gaearon commented Jan 11, 2019

bvaughn commented Jan 11, 2019 •

edited

Loading

ksaldana1 Jan 11, 2019

bvaughn Jan 11, 2019

threepointone left a comment

threepointone Jan 13, 2019

threepointone Jan 14, 2019

bvaughn Jan 14, 2019

bvaughn commented Jan 14, 2019 •

edited

Loading

gaearon commented Jan 14, 2019

bvaughn commented Jan 14, 2019

bvaughn commented Jan 14, 2019

MVP hooks support #1272

MVP hooks support #1272

Conversation

bvaughn commented Jan 9, 2019 • edited Loading

Open questions

How should we handle production mode?

Development:

Production:

Jessidhia commented Jan 9, 2019 • edited Loading

bvaughn commented Jan 9, 2019

TryingToImprove commented Jan 10, 2019

Saifadin commented Jan 10, 2019

bvaughn commented Jan 10, 2019

bvaughn commented Jan 11, 2019

bvaughn Jan 11, 2019

Choose a reason for hiding this comment

bvaughn Jan 11, 2019

Choose a reason for hiding this comment

gaearon commented Jan 11, 2019

bvaughn commented Jan 11, 2019 • edited Loading

ksaldana1 Jan 11, 2019

Choose a reason for hiding this comment

bvaughn Jan 11, 2019

Choose a reason for hiding this comment

threepointone left a comment

Choose a reason for hiding this comment

threepointone Jan 13, 2019

Choose a reason for hiding this comment

threepointone Jan 14, 2019

Choose a reason for hiding this comment

bvaughn Jan 14, 2019

Choose a reason for hiding this comment

bvaughn commented Jan 14, 2019 • edited Loading

gaearon commented Jan 14, 2019

bvaughn commented Jan 14, 2019

bvaughn commented Jan 14, 2019

bvaughn commented Jan 9, 2019 •

edited

Loading

Jessidhia commented Jan 9, 2019 •

edited

Loading

bvaughn commented Jan 11, 2019 •

edited

Loading

bvaughn commented Jan 14, 2019 •

edited

Loading