Fixed incorrect value returned as public instance from reconciler (#26283)

rubennorte · web-flow · commit b72ed698fb5f · 2023-03-03T09:38:08.000Z
## Summary

A few methods in `ReactFiberReconciler` are supposed to return
`PublicInstance` values, but they return the `stateNode` from the fiber
directly. This assumes that the `stateNode` always matches the public
instance (which it does on Web) but that's not the case in React Native,
where the public instance is a field in that object.

This hasn't caused issues because everywhere where we use that method in
React Native we actually extract the real public instance from this
"fake" public instance.

This PR fixes the inconsistency and cleans up some code.

## How did you test this change?

Existing tests.
diff --git a/packages/react-native-renderer/src/ReactFabric.js b/packages/react-native-renderer/src/ReactFabric.js
@@ -90,15 +90,6 @@ function findHostInstance_DEPRECATED<TElementType: ElementType>(
     hostInstance = findHostInstance(componentOrHandle);
   }
 
-  if (hostInstance == null) {
-    return hostInstance;
-  }
-  if ((hostInstance: any).canonical) {
-    // Fabric
-    return (hostInstance: any).canonical;
-  }
-  // $FlowFixMe[incompatible-return]
-  // $FlowFixMe[incompatible-exact]
   return hostInstance;
 }
 
@@ -146,12 +137,7 @@ function findNodeHandle(componentOrHandle: any): ?number {
   if (hostInstance == null) {
     return hostInstance;
   }
-  // TODO: the code is right but the types here are wrong.
-  // https://github.com/facebook/react/pull/12863
-  if ((hostInstance: any).canonical) {
-    // Fabric
-    return (hostInstance: any).canonical._nativeTag;
-  }
+
   return hostInstance._nativeTag;
 }
 
