devtools: Add `ref` to inspect element view #21879

eps1lon · 2021-07-14T14:23:54Z

Summary

Displays the ref of an inspected element if the element has one. Was a bit difficult to track a particular ref through a somewhat larger component tree.

Ref to a class instance

Ref to an Element

anonymous callback ref

named callback ref

The UI feels a bit lost since we only have a single value in this new section. Ideally we'd have the key in the view too so we could create a "Misc" view.

On the other hand, the ref will eventually be just another prop (if I understood reactjs/rfcs#107 correctly) so it'll merge with the props view eventually.

Test Plan

Tested in shell
CI green

eps1lon · 2021-07-14T14:27:31Z

packages/react-devtools-shared/src/utils.js

+  if (
+    (typeof HTMLElement !== 'undefined' && data instanceof HTMLElement) ||
+    // cross-realm HTMLElement?
+    /^\[object HTML.+Element\]$/.test(Object.prototype.toString.call(data))


As far as I can tell this is only needed for the shell at the moment. Without this change

function Example() { const ref = React.useRef(); return <React.Fragment><div ref={ref} /><SomeComponent foo={ref} /></React.Fragment> }

would also crash if one would attempt to inspect SomeComponent since foo.current would not be considered an HTMLElement. I can add the described case to the existing shell to make sure we don't regress when removing the special casing of ref.

bvaughn · 2021-07-14T19:31:35Z

I think I dig this change overall. I'm going to poke at the UI a little. May have a nit or two.

bvaughn · 2021-07-14T19:32:34Z

The UI feels a bit lost since we only have a single value in this new section. Ideally we'd have the key in the view too so we could create a "Misc" view.

The key is shown as part of the name up top, e.g. the little "3" to the left of the name:

On the other hand, the ref will eventually be just another prop (if I understood reactjs/rfcs#107 correctly) so it'll merge with the props view eventually.

Yeah. Makes me wonder if we should just show it inline, within the props panel...but maybe this would be confusing to people, since it's not editable?

eps1lon · 2021-07-14T19:51:45Z

Yeah. Makes me wonder if we should just show it inline, within the props panel...but maybe this would be confusing to people, since it's not editable?

Another idea would be to have it under "Props" but a little bit separate (e.g. with a horizontal divider).

Though my initial idea also was to just put them in the props. My concern was not really about editable vs not editable but that ref needs a special API to work "like" a prop: React.forwardRef.

The key is shown as part of the name up top, e.g. the little "3" to the left of the name:

I always miss that, thanks!.

bvaughn · 2021-07-14T20:32:32Z

packages/react-devtools-shared/src/devtools/views/Components/InspectedElementView.js

@@ -131,6 +132,13 @@ export default function InspectedElementView({
          store={store}
        />

+        <InspectedElementRefTree


If we do keep ref as a separate panel, I think it should be above "props" because it's kind of a top-level thing (like key)

Just marking this unresolved for visibility.

I always view the order in "what is most important during debugging" and I don't think ref is more important than other props. Case in point: There hasn't been any request to expose the ref in devtools.

There hasn't been any request to expose the ref in devtools.

That's a good point!

Although at the same time, sometimes people don't know that it's something they could have asked for.

packages/react-devtools-shared/src/devtools/views/Components/InspectedElementRefTree.js

bvaughn · 2021-07-14T20:45:20Z

Displays the ref of an inspected element if the element has one. Was a bit difficult to track a particular ref through a somewhat larger component tree.

Can you tell me a little more about this case?

bvaughn · 2021-07-14T21:15:19Z

If we land a change like this, I think I'm leaning towards showing these special not-quite-props in the "props" section, since their values get passed in like props:

bvaughn · 2021-07-14T21:26:29Z

What if I added a new section called "special props" (to match the React docs) and it had "key" and "ref" ?

bvaughn · 2021-07-14T21:44:51Z

@eps1lon How do you feel about an alternative like this?

Key may not make the cut here. Despite what the docs say, Sebastian thinks it's more about the parent than the currently-selected element (and I see this point)

ryota-murakami · 2021-07-15T05:59:21Z

This Twitter vote seems like here's context 🗳
https://twitter.com/brian_d_vaughn/status/1415533461583147009

eps1lon · 2021-07-15T06:35:59Z

Can you tell me a little more about this case?

I had some trouble making sure that nodeRef on PickersCalendarSlideTransition was passed as a ref to PickersCalendarWeekContainer and vice versa.
In another instance I had a forwardRef component that rendered div > div and I wanted to make sure the ref is passed to the second div.

All of these things can be done in code but I'm relying more and more on devtools for analyzing which prop goes where. ref not being part of that story slows things down quite a bit since I now have to switch between IDE and devtools.

@eps1lon How do you feel about an alternative like this?

That seems fine to me. Though I don't have any strong opinions about either variant (props, special props, with key, without key) etc since I'm comfortable distinguishing their semantics. I think we'll see over time if the UI is confusing or not.

eps1lon · 2021-07-15T06:42:10Z

packages/react-devtools-shared/src/backend/renderer.js

+    // Ref is a special case;
+    // don't dehydrate it in the same way (because it's not hydratable/inspectable).
+    // Just stringify it instead...


How is ref different than a prop holding a ref? For example, before React.forwardRef we often had a custom prop like forwardedRef.

This is a good question. Ref is different (at least currently) because it has special React semantics. But you're right, a custom prop name like forwardedRef would be inspectable and would show Fiber internals. Maybe we need a more robust solution for filtering keys like that on the DevTools side. 🤔 Could be a follow up to consider.

Also to consider: The ref might not point to a DOM element but point to imperative methods or other mutable values.

My gut feeling was that stringifying is still okay in that case, but maybe...I don't know. I don't really know if I believe in this change overall yet.

bvaughn · 2021-07-15T14:50:11Z

All of these things can be done in code but I'm relying more and more on devtools for analyzing which prop goes where.

This is nice to hear 😄

Some feedback from Rachel and Dan led me to try a slight variation here (basically just getting rid of the "special props" header):

Feedback from Sebastian has me considering dropping key too and just showing ref here.

eps1lon · 2021-07-15T15:16:45Z

Some feedback from Rachel and Dan led me to try a slight variation here (basically just getting rid of the "special props" header):

I like that. But we need to make sure that no other component with a header can be rendered in between (in the future).

bvaughn · 2021-07-15T20:42:34Z

I think I might just remove key and combine the ref and HOC badges into a single panel up top.

Current UI:

Proposed change:

Dan shared a concern (that I agree with) about showing "hocs" as a props-like-thing-that-has-a-name so maybe this mock isn't going to fly.

bvaughn · 2021-07-15T21:03:16Z

@gaearon said: #21796 (comment)

I also really disliked seeing a HOC strip and a "ref" strip as separate panels

HOCs are kind of fading away though? I don't know if they're even that common in practice in modern codebases.

Yes and no. At least React.forwardRef is going to be around for a while. And React.memo too for some cases.

bvaughn · 2021-07-16T03:37:16Z

Okay. I split refs and hocs into separate panels.

Looks a little weird when there's both, but I guess not too common.

ryota-murakami · 2021-09-29T09:06:54Z

I think most DOM related ref is used to escape hatch for getting access raw DOM API.
So ref behavior programed in mostly useEffect/customHook that is not appeared on React Element tree.
I think good idea showing ref label explicitly and it's great I could [go to definition] from ref label(click,dBclick, etc...).
Destination is devlools source tab that provide sourcemaped original code, or just jump user editor.
I think read real code is pretty much well option to understanding what does the 'ref' accutualy doing from the ''ref" label.

I guess the proxy snapshot just lists every accessed property

facebook-github-bot added the CLA Signed label Jul 14, 2021

eps1lon reviewed Jul 14, 2021

View changes

eps1lon marked this pull request as ready for review Jul 14, 2021

bvaughn self-requested a review Jul 14, 2021

bvaughn self-assigned this Jul 14, 2021

bvaughn reviewed Jul 14, 2021

View changes

bvaughn added the Component: Developer Tools label Jul 15, 2021

eps1lon reviewed Jul 15, 2021

View changes

crypt096 approved these changes Jul 15, 2021

View changes

bvaughn added the Type: Discussion label Jul 15, 2021

bvaughn force-pushed the feat/devtools/props-ref branch from 24370f6 to 04f1176 Compare Jul 15, 2021

bvaughn mentioned this pull request Jul 15, 2021

[DevTools] Improve Fast Refresh support for named hook detection #21796

Closed

bvaughn force-pushed the feat/devtools/props-ref branch from 04f1176 to bcaf20b Compare Jul 16, 2021

eps1lon force-pushed the feat/devtools/props-ref branch from bcaf20b to c39658e Compare Jul 19, 2021

eps1lon force-pushed the feat/devtools/props-ref branch from c39658e to 474d8a5 Compare Sep 29, 2021

eps1lon and others added 3 commits Jul 3, 2022

Add ref to inspect element view

fe36ca7

Update test

7ad89dc

I guess the proxy snapshot just lists every accessed property

Moved ref to its own section, without a head, displayed above props

3fc0c2e

eps1lon force-pushed the feat/devtools/props-ref branch from 474d8a5 to 3fc0c2e Compare Jul 3, 2022

Jun	JUL	Aug
	08
2021	2022	2023

devtools: Add `ref` to inspect element view #21879

devtools: Add `ref` to inspect element view #21879

eps1lon commented Jul 14, 2021 •

edited

eps1lon Jul 14, 2021

bvaughn commented Jul 14, 2021

bvaughn commented Jul 14, 2021

eps1lon commented Jul 14, 2021

bvaughn Jul 14, 2021

eps1lon Jul 15, 2021 •

edited

bvaughn Jul 15, 2021

bvaughn commented Jul 14, 2021

bvaughn commented Jul 14, 2021

bvaughn commented Jul 14, 2021

bvaughn commented Jul 14, 2021 •

edited

ryota-murakami commented Jul 15, 2021

eps1lon commented Jul 15, 2021

eps1lon Jul 15, 2021

bvaughn Jul 15, 2021

eps1lon Jul 19, 2021

bvaughn Jul 19, 2021

bvaughn commented Jul 15, 2021 •

edited

eps1lon commented Jul 15, 2021

bvaughn commented Jul 15, 2021

bvaughn commented Jul 15, 2021

bvaughn commented Jul 16, 2021 •

edited

ryota-murakami commented Sep 29, 2021

devtools: Add ref to inspect element view #21879

Are you sure you want to change the base?

devtools: Add ref to inspect element view #21879

Conversation

eps1lon commented Jul 14, 2021 • edited

Summary

Test Plan

eps1lon Jul 14, 2021

Choose a reason for hiding this comment

bvaughn commented Jul 14, 2021

bvaughn commented Jul 14, 2021

eps1lon commented Jul 14, 2021

bvaughn Jul 14, 2021

Choose a reason for hiding this comment

eps1lon Jul 15, 2021 • edited

Choose a reason for hiding this comment

bvaughn Jul 15, 2021

Choose a reason for hiding this comment

bvaughn commented Jul 14, 2021

bvaughn commented Jul 14, 2021

bvaughn commented Jul 14, 2021

bvaughn commented Jul 14, 2021 • edited

ryota-murakami commented Jul 15, 2021

eps1lon commented Jul 15, 2021

eps1lon Jul 15, 2021

Choose a reason for hiding this comment

bvaughn Jul 15, 2021

Choose a reason for hiding this comment

eps1lon Jul 19, 2021

Choose a reason for hiding this comment

bvaughn Jul 19, 2021

Choose a reason for hiding this comment

bvaughn commented Jul 15, 2021 • edited

eps1lon commented Jul 15, 2021

bvaughn commented Jul 15, 2021

bvaughn commented Jul 15, 2021

bvaughn commented Jul 16, 2021 • edited

ryota-murakami commented Sep 29, 2021

devtools: Add `ref` to inspect element view #21879

devtools: Add `ref` to inspect element view #21879

eps1lon commented Jul 14, 2021 •

edited

eps1lon Jul 15, 2021 •

edited

bvaughn commented Jul 14, 2021 •

edited

bvaughn commented Jul 15, 2021 •

edited

bvaughn commented Jul 16, 2021 •

edited