-
Notifications
You must be signed in to change notification settings - Fork 46.7k
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
fix: use public instance in Fiber renderer and expose it from getInspectorDataForViewAtPoint #31068
fix: use public instance in Fiber renderer and expose it from getInspectorDataForViewAtPoint #31068
Conversation
…ectorDataForViewAtPoint
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Comparing: 778e1ed...0180849 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
typeof instance.canonical.publicInstance === 'object' && | ||
instance.canonical.publicInstance !== null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be a non-object? In Legacy Fabric, for example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not AFAIK. And canonical
is only available in Fabric.
if (typeof instance === 'object' && instance !== null) { | ||
if (typeof instance.canonical === 'object' && instance.canonical !== null) { | ||
if ( | ||
typeof instance.canonical.publicInstance === 'object' && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: you can reduce nesting here combining the conditions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I've decided to do nesting instead, because combining will produce a very long line, looks a bit uglier :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…ectorDataForViewAtPoint (#31068) React DevTools no longer operates with just Fibers, it now builds its own Shadow Tree, which represents the tree on the Host (Fabric on Native, DOM on Web). We have to keep track of public instances for a select-to-inspect feature. We've recently changed this logic in #30831, and looks like we've been incorrectly getting a public instance for Fabric case. Not only this, turns out that all `getInspectorData...` APIs are returning Fibers, and not public instances. I have to expose it, so that React DevTools can correctly identify the element, which was selected. Changes for React Native are in [D63421463](https://www.internalfb.com/diff/D63421463) DiffTrain build for commit d66fa02.
Summary: # Changelog: [Internal] Please see facebook/react#31068. Synced changes for React Native - D63453667. Differential Revision: D63421463
Summary: Pull Request resolved: facebook#46664 # Changelog: [Internal] Please see facebook/react#31068. Synced changes for React Native - D63453667. Differential Revision: D63421463
Summary: Pull Request resolved: facebook#46664 # Changelog: [Internal] Please see facebook/react#31068. Synced changes for React Native - D63453667. Differential Revision: D63421463
Summary: Pull Request resolved: facebook#46664 # Changelog: [Internal] Please see facebook/react#31068. Synced changes for React Native - D63453667. Reviewed By: huntie Differential Revision: D63421463
Summary: Pull Request resolved: facebook#46664 # Changelog: [Internal] Please see facebook/react#31068. Synced changes for React Native - D63453667. Reviewed By: huntie Differential Revision: D63421463
Summary: Pull Request resolved: #46664 # Changelog: [Internal] Please see facebook/react#31068. Synced changes for React Native - D63453667. Reviewed By: huntie Differential Revision: D63421463 fbshipit-source-id: e455711206598a06f7cdce4150e79c3548843b99
**breaking change for canary users: Bumps peer dependency of React from `19.0.0-rc-778e1ed2-20240926` to `19.0.0-rc-3edc000d-20240926`** [diff facebook/react@778e1ed2...3edc000d](facebook/react@778e1ed...3edc000) <details> <summary>React upstream changes</summary> - facebook/react#31078 - facebook/react#31083 - facebook/react#31079 - facebook/react#31080 - facebook/react#31076 - facebook/react#31021 - facebook/react#31069 - facebook/react#31074 - facebook/react#31073 - facebook/react#31047 - facebook/react#31046 - facebook/react#31045 - facebook/react#31072 - facebook/react#30980 - facebook/react#30463 - facebook/react#30694 - facebook/react#31039 - facebook/react#31048 - facebook/react#31068 </details> --------- Co-authored-by: Josh Story <[email protected]>
…#70560) **breaking change for canary users: Bumps peer dependency of React from `19.0.0-rc-778e1ed2-20240926` to `19.0.0-rc-3edc000d-20240926`** [diff facebook/react@778e1ed2...3edc000d](facebook/react@778e1ed...3edc000) <details> <summary>React upstream changes</summary> - facebook/react#31078 - facebook/react#31083 - facebook/react#31079 - facebook/react#31080 - facebook/react#31076 - facebook/react#31021 - facebook/react#31069 - facebook/react#31074 - facebook/react#31073 - facebook/react#31047 - facebook/react#31046 - facebook/react#31045 - facebook/react#31072 - facebook/react#30980 - facebook/react#30463 - facebook/react#30694 - facebook/react#31039 - facebook/react#31048 - facebook/react#31068 </details> --------- Co-authored-by: Josh Story <[email protected]>
Changes in this release: * Fix React Compiler badging ([poteto](https:/poteto) in [#31196](#31196)) * fix[react-devtools]: fixed timeline profiler tests ([hoxyq](https:/hoxyq) in [#31261](#31261)) * fix[react-devtools]: record timeline data only when supported ([hoxyq](https:/hoxyq) in [#31154](#31154)) * refactor[react-devtools]: flatten reload and profile config ([hoxyq](https:/hoxyq) in [#31132](#31132)) * fix[react-devtools]: remove all listeners when Agent is shutdown ([hoxyq](https:/hoxyq) in [#31151](#31151)) * fix[react-devtools]: removed redundant startProfiling call ([hoxyq](https:/hoxyq) in [#31131](#31131)) * refactor[react-devtools/fiber/renderer]: optimize durations resolution ([hoxyq](https:/hoxyq) in [#31118](#31118)) * fix[react-devtools]: update profiling status before receiving response from backend ([hoxyq](https:/hoxyq) in [#31117](#31117)) * fix[react-devtools]: wrap key string in preformatted text html element ([hoxyq](https:/hoxyq) in [#31153](#31153)) * chore[react-devtools]: drop legacy context tests ([hoxyq](https:/hoxyq) in [#31059](#31059)) * chore[react-devtools]: add legacy mode error message to the ignore list for tests ([hoxyq](https:/hoxyq) in [#31060](#31060)) * fix[react-devtools]: request hook initialization inside http server response ([hoxyq](https:/hoxyq) in [#31102](#31102)) * [Flight] Serialize Server Components Props in DEV ([sebmarkbage](https:/sebmarkbage) in [#31105](#31105)) * Add: reload to profile for Fusebox ([EdmondChuiHW](https:/EdmondChuiHW) in [#31021](#31021)) * refactor: allow custom impl of backend realod-to-profile support check ([EdmondChuiHW](https:/EdmondChuiHW) in [#31048](#31048)) * fix: use public instance in Fiber renderer and expose it from getInspectorDataForViewAtPoint ([hoxyq](https:/hoxyq) in [#31068](#31068))
React DevTools no longer operates with just Fibers, it now builds its own Shadow Tree, which represents the tree on the Host (Fabric on Native, DOM on Web).
We have to keep track of public instances for a select-to-inspect feature. We've recently changed this logic in #30831, and looks like we've been incorrectly getting a public instance for Fabric case.
Not only this, turns out that all
getInspectorData...
APIs are returning Fibers, and not public instances. I have to expose it, so that React DevTools can correctly identify the element, which was selected.Changes for React Native are in D63421463