diff --git a/packages/react-native-renderer/src/ReactFabricHostConfig.js b/packages/react-native-renderer/src/ReactFabricHostConfig.js
@@ -110,7 +110,7 @@ if (registerEventHandler) {
 /**
  * This is used for refs on host components.
  */
-class ReactFabricHostComponent {
+class ReactFabricHostComponent implements NativeMethods {
   _nativeTag: number;
   viewConfig: ViewConfig;
   currentProps: Props;
@@ -215,10 +215,6 @@ class ReactFabricHostComponent {
   }
 }
 
-// $FlowFixMe[class-object-subtyping] found when upgrading Flow
-// $FlowFixMe[method-unbinding] found when upgrading Flow
-(ReactFabricHostComponent.prototype: $ReadOnly<{...NativeMethods, ...}>);
-
 export * from 'react-reconciler/src/ReactFiberHostConfigWithNoMutation';
 export * from 'react-reconciler/src/ReactFiberHostConfigWithNoHydration';
 export * from 'react-reconciler/src/ReactFiberHostConfigWithNoScopes';
@@ -342,8 +338,17 @@ export function getChildHostContext(
   }
 }
 
-export function getPublicInstance(instance: Instance): * {
-  return instance.canonical;
+export function getPublicInstance(instance: Instance): null | PublicInstance {
+  if (instance.canonical) {
+    return instance.canonical;
+  }
+
+  // For compatibility with Paper
+  if (instance._nativeTag != null) {
+    return instance;
+  }
+
+  return null;
 }
 
 export function prepareForCommit(containerInfo: Container): null | Object {
diff --git a/packages/react-native-renderer/src/ReactNativeFiberHostComponent.js b/packages/react-native-renderer/src/ReactNativeFiberHostComponent.js
@@ -30,7 +30,7 @@ import {
   warnForStyleProps,
 } from './NativeMethodsMixinUtils';
 
-class ReactNativeFiberHostComponent {
+class ReactNativeFiberHostComponent implements NativeMethods {
   _children: Array<Instance | number>;
   _nativeTag: number;
   _internalFiberInstanceHandleDEV: Object;
@@ -127,8 +127,4 @@ class ReactNativeFiberHostComponent {
   }
 }
 
-// $FlowFixMe[class-object-subtyping] found when upgrading Flow
-// $FlowFixMe[method-unbinding] found when upgrading Flow
-(ReactNativeFiberHostComponent.prototype: $ReadOnly<{...NativeMethods, ...}>);
-
 export default ReactNativeFiberHostComponent;
diff --git a/packages/react-native-renderer/src/ReactNativeHostConfig.js b/packages/react-native-renderer/src/ReactNativeHostConfig.js
@@ -217,6 +217,11 @@ export function getChildHostContext(
 }
 
 export function getPublicInstance(instance: Instance): * {
+  // $FlowExpectedError[prop-missing] For compatibility with Fabric
+  if (instance.canonical) {
+    return instance.canonical;
+  }
+
   return instance;
 }
 
diff --git a/packages/react-native-renderer/src/ReactNativeRenderer.js b/packages/react-native-renderer/src/ReactNativeRenderer.js
@@ -89,15 +89,6 @@ function findHostInstance_DEPRECATED(
     hostInstance = findHostInstance(componentOrHandle);
   }
 
-  if (hostInstance == null) {
-    return hostInstance;
-  }
-  if ((hostInstance: any).canonical) {
-    // Fabric
-    return (hostInstance: any).canonical;
-  }
-  // $FlowFixMe[incompatible-return]
-  // $FlowFixMe[incompatible-exact]
   return hostInstance;
 }
 
@@ -145,10 +136,7 @@ function findNodeHandle(componentOrHandle: any): ?number {
   if (hostInstance == null) {
     return hostInstance;
   }
-  if ((hostInstance: any).canonical) {
-    // Fabric
-    return (hostInstance: any).canonical._nativeTag;
-  }
+
   return hostInstance._nativeTag;
 }
 
diff --git a/packages/react-native-renderer/src/ReactNativeTypes.js b/packages/react-native-renderer/src/ReactNativeTypes.js
@@ -95,18 +95,18 @@ export type PartialViewConfig = $ReadOnly<{
   validAttributes?: PartialAttributeConfiguration,
 }>;
 
-export type NativeMethods = $ReadOnly<{
-  blur(): void,
-  focus(): void,
-  measure(callback: MeasureOnSuccessCallback): void,
-  measureInWindow(callback: MeasureInWindowOnSuccessCallback): void,
+export interface NativeMethods {
+  blur(): void;
+  focus(): void;
+  measure(callback: MeasureOnSuccessCallback): void;
+  measureInWindow(callback: MeasureInWindowOnSuccessCallback): void;
   measureLayout(
     relativeToNativeNode: number | ElementRef<HostComponent<mixed>>,
     onSuccess: MeasureLayoutOnSuccessCallback,
     onFail?: () => void,
-  ): void,
-  setNativeProps(nativeProps: {...}): void,
-}>;
+  ): void;
+  setNativeProps(nativeProps: {...}): void;
+}
 
 export type HostComponent<T> = AbstractComponent<T, $ReadOnly<NativeMethods>>;
 
diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js
@@ -175,7 +175,7 @@ function findHostInstance(component: Object): PublicInstance | null {
   if (hostFiber === null) {
     return null;
   }
-  return hostFiber.stateNode;
+  return getPublicInstance(hostFiber.stateNode);
 }
 
 function findHostInstanceWithWarning(
@@ -240,7 +240,7 @@ function findHostInstanceWithWarning(
         }
       }
     }
-    return hostFiber.stateNode;
+    return getPublicInstance(hostFiber.stateNode);
   }
   return findHostInstance(component);
 }
@@ -524,7 +524,7 @@ export function findHostInstanceWithNoPortals(
   if (hostFiber === null) {
     return null;
   }
-  return hostFiber.stateNode;
+  return getPublicInstance(hostFiber.stateNode);
 }
 
 let shouldErrorImpl: Fiber => ?boolean = fiber => null;
