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.

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

Profiler: Show which hooks changed #312

Closed
bvaughn opened this issue Jun 9, 2019 · 7 comments
Closed

Profiler: Show which hooks changed #312

bvaughn opened this issue Jun 9, 2019 · 7 comments
Labels

Comments

@bvaughn
Copy link
Owner

bvaughn commented Jun 9, 2019

"Can you show which hooks changed?"

...is a question I've heard a couple of times with regard to the new Profiler change-tracking feature. This request is certainly understandable, but it presents a couple of challenges:

  1. Identifying which hooks values change would requires shallowly re-rendering each function component.
  2. Identifying a hook in a non-ambiguous way requires displaying the full hooks tree structure (since hooks aren't named).

Let's take each of a look at each of these below.

1 - Identifying which hooks values change

One of the challenge for DevTools when it comes to hooks is identifying custom hooks. Sebastian's proposed solution is that DevTools temporarily overrides React's hooks dispatcher while it shallowly re-renders the component. During the re-render, each time one of the built-in hooks is used, our override implementation parses the stack to identify "custom hooks" (functions higher up in the callstack that begin with "use"). After render is completed, we reassemble this information into a tree structure which DevTools can display.

Currently we only do this shallow render when a component is inspected, but in order for us to track which hooks have changed while profiling, we would need to shallowly render every component using hooks during the profiling session. Mostly likely we would have to do this during the performance sensitive "commit" phase since that's when DevTools is notified of an update.

I think we could do better than re-running the above hooks override for every component on every commit if we:

  • Created a map of Fiber to cached hooks tree structure.
  • Lazily populate the above map (by shallow re-rendering) only when a component was updated for the first time.
  • Compared Fiber memoizedStates to identify changes on future commits and map them back to the tree structure based on their position in the list structure. 1

However, even with the above optimizations this would still add significant overhead to a performance sensitive phase.

1 I think this should work but might also end up being complicated to implement.

2 - Identifying a hook

Although the variables that hooks values are assigned to are meaningfully named, the hooks themselves are unnamed. Because of this, DevTools has no feasible way of identifying a hook short of displaying the entire hooks tree structure. Consider the following example code:

function useCustomHook(...) {
  const [foo, setFoo] = useState(...);
  // ...
}

function ExampleComponent(props) {
  const [bar, setBar] = useState(...);
  const [baz, setBaz] = useState(...);
  const custom = useCustomHook(...);
  // ...
}

The example above shows 4 hooks: three useState and one custom. Let's say that "foo" and "baz" changed in a particular render. How would DevTools identify this? It could just show "two state hooks" but that's not very helpful. I think the only way we could identify it would be to show the entire tree, and visually highlight which hooks in it have changed:

State
State *
CustomHook
  State *

This is okay but it's not great unless the developer is cross-referencing the component (and probably the custom hooks definition as well). To help with this, we could also show the values but now we're adding more overhead in terms of trackin and bridge traffic.

In summary

Clearly both of these challenges can be overcome but they are non-trivial to implement and they will certainly add more runtime overhead to the profiler. Because of this, it may be a while before we add this feature to the DevTools.

@bvaughn bvaughn added 😎 enhancement New feature or request 💬 discussion labels Jun 9, 2019
@sompylasar
Copy link
Contributor

Babel JSX transform already adds source position to every JSX call. Can the compiler help figure out the Hooks tree statically, too, instead of trying to solve this in runtime?

@sompylasar
Copy link
Contributor

sompylasar commented Jun 9, 2019

The example above shows 4 hooks: three useState and one custom. Let's say that "foo" and "baz" changed in a particular render. How would DevTools identify this?
I think the only way we could identify it would be to show the entire tree, and visually highlight which hooks in it have changed

Another way: "useState#2 and useCustomHook changed" (but yes, it would be more helpful to see inside useCustomHook, so tree is better).

@sompylasar
Copy link
Contributor

Related Babel plugin: facebook/react#15733

function ButtonV1() {
  const [hover, setHover] = useState(false)
  const [pressed, setPressed] = useState(false)
  // ...
}
__signature__(ButtonV1, 'useState{[hover, setHover]}\nuseState{[pressed, setPressed]}')

@bvaughn
Copy link
Owner Author

bvaughn commented Jun 10, 2019

Babel JSX transform already adds source position to every JSX call. Can the compiler help figure out the Hooks tree statically, too, instead of trying to solve this in runtime?

Yeah, I'm aware of Dan's hot reload plug-in. I've been code reviewing the PRs.

That kind of solution would only work for DEV. (We wouldn't want to bloat production bundles.) Hooks inspection should work for prod bundles too.

Not saying it couldn't still be a potentially useful route for getting e.g. more meaningful names for hooks, but I don't think it would be a solution to this particular problem.

Another way: "useState#2 and useCustomHook changed"

This is not really sufficient if you also maintain useCustomHook and want to know why it changed. It also only punts the problem down the road a bit. Imagine the custom hook is e.g. useSubscription and you have multiple subscriptions setup. Now it's vague again.

@sompylasar
Copy link
Contributor

Hooks inspection should work for prod bundles too.

Do you think it must work the same way? Can it work in a reduced "some hooks changed" in PROD and "this tree of hooks changed" in DEV? The JSX debug source does not work in PROD, does it?

This is not really sufficient if

Yes, I agree, it's a half-solution.

@brunolemos
Copy link

This would be pretty useful, hopefully it gets worked on. Came here searching for this.

@bvaughn
Copy link
Owner Author

bvaughn commented Aug 19, 2019

This repository is being merged into the main React repo (github.com/facebook/react). As part of this, I am moving all issues to that repository as well and closing them here.

This issue has been relocated to:
facebook/react#16477

@bvaughn bvaughn closed this as completed Aug 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants