refactor[renderer]: expose getInspectorDataForInstance in rendererConfig #26913

hoxyq · 2023-06-08T15:28:12Z

Summary

This is required for the case when we have an instance and want to get inspector data for it. Such case occurs when RN's application being debugged via React DevTools.

React DevTools sends instance to RN, which then gets all auxiliary data to highlight some elements. Having getInspectorDataForInstance method exposed makes it possible to easily get current props from fiber, which then can be used to display some margins & paddings for hovered element (via props.style).

I see that getInspectorDataForInstance is being exported at the top level of the renderer, but feels like this should also be inside DevTools global hook, the same way we use it for getInspectorDataForViewAtPoint.

react-sizebot · 2023-06-08T15:32:51Z

Comparing: 9100456...8b99a4f

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js	=	164.23 kB	164.23 kB	=	51.73 kB	51.73 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	171.67 kB	171.67 kB	=	53.97 kB	53.97 kB
facebook-www/ReactDOM-prod.classic.js	=	570.12 kB	570.12 kB	=	100.58 kB	100.58 kB
facebook-www/ReactDOM-prod.modern.js	=	553.90 kB	553.90 kB	=	97.75 kB	97.75 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 8b99a4f

hoxyq · 2023-06-08T16:53:01Z

packages/react-native-renderer/src/ReactFabric.js

@@ -137,6 +137,7 @@ injectIntoDevTools({
  version: ReactVersion,
  rendererPackageName: 'react-native-renderer',
  rendererConfig: {
+    getInspectorDataForInstance,


Q: Why its named getInspectorDataForInstance?

Should it be getInspectorDataForFiber?

rubennorte · 2023-06-09T09:52:01Z

I see that getInspectorDataForInstance is being exported at the top level of the renderer, but feels like this should also be inside DevTools global hook, the same way we use it for getInspectorDataForViewAtPoint.

Yeah, I think this makes sense for alignment with what we're doing now, but it also looks like React Native is (even before this change) abusing the DevTools renderer registration system to access information that DevTools has no use for. In other words, React Native shouldn't be doing this:

https://github.com/facebook/react-native/blob/5cc8ceeae210f2f23ef18ded7b7b614682f0d67b/packages/react-native/Libraries/Inspector/getInspectorDataForViewAtPoint.js#L32-L41

Perhaps in the future we can refactor this and implement this outside of DevTools.

In any case, this makes sense here considering what we're doing now.

…fig (facebook#26913) ## Summary This is required for the case when we have an instance and want to get inspector data for it. Such case occurs when RN's application being debugged via React DevTools. React DevTools sends instance to RN, which then gets all auxiliary data to highlight some elements. Having `getInspectorDataForInstance` method exposed makes it possible to easily get current props from fiber, which then can be used to display some margins & paddings for hovered element (via props.style). I see that `getInspectorDataForInstance` is being exported at the top level of the renderer, but feels like this should also be inside DevTools global hook, the same way we use it for [`getInspectorDataForViewAtPoint`](https://github.com/facebook/react-native/blob/e7d3662904e0e35e43380ee6d54859388713a592/packages/react-native/Libraries/Inspector/getInspectorDataForViewAtPoint.js).

…fig (#26913) ## Summary This is required for the case when we have an instance and want to get inspector data for it. Such case occurs when RN's application being debugged via React DevTools. React DevTools sends instance to RN, which then gets all auxiliary data to highlight some elements. Having `getInspectorDataForInstance` method exposed makes it possible to easily get current props from fiber, which then can be used to display some margins & paddings for hovered element (via props.style). I see that `getInspectorDataForInstance` is being exported at the top level of the renderer, but feels like this should also be inside DevTools global hook, the same way we use it for [`getInspectorDataForViewAtPoint`](https://github.com/facebook/react-native/blob/e7d3662904e0e35e43380ee6d54859388713a592/packages/react-native/Libraries/Inspector/getInspectorDataForViewAtPoint.js). DiffTrain build for commit 21a161f.

hoxyq requested review from rubennorte and rickhanlonii June 8, 2023 15:28

facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Jun 8, 2023

refactor[renderer]: expose getInspectorDataForInstance in rendererConfig

8b99a4f

hoxyq force-pushed the renderer/expose-get-inspector-data-for-instance branch from fa3f8bd to 8b99a4f Compare June 8, 2023 15:51

hoxyq commented Jun 8, 2023

View reviewed changes

rubennorte approved these changes Jun 9, 2023

View reviewed changes

hoxyq merged commit 21a161f into facebook:main Jun 9, 2023

hoxyq deleted the renderer/expose-get-inspector-data-for-instance branch June 9, 2023 09:55

styfle mentioned this pull request Jun 30, 2023

chore(deps): bump react 18.3.0-canary-1fdacbefd-20230630 vercel/next.js#52005

Merged

styfle mentioned this pull request Jul 5, 2023

chore(deps): bump react to 7118f5dd7 vercel/next.js#52282

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor[renderer]: expose getInspectorDataForInstance in rendererConfig #26913

refactor[renderer]: expose getInspectorDataForInstance in rendererConfig #26913

hoxyq commented Jun 8, 2023

react-sizebot commented Jun 8, 2023 •

edited

Loading

hoxyq Jun 8, 2023

rubennorte commented Jun 9, 2023 •

edited

Loading

refactor[renderer]: expose getInspectorDataForInstance in rendererConfig #26913

refactor[renderer]: expose getInspectorDataForInstance in rendererConfig #26913

Conversation

hoxyq commented Jun 8, 2023

Summary

react-sizebot commented Jun 8, 2023 • edited Loading

Critical size changes

Significant size changes

hoxyq Jun 8, 2023

Choose a reason for hiding this comment

rubennorte commented Jun 9, 2023 • edited Loading

react-sizebot commented Jun 8, 2023 •

edited

Loading

rubennorte commented Jun 9, 2023 •

edited

Loading