From 21396bb38015e266bab944b3ab1e35db64686ea5 Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Mon, 30 Mar 2020 14:02:41 -0700 Subject: [PATCH] Enable inspector for Fabric Summary: ## Overview This diff refactors the Inspector, moving logic to look up view data for a touched view inside the renderer as `getInspectorDataForViewAtPoint`. We then implement that same function for Fabric in order to support the inspector in that renderer. Requires https://github.com/facebook/react/pull/18388 ## Motivation Reason one for this refactor is that, previously, the inspector held all of the logic to look up view data for a given x,y touch coordinate. To do this, it would take the React tag and coordinates, look up the native view tag, measure it, and then ask React internals for the Fiber information of that tag. All of this is deeply coupled to React internals, yet the logic is outside of React core in the Inspector. Reason two is that, for Fabric, the logic for getting the view data is different than Paper. In Fabric, we pass the x,y coordinates to native directly, which returns an instance handle. That handle can be used to measure the ShadowNode, or retrieve the Fiber information. By moving the logic into the renderer in React core, we decouple the implementation details of looking up view data for a tapped point and allow ourselves the ability to add and change renderer-specific code for the actual lookup without impacting outsiders like the Inspector. Changelog: [Internal] Reviewed By: TheSavior Differential Revision: D20291710 fbshipit-source-id: a125223f2e44a6483120c41dc6146ad75a0e3e68 --- Libraries/Inspector/Inspector.js | 122 +++++++++++++++--------- Libraries/Inspector/InspectorOverlay.js | 20 +--- Libraries/ReactNative/AppContainer.js | 1 - 3 files changed, 82 insertions(+), 61 deletions(-) diff --git a/Libraries/Inspector/Inspector.js b/Libraries/Inspector/Inspector.js index 25993952658..2394a3dded9 100644 --- a/Libraries/Inspector/Inspector.js +++ b/Libraries/Inspector/Inspector.js @@ -22,11 +22,23 @@ const View = require('../Components/View/View'); const invariant = require('invariant'); -import type {HostComponent} from '../Renderer/shims/ReactNativeTypes'; +import type { + HostComponent, + TouchedViewDataAtPoint, +} from '../Renderer/shims/ReactNativeTypes'; + +type HostRef = React.ElementRef>; export type ReactRenderer = { - getInspectorDataForViewTag: (viewTag: number) => Object, - ... + rendererConfig: { + getInspectorDataForViewAtPoint: ( + inspectedView: ?HostRef, + locationX: number, + locationY: number, + callback: Function, + ) => void, + ... + }, }; const hook = window.__REACT_DEVTOOLS_GLOBAL_HOOK__; @@ -49,29 +61,33 @@ function findRenderers(): $ReadOnlyArray { return allRenderers; } -function getInspectorDataForViewTag(touchedViewTag: number) { +function getInspectorDataForViewAtPoint( + inspectedView: ?HostRef, + locationX: number, + locationY: number, + callback: (viewData: TouchedViewDataAtPoint) => void, +) { + // Check all renderers for inspector data. for (let i = 0; i < renderers.length; i++) { const renderer = renderers[i]; - if ( - Object.prototype.hasOwnProperty.call( - renderer, - 'getInspectorDataForViewTag', - ) - ) { - const inspectorData = renderer.getInspectorDataForViewTag(touchedViewTag); - if (inspectorData.hierarchy.length > 0) { - return inspectorData; - } + if (renderer?.rendererConfig?.getInspectorDataForViewAtPoint != null) { + renderer.rendererConfig.getInspectorDataForViewAtPoint( + inspectedView, + locationX, + locationY, + viewData => { + // Only return with non-empty view data since only one renderer will have this view. + if (viewData && viewData.hierarchy.length > 0) { + callback(viewData); + } + }, + ); } } - throw new Error('Expected to find at least one React renderer.'); } -type HostRef = React.ElementRef>; - class Inspector extends React.Component< { - isFabric: boolean, inspectedView: ?HostRef, onRequestRerenderApp: (callback: (instance: ?HostRef) => void) => void, ... @@ -91,6 +107,7 @@ class Inspector extends React.Component< > { _hideTimeoutID: TimeoutID | null = null; _subs: ?Array<() => void>; + _setTouchedViewData: ?(TouchedViewDataAtPoint) => void; constructor(props: Object) { super(props); @@ -121,6 +138,7 @@ class Inspector extends React.Component< this._subs.map(fn => fn()); } hook.off('react-devtools', this._attachToDevtools); + this._setTouchedViewData = null; } UNSAFE_componentWillReceiveProps(newProps: Object) { @@ -198,30 +216,50 @@ class Inspector extends React.Component< }); } - onTouchViewTag(touchedViewTag: number, frame: Object, pointerY: number) { - // Most likely the touched instance is a native wrapper (like RCTView) - // which is not very interesting. Most likely user wants a composite - // instance that contains it (like View) - const {hierarchy, props, selection, source} = getInspectorDataForViewTag( - touchedViewTag, - ); - - if (this.state.devtoolsAgent) { - // Skip host leafs - this.state.devtoolsAgent.selectNode(touchedViewTag); - } - - this.setState({ - panelPos: - pointerY > Dimensions.get('window').height / 2 ? 'top' : 'bottom', - selection, - hierarchy, - inspected: { - style: props.style, - frame, + onTouchPoint(locationX: number, locationY: number) { + this._setTouchedViewData = viewData => { + const { + hierarchy, + props, + selectedIndex, source, + frame, + pointerY, + touchedViewTag, + } = viewData; + + // Sync the touched view with React DevTools. + // Note: This is Paper only. To support Fabric, + // DevTools needs to be updated to not rely on view tags. + if (this.state.devtoolsAgent && touchedViewTag) { + this.state.devtoolsAgent.selectNode( + ReactNative.findNodeHandle(touchedViewTag), + ); + } + + this.setState({ + panelPos: + pointerY > Dimensions.get('window').height / 2 ? 'top' : 'bottom', + selection: selectedIndex, + hierarchy, + inspected: { + style: props.style, + frame, + source, + }, + }); + }; + getInspectorDataForViewAtPoint( + this.state.inspectedView, + locationX, + locationY, + viewData => { + if (this._setTouchedViewData != null) { + this._setTouchedViewData(viewData); + this._setTouchedViewData = null; + } }, - }); + ); } setPerfing(val: boolean) { @@ -265,10 +303,8 @@ class Inspector extends React.Component< {this.state.inspecting && ( )} diff --git a/Libraries/Inspector/InspectorOverlay.js b/Libraries/Inspector/InspectorOverlay.js index caaa38700d2..51a1ce55607 100644 --- a/Libraries/Inspector/InspectorOverlay.js +++ b/Libraries/Inspector/InspectorOverlay.js @@ -14,11 +14,8 @@ const Dimensions = require('../Utilities/Dimensions'); const ElementBox = require('./ElementBox'); const React = require('react'); const StyleSheet = require('../StyleSheet/StyleSheet'); -const ReactNative = require('../Renderer/shims/ReactNative'); -const UIManager = require('../ReactNative/UIManager'); const View = require('../Components/View/View'); -import type {HostComponent} from '../Renderer/shims/ReactNativeTypes'; import type {ViewStyleProp} from '../StyleSheet/StyleSheet'; import type {PressEvent} from '../Types/CoreEventTypes'; @@ -28,26 +25,15 @@ type Inspected = $ReadOnly<{| |}>; type Props = $ReadOnly<{| - isFabric: boolean, inspected?: Inspected, - inspectedView?: ?React.ElementRef>, - onTouchViewTag: (tag: number, frame: Object, pointerY: number) => mixed, + onTouchPoint: (locationX: number, locationY: number) => void, |}>; class InspectorOverlay extends React.Component { findViewForTouchEvent: (e: PressEvent) => void = (e: PressEvent) => { const {locationX, locationY} = e.nativeEvent.touches[0]; - UIManager.findSubviewIn( - ReactNative.findNodeHandle(this.props.inspectedView), - [locationX, locationY], - (nativeViewTag, left, top, width, height) => { - this.props.onTouchViewTag( - nativeViewTag, - {left, top, width, height}, - locationY, - ); - }, - ); + + this.props.onTouchPoint(locationX, locationY); }; shouldSetResponser: (e: PressEvent) => boolean = (e: PressEvent): boolean => { diff --git a/Libraries/ReactNative/AppContainer.js b/Libraries/ReactNative/AppContainer.js index e7772fc14fd..22e3bd3bef1 100644 --- a/Libraries/ReactNative/AppContainer.js +++ b/Libraries/ReactNative/AppContainer.js @@ -67,7 +67,6 @@ class AppContainer extends React.Component { const Inspector = require('../Inspector/Inspector'); const inspector = this.state.inspector ? null : ( { this.setState(