From 3554c8852fe209ad02380ebd24d32f56d6399906 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Mon, 20 Mar 2023 13:35:18 +0000 Subject: [PATCH] Clean interface for public instances between React and React Native (#26416) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary We are going to move the definition of public instances from React to React Native to have them together with the native methods in Fabric that they invoke. This will allow us to have a better type safety between them and iterate faster on the implementation of this proposal: https://github.com/react-native-community/discussions-and-proposals/pull/607 The interface between React and React Native would look like this after this change and a following PR (#26418): React → React Native: ```javascript ReactNativePrivateInterface.createPublicInstance // to provide via refs ReactNativePrivateInterface.getNodeFromPublicInstance // for DevTools, commands, etc. ReactNativePrivateInterface.getNativeTagFromPublicInstance // to implement `findNodeHandle` ``` React Native → React (ReactFabric): ```javascript ReactFabric.getNodeFromInternalInstanceHandle // to get most recent node to call into native ReactFabric.getPublicInstanceFromInternalInstanceHandle // to get public instances from results from native ``` ## How did you test this change? Flow Existing unit tests --- .../react-native-renderer/src/ReactFabric.js | 5 +++++ .../src/ReactFabricHostConfig.js | 7 ++++++ .../src/ReactFabricPublicInstanceUtils.js | 11 ++++++++-- .../src/ReactNativeFiberInspector.js | 9 ++------ .../src/ReactNativePublicCompat.js | 22 ++++++------------- .../src/ReactNativeTypes.js | 9 +++++++- .../__tests__/ReactFabric-test.internal.js | 13 +++++------ 7 files changed, 43 insertions(+), 33 deletions(-) diff --git a/packages/react-native-renderer/src/ReactFabric.js b/packages/react-native-renderer/src/ReactFabric.js index a6e2bd883b84f..0560e37bdb544 100644 --- a/packages/react-native-renderer/src/ReactFabric.js +++ b/packages/react-native-renderer/src/ReactFabric.js @@ -40,6 +40,7 @@ import { sendAccessibilityEvent, getNodeFromInternalInstanceHandle, } from './ReactNativePublicCompat'; +import {getPublicInstanceFromInternalInstanceHandle} from './ReactFabricHostConfig'; // $FlowFixMe[missing-local-annot] function onRecoverableError(error) { @@ -124,6 +125,10 @@ export { // This method allows it to acess the most recent shadow node for // the instance (it's only accessible through it). getNodeFromInternalInstanceHandle, + // Fabric native methods to traverse the host tree return the same internal + // instance handles we use to dispatch events. This provides a way to access + // the public instances we created from them (potentially created lazily). + getPublicInstanceFromInternalInstanceHandle, }; injectIntoDevTools({ diff --git a/packages/react-native-renderer/src/ReactFabricHostConfig.js b/packages/react-native-renderer/src/ReactFabricHostConfig.js index 2c2d5c598b366..c612fd1eb941f 100644 --- a/packages/react-native-renderer/src/ReactFabricHostConfig.js +++ b/packages/react-native-renderer/src/ReactFabricHostConfig.js @@ -241,6 +241,13 @@ export function getPublicInstance(instance: Instance): null | PublicInstance { return null; } +export function getPublicInstanceFromInternalInstanceHandle( + internalInstanceHandle: Object, +): null | PublicInstance { + const instance: Instance = internalInstanceHandle.stateNode; + return getPublicInstance(instance); +} + export function prepareForCommit(containerInfo: Container): null | Object { // Noop return null; diff --git a/packages/react-native-renderer/src/ReactFabricPublicInstanceUtils.js b/packages/react-native-renderer/src/ReactFabricPublicInstanceUtils.js index a1d9fbea4dcc5..203f96589fb7f 100644 --- a/packages/react-native-renderer/src/ReactFabricPublicInstanceUtils.js +++ b/packages/react-native-renderer/src/ReactFabricPublicInstanceUtils.js @@ -8,6 +8,7 @@ */ import type {ReactFabricHostComponent} from './ReactFabricPublicInstance'; +import {getNodeFromInternalInstanceHandle} from './ReactNativePublicCompat'; /** * IMPORTANT: This module is used in Paper and Fabric. It needs to be defined @@ -22,8 +23,14 @@ export function getNativeTagFromPublicInstance( return publicInstance.__nativeTag; } -export function getInternalInstanceHandleFromPublicInstance( +export function getNodeFromPublicInstance( publicInstance: ReactFabricHostComponent, ): mixed { - return publicInstance.__internalInstanceHandle; + if (publicInstance.__internalInstanceHandle == null) { + return null; + } + + return getNodeFromInternalInstanceHandle( + publicInstance.__internalInstanceHandle, + ); } diff --git a/packages/react-native-renderer/src/ReactNativeFiberInspector.js b/packages/react-native-renderer/src/ReactNativeFiberInspector.js index 94f9abf35f9a2..38a3855ea92a3 100644 --- a/packages/react-native-renderer/src/ReactNativeFiberInspector.js +++ b/packages/react-native-renderer/src/ReactNativeFiberInspector.js @@ -20,7 +20,7 @@ import {HostComponent} from 'react-reconciler/src/ReactWorkTags'; import {UIManager} from 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface'; import {enableGetInspectorDataForInstanceInProduction} from 'shared/ReactFeatureFlags'; import {getClosestInstanceFromNode} from './ReactNativeComponentTree'; -import {getInternalInstanceHandleFromPublicInstance} from './ReactFabricPublicInstanceUtils'; +import {getNodeFromPublicInstance} from './ReactFabricPublicInstanceUtils'; import {getNodeFromInternalInstanceHandle} from './ReactNativePublicCompat'; const emptyObject = {}; @@ -196,12 +196,7 @@ function getInspectorDataForViewAtPoint( if (__DEV__) { let closestInstance = null; - const fabricInstanceHandle = - getInternalInstanceHandleFromPublicInstance(inspectedView); - const fabricNode = - fabricInstanceHandle != null - ? getNodeFromInternalInstanceHandle(fabricInstanceHandle) - : null; + const fabricNode = getNodeFromPublicInstance(inspectedView); if (fabricNode) { // For Fabric we can look up the instance handle directly and measure it. nativeFabricUIManager.findNodeAtPoint( diff --git a/packages/react-native-renderer/src/ReactNativePublicCompat.js b/packages/react-native-renderer/src/ReactNativePublicCompat.js index a2979666d7e14..569397b4e8347 100644 --- a/packages/react-native-renderer/src/ReactNativePublicCompat.js +++ b/packages/react-native-renderer/src/ReactNativePublicCompat.js @@ -24,7 +24,7 @@ import ReactSharedInternals from 'shared/ReactSharedInternals'; import getComponentNameFromType from 'shared/getComponentNameFromType'; import { - getInternalInstanceHandleFromPublicInstance, + getNodeFromPublicInstance, getNativeTagFromPublicInstance, } from './ReactFabricPublicInstanceUtils'; @@ -176,14 +176,10 @@ export function dispatchCommand( return; } - const internalInstanceHandle = - getInternalInstanceHandleFromPublicInstance(handle); + const node = getNodeFromPublicInstance(handle); - if (internalInstanceHandle != null) { - const node = getNodeFromInternalInstanceHandle(internalInstanceHandle); - if (node != null) { - nativeFabricUIManager.dispatchCommand(node, command, args); - } + if (node != null) { + nativeFabricUIManager.dispatchCommand(node, command, args); } else { UIManager.dispatchViewManagerCommand(nativeTag, command, args); } @@ -204,13 +200,9 @@ export function sendAccessibilityEvent(handle: any, eventType: string) { return; } - const internalInstanceHandle = - getInternalInstanceHandleFromPublicInstance(handle); - if (internalInstanceHandle != null) { - const node = getNodeFromInternalInstanceHandle(internalInstanceHandle); - if (node != null) { - nativeFabricUIManager.sendAccessibilityEvent(node, eventType); - } + const node = getNodeFromPublicInstance(handle); + if (node != null) { + nativeFabricUIManager.sendAccessibilityEvent(node, eventType); } else { legacySendAccessibilityEvent(nativeTag, eventType); } diff --git a/packages/react-native-renderer/src/ReactNativeTypes.js b/packages/react-native-renderer/src/ReactNativeTypes.js index ca9a9dfc04d03..25436085debdb 100644 --- a/packages/react-native-renderer/src/ReactNativeTypes.js +++ b/packages/react-native-renderer/src/ReactNativeTypes.js @@ -213,6 +213,8 @@ export type ReactNativeType = { }; export opaque type Node = mixed; +type InternalInstanceHandle = mixed; +type PublicInstance = mixed; export type ReactFabricType = { findHostInstance_DEPRECATED( @@ -237,7 +239,12 @@ export type ReactFabricType = { concurrentRoot: ?boolean, ): ?ElementRef, unmountComponentAtNode(containerTag: number): void, - getNodeFromInternalInstanceHandle(internalInstanceHandle: mixed): ?Node, + getNodeFromInternalInstanceHandle( + internalInstanceHandle: InternalInstanceHandle, + ): ?Node, + getPublicInstanceFromInternalInstanceHandle( + internalInstanceHandle: InternalInstanceHandle, + ): PublicInstance, ... }; diff --git a/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js b/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js index 42ed69a8c290c..72377b7d12cc5 100644 --- a/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js @@ -16,7 +16,7 @@ let createReactNativeComponentClass; let StrictMode; let act; let getNativeTagFromPublicInstance; -let getInternalInstanceHandleFromPublicInstance; +let getNodeFromPublicInstance; const DISPATCH_COMMAND_REQUIRES_HOST_COMPONENT = "Warning: dispatchCommand was called with a ref that isn't a " + @@ -44,8 +44,8 @@ describe('ReactFabric', () => { .ReactNativeViewConfigRegistry.register; getNativeTagFromPublicInstance = require('../ReactFabricPublicInstanceUtils').getNativeTagFromPublicInstance; - getInternalInstanceHandleFromPublicInstance = - require('../ReactFabricPublicInstanceUtils').getInternalInstanceHandleFromPublicInstance; + getNodeFromPublicInstance = + require('../ReactFabricPublicInstanceUtils').getNodeFromPublicInstance; act = require('internal-test-utils').act; }); @@ -1032,10 +1032,7 @@ describe('ReactFabric', () => { nativeFabricUIManager.createNode.mock.results[0].value; expect(expectedShadowNode).toEqual(expect.any(Object)); - const internalInstanceHandle = - getInternalInstanceHandleFromPublicInstance(viewRef); - expect( - ReactFabric.getNodeFromInternalInstanceHandle(internalInstanceHandle), - ).toBe(expectedShadowNode); + const node = getNodeFromPublicInstance(viewRef); + expect(node).toBe(expectedShadowNode); }); });