-
Notifications
You must be signed in to change notification settings - Fork 49.8k
[React Native] Add getInspectorDataForViewAtPoint #18233
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
Changes from all commits
d568f13
1d7db7a
2a0a0b3
f2a947b
9c54afd
5b3e8a1
96b3319
a3b8921
cf2aa24
b572c1d
aa3719a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -100,6 +100,46 @@ type SecretInternalsType = { | |
| ... | ||
| }; | ||
|
|
||
| type InspectorDataProps = $ReadOnly<{ | ||
| [propName: string]: string, | ||
| ..., | ||
| }>; | ||
|
|
||
| type InspectorDataSource = $ReadOnly<{| | ||
| fileName?: string, | ||
| lineNumber?: number, | ||
| |}>; | ||
|
|
||
| type InspectorDataGetter = ( | ||
| (componentOrHandle: any) => ?number, | ||
| ) => $ReadOnly<{| | ||
| measure: Function, | ||
| props: InspectorDataProps, | ||
| source: InspectorDataSource, | ||
| |}>; | ||
|
|
||
| export type InspectorData = $ReadOnly<{| | ||
| hierarchy: Array<{| | ||
| name: ?string, | ||
| getInspectorData: InspectorDataGetter, | ||
| |}>, | ||
| selection: ?number, | ||
| props: InspectorDataProps, | ||
| source: ?InspectorDataSource, | ||
| |}>; | ||
|
|
||
| export type TouchedViewDataAtPoint = $ReadOnly<{| | ||
| pointerY: number, | ||
| touchedViewTag?: number, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is this used for? I thought we're getting rid of all these tags so seems odd to add new users of it and expose more implementation details about to change to future work. Shouldn't there be another mechanism?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is to support the current devtools behavior in Paper where tapping in the app will open the element in the tree. I was thinking we would add this now to prevent the regression, while we figure out the correct solution for Fabric and Paper moving forward. What do you think? |
||
| frame: $ReadOnly<{| | ||
| top: number, | ||
| left: number, | ||
| width: number, | ||
| height: number, | ||
| |}>, | ||
| ...InspectorData, | ||
| |}>; | ||
|
|
||
| /** | ||
| * Flat ReactNative renderer bundles are too big for Flow to parse efficiently. | ||
| * Provide minimal Flow typing for the high-level RN API and call it a day. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ | |
| import type {Fiber} from './ReactFiber'; | ||
| import type {FiberRoot} from './ReactFiberRoot'; | ||
| import type {RootTag} from 'shared/ReactRootTags'; | ||
| import type {TouchedViewDataAtPoint} from 'react-native-renderer/src/ReactNativeTypes'; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This import is causing CI to fail.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can't have dependencies on RN from the reconciler bundle. The folders are checked independently. Also from a layering perspective it's probably not right.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm a little confused that some of the commits seemed to pass CI |
||
| import type { | ||
| Instance, | ||
| TextInstance, | ||
|
|
@@ -109,6 +110,13 @@ type DevToolsConfig = {| | |
| // This API is unfortunately RN-specific. | ||
| // TODO: Change it to accept Fiber instead and type it properly. | ||
| getInspectorDataForViewTag?: (tag: number) => Object, | ||
| // Used by RN in-app inspector. | ||
| getInspectorDataForViewAtPoint?: ( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think either of these two should be part of this type because they're not actually used by the core DevTools protocol. It's kind of an adhoc RN specific addition. Instead we should make this a renderer specific object. Then you can define the type for RendererInspectionConfig through the HostConfig mechanism. So that each renderer gets their own type. That can then be safely referenced in the react-native folder. So Fabric and RN host configs gets this type: You then import that from here using:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like this suggestion. |
||
| inspectedView: Object, | ||
| locationX: number, | ||
| locationY: number, | ||
| callback: (viewData: TouchedViewDataAtPoint) => mixed, | ||
| ) => void, | ||
| |}; | ||
|
|
||
| let didWarnAboutNestedUpdates; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.