Implement public instances for text nodes in Fabric (#26516)

rubennorte · web-flow · commit 0700dd50bda9 · 2023-04-04T14:43:35.000+01:00
## Summary This adds the ability to create public instances for text nodes in Fabric. The implementation for the public instances lives in React Native (as it does for host components after #26437). The logic here just handles their lazy instantiation when requested via `getPublicInstanceFromInternalInstanceHandle`, which is called by Fabric with information coming from the shadow tree. It's important that the creation of public instances for text nodes is done lazily to avoid regressing memory usage when unused. Instances for text nodes are left intact if the public instance is never accessed. This is necessary to implement access to text nodes in React Native as explained in react-native-community/discussions-and-proposals#607 ## How did you test this change? Added unit tests (also fixed a test that was only testing the logic in a mock :S).
diff --git a/packages/react-native-renderer/src/ReactFabricHostConfig.js b/packages/react-native-renderer/src/ReactFabricHostConfig.js
@@ -14,13 +14,16 @@ import {
   DefaultEventPriority,
   DiscreteEventPriority,
 } from 'react-reconciler/src/ReactEventPriorities';
+import {HostText} from 'react-reconciler/src/ReactWorkTags';
 
 // Modules provided by RN:
 import {
   ReactNativeViewConfigRegistry,
   deepFreezeAndThrowOnMutationInDev,
   createPublicInstance,
+  createPublicTextInstance,
   type PublicInstance as ReactNativePublicInstance,
+  type PublicTextInstance,
 } from 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface';
 
 const {
@@ -47,23 +50,33 @@ const {get: getViewConfigForType} = ReactNativeViewConfigRegistry;
 // This means that they never overlap.
 let nextReactTag = 2;
 
+type InternalInstanceHandle = Object;
 type Node = Object;
 export type Type = string;
 export type Props = Object;
 export type Instance = {
   // Reference to the shadow node.
   node: Node,
+  // This object is shared by all the clones of the instance.
+  // We use it to access their shared public instance (exposed through refs)
+  // and to access its committed state for events, etc.
   canonical: {
     nativeTag: number,
     viewConfig: ViewConfig,
     currentProps: Props,
     // Reference to the React handle (the fiber)
-    internalInstanceHandle: Object,
+    internalInstanceHandle: InternalInstanceHandle,
     // Exposed through refs.
     publicInstance: PublicInstance,
   },
 };
-export type TextInstance = {node: Node, ...};
+export type TextInstance = {
+  // Reference to the shadow node.
+  node: Node,
+  // Text instances are never cloned, so we don't need to keep a "canonical"
+  // reference to make sure all clones of the instance point to the same values.
+  publicInstance?: PublicTextInstance,
+};
 export type HydratableInstance = Instance | TextInstance;
 export type PublicInstance = ReactNativePublicInstance;
 export type Container = number;
@@ -115,7 +128,7 @@ export function createInstance(
   props: Props,
   rootContainerInstance: Container,
   hostContext: HostContext,
-  internalInstanceHandle: Object,
+  internalInstanceHandle: InternalInstanceHandle,
 ): Instance {
   const tag = nextReactTag;
   nextReactTag += 2;
@@ -162,7 +175,7 @@ export function createTextInstance(
   text: string,
   rootContainerInstance: Container,
   hostContext: HostContext,
-  internalInstanceHandle: Object,
+  internalInstanceHandle: InternalInstanceHandle,
 ): TextInstance {
   if (__DEV__) {
     if (!hostContext.isInAParentText) {
@@ -239,9 +252,26 @@ export function getPublicInstance(instance: Instance): null | PublicInstance {
   return null;
 }
 
+function getPublicTextInstance(
+  textInstance: TextInstance,
+  internalInstanceHandle: InternalInstanceHandle,
+): PublicTextInstance {
+  if (textInstance.publicInstance == null) {
+    textInstance.publicInstance = createPublicTextInstance(
+      internalInstanceHandle,
+    );
+  }
+  return textInstance.publicInstance;
+}
+
 export function getPublicInstanceFromInternalInstanceHandle(
-  internalInstanceHandle: Object,
-): null | PublicInstance {
+  internalInstanceHandle: InternalInstanceHandle,
+): null | PublicInstance | PublicTextInstance {
+  if (internalInstanceHandle.tag === HostText) {
+    const textInstance: TextInstance = internalInstanceHandle.stateNode;
+    return getPublicTextInstance(textInstance, internalInstanceHandle);
+  }
+
   const instance: Instance = internalInstanceHandle.stateNode;
   return getPublicInstance(instance);
 }
@@ -321,7 +351,7 @@ export function cloneInstance(
   type: string,
   oldProps: Props,
   newProps: Props,
-  internalInstanceHandle: Object,
+  internalInstanceHandle: InternalInstanceHandle,
   keepChildren: boolean,
   recyclableInstance: null | Instance,
 ): Instance {
@@ -350,7 +380,7 @@ export function cloneHiddenInstance(
   instance: Instance,
   type: string,
   props: Props,
-  internalInstanceHandle: Object,
+  internalInstanceHandle: InternalInstanceHandle,
 ): Instance {
   const viewConfig = instance.canonical.viewConfig;
   const node = instance.node;
@@ -367,7 +397,7 @@ export function cloneHiddenInstance(
 export function cloneHiddenTextInstance(
   instance: Instance,
   text: string,
-  internalInstanceHandle: Object,
+  internalInstanceHandle: InternalInstanceHandle,
 ): TextInstance {
   throw new Error('Not yet implemented.');
 }
@@ -399,7 +429,9 @@ export function getInstanceFromNode(node: any): empty {
   throw new Error('Not yet implemented.');
 }
 
-export function beforeActiveInstanceBlur(internalInstanceHandle: Object) {
+export function beforeActiveInstanceBlur(
+  internalInstanceHandle: InternalInstanceHandle,
+) {
   // noop
 }
 
diff --git a/packages/react-native-renderer/src/ReactNativeTypes.js b/packages/react-native-renderer/src/ReactNativeTypes.js
@@ -215,6 +215,7 @@ export type ReactNativeType = {
 export opaque type Node = mixed;
 export opaque type InternalInstanceHandle = mixed;
 type PublicInstance = mixed;
+type PublicTextInstance = mixed;
 
 export type ReactFabricType = {
   findHostInstance_DEPRECATED<TElementType: ElementType>(
@@ -244,7 +245,7 @@ export type ReactFabricType = {
   ): ?Node,
   getPublicInstanceFromInternalInstanceHandle(
     internalInstanceHandle: InternalInstanceHandle,
-  ): PublicInstance,
+  ): PublicInstance | PublicTextInstance,
   ...
 };
 
diff --git a/packages/react-native-renderer/src/__mocks__/react-native/Libraries/ReactPrivate/ReactNativePrivateInterface.js b/packages/react-native-renderer/src/__mocks__/react-native/Libraries/ReactPrivate/ReactNativePrivateInterface.js
@@ -8,6 +8,7 @@
  */
 
 export opaque type PublicInstance = mixed;
+export opaque type PublicTextInstance = mixed;
 
 module.exports = {
   get BatchedBridge() {
@@ -55,4 +56,7 @@ module.exports = {
   get createPublicInstance() {
     return require('./createPublicInstance').default;
   },
+  get createPublicTextInstance() {
+    return require('./createPublicTextInstance').default;
+  },
 };
diff --git a/packages/react-native-renderer/src/__mocks__/react-native/Libraries/ReactPrivate/createPublicTextInstance.js b/packages/react-native-renderer/src/__mocks__/react-native/Libraries/ReactPrivate/createPublicTextInstance.js
@@ -0,0 +1,18 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow strict
+ */
+
+import type {PublicInstance} from './ReactNativePrivateInterface';
+
+export default function createPublicTextInstance(
+  internalInstanceHandle: mixed,
+): PublicInstance {
+  return {
+    __internalInstanceHandle: internalInstanceHandle,
+  };
+}
diff --git a/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js b/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js
@@ -12,11 +12,10 @@
 
 let React;
 let ReactFabric;
+let ReactNativePrivateInterface;
 let createReactNativeComponentClass;
 let StrictMode;
 let act;
-let getNativeTagFromPublicInstance;
-let getNodeFromPublicInstance;
 
 const DISPATCH_COMMAND_REQUIRES_HOST_COMPONENT =
   "Warning: dispatchCommand was called with a ref that isn't a " +
@@ -39,14 +38,10 @@ describe('ReactFabric', () => {
     React = require('react');
     StrictMode = React.StrictMode;
     ReactFabric = require('react-native-renderer/fabric');
+    ReactNativePrivateInterface = require('react-native/Libraries/ReactPrivate/ReactNativePrivateInterface');
     createReactNativeComponentClass =
       require('react-native/Libraries/ReactPrivate/ReactNativePrivateInterface')
         .ReactNativeViewConfigRegistry.register;
-    getNativeTagFromPublicInstance =
-      require('react-native/Libraries/ReactPrivate/ReactNativePrivateInterface').getNativeTagFromPublicInstance;
-    getNodeFromPublicInstance =
-      require('react-native/Libraries/ReactPrivate/ReactNativePrivateInterface').getNodeFromPublicInstance;
-
     act = require('internal-test-utils').act;
   });
 
@@ -937,7 +932,9 @@ describe('ReactFabric', () => {
         '\n    in RCTView (at **)' +
         '\n    in ContainsStrictModeChild (at **)',
     ]);
-    expect(match).toBe(getNativeTagFromPublicInstance(child));
+    expect(match).toBe(
+      ReactNativePrivateInterface.getNativeTagFromPublicInstance(child),
+    );
   });
 
   it('findNodeHandle should warn if passed a component that is inside StrictMode', async () => {
@@ -974,7 +971,9 @@ describe('ReactFabric', () => {
         '\n    in RCTView (at **)' +
         '\n    in IsInStrictMode (at **)',
     ]);
-    expect(match).toBe(getNativeTagFromPublicInstance(child));
+    expect(match).toBe(
+      ReactNativePrivateInterface.getNativeTagFromPublicInstance(child),
+    );
   });
 
   it('should no-op if calling sendAccessibilityEvent on unmounted refs', async () => {
@@ -1015,6 +1014,30 @@ describe('ReactFabric', () => {
       uiViewClassName: 'RCTView',
     }));
 
+    await act(() => {
+      ReactFabric.render(<View foo="test" />, 1);
+    });
+
+    const internalInstanceHandle =
+      nativeFabricUIManager.createNode.mock.calls[0][4];
+    expect(internalInstanceHandle).toEqual(expect.any(Object));
+
+    const expectedShadowNode =
+      nativeFabricUIManager.createNode.mock.results[0].value;
+    expect(expectedShadowNode).toEqual(expect.any(Object));
+
+    const node = ReactFabric.getNodeFromInternalInstanceHandle(
+      internalInstanceHandle,
+    );
+    expect(node).toBe(expectedShadowNode);
+  });
+
+  it('getPublicInstanceFromInternalInstanceHandle should provide public instances for HostComponent', async () => {
+    const View = createReactNativeComponentClass('RCTView', () => ({
+      validAttributes: {foo: true},
+      uiViewClassName: 'RCTView',
+    }));
+
     let viewRef;
     await act(() => {
       ReactFabric.render(
@@ -1028,11 +1051,55 @@ describe('ReactFabric', () => {
       );
     });
 
-    const expectedShadowNode =
-      nativeFabricUIManager.createNode.mock.results[0].value;
-    expect(expectedShadowNode).toEqual(expect.any(Object));
+    const internalInstanceHandle =
+      nativeFabricUIManager.createNode.mock.calls[0][4];
+    expect(internalInstanceHandle).toEqual(expect.any(Object));
 
-    const node = getNodeFromPublicInstance(viewRef);
-    expect(node).toBe(expectedShadowNode);
+    const publicInstance =
+      ReactFabric.getPublicInstanceFromInternalInstanceHandle(
+        internalInstanceHandle,
+      );
+    expect(publicInstance).toBe(viewRef);
+  });
+
+  it('getPublicInstanceFromInternalInstanceHandle should provide public instances for HostText', async () => {
+    jest.spyOn(ReactNativePrivateInterface, 'createPublicTextInstance');
+
+    const RCTText = createReactNativeComponentClass('RCTText', () => ({
+      validAttributes: {},
+      uiViewClassName: 'RCTText',
+    }));
+
+    await act(() => {
+      ReactFabric.render(<RCTText>Text content</RCTText>, 1);
+    });
+
+    // Access the internal instance handle used to create the text node.
+    const internalInstanceHandle =
+      nativeFabricUIManager.createNode.mock.calls[0][4];
+    expect(internalInstanceHandle).toEqual(expect.any(Object));
+
+    // Text public instances should be created lazily.
+    expect(
+      ReactNativePrivateInterface.createPublicTextInstance,
+    ).not.toHaveBeenCalled();
+
+    const publicInstance =
+      ReactFabric.getPublicInstanceFromInternalInstanceHandle(
+        internalInstanceHandle,
+      );
+
+    // We just requested the text public instance, so it should have been created at this point.
+    expect(
+      ReactNativePrivateInterface.createPublicTextInstance,
+    ).toHaveBeenCalledTimes(1);
+    expect(
+      ReactNativePrivateInterface.createPublicTextInstance,
+    ).toHaveBeenCalledWith(internalInstanceHandle);
+
+    const expectedPublicInstance =
+      ReactNativePrivateInterface.createPublicTextInstance.mock.results[0]
+        .value;
+    expect(publicInstance).toBe(expectedPublicInstance);
   });
 });
diff --git a/scripts/flow/react-native-host-hooks.js b/scripts/flow/react-native-host-hooks.js
@@ -145,6 +145,7 @@ declare module 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface'
     ...
   };
   declare export opaque type PublicInstance;
+  declare export opaque type PublicTextInstance;
   declare export function getNodeFromPublicInstance(
     publicInstance: PublicInstance,
   ): Object;
@@ -156,6 +157,9 @@ declare module 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface'
     viewConfig: __ViewConfig,
     internalInstanceHandle: mixed,
   ): PublicInstance;
+  declare export function createPublicTextInstance(
+    internalInstanceHandle: mixed,
+  ): PublicTextInstance;
 }
 
 declare module 'react-native/Libraries/ReactPrivate/ReactNativePrivateInitializeCore' {
