@samitouri / QOS-React / commits / b72ed698fb

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

## 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.

Rubén Norte committed Mar 3, 2023 at 09:38 UTC b72ed698fb5f15e0c33c5017422b8428b01ff12a
7 files changed +31 -51
packages/react-native-renderer/src/ReactFabric.js
+1 -15
@@ -90,15 +90,6 @@ function findHostInstance_DEPRECATED<TElementType: ElementType>(
90 hostInstance = findHostInstance(componentOrHandle);
91 }
92
93 - if (hostInstance == null) {
94 - return hostInstance;
95 - }
96 - if ((hostInstance: any).canonical) {
97 - // Fabric
98 - return (hostInstance: any).canonical;
99 - }
100 - // $FlowFixMe[incompatible-return]
101 - // $FlowFixMe[incompatible-exact]
93 return hostInstance;
94 }
95
@@ -146,12 +137,7 @@ function findNodeHandle(componentOrHandle: any): ?number {
137 if (hostInstance == null) {
138 return hostInstance;
139 }
149 - // TODO: the code is right but the types here are wrong.
150 - // https://github.com/facebook/react/pull/12863
151 - if ((hostInstance: any).canonical) {
152 - // Fabric
153 - return (hostInstance: any).canonical._nativeTag;
154 - }
140 +
141 return hostInstance._nativeTag;
142 }
143
packages/react-native-renderer/src/ReactFabricHostConfig.js
+12 -7
@@ -110,7 +110,7 @@ if (registerEventHandler) {
110 /**
111 * This is used for refs on host components.
112 */
113 -class ReactFabricHostComponent {
113 +class ReactFabricHostComponent implements NativeMethods {
114 _nativeTag: number;
115 viewConfig: ViewConfig;
116 currentProps: Props;
@@ -215,10 +215,6 @@ class ReactFabricHostComponent {
215 }
216 }
217
218 -// $FlowFixMe[class-object-subtyping] found when upgrading Flow
219 -// $FlowFixMe[method-unbinding] found when upgrading Flow
220 -(ReactFabricHostComponent.prototype: $ReadOnly<{...NativeMethods, ...}>);
221 -
218 export * from 'react-reconciler/src/ReactFiberHostConfigWithNoMutation';
219 export * from 'react-reconciler/src/ReactFiberHostConfigWithNoHydration';
220 export * from 'react-reconciler/src/ReactFiberHostConfigWithNoScopes';
@@ -342,8 +338,17 @@ export function getChildHostContext(
338 }
339 }
340
345 -export function getPublicInstance(instance: Instance): * {
346 - return instance.canonical;
341 +export function getPublicInstance(instance: Instance): null | PublicInstance {
342 + if (instance.canonical) {
343 + return instance.canonical;
344 + }
345 +
346 + // For compatibility with Paper
347 + if (instance._nativeTag != null) {
348 + return instance;
349 + }
350 +
351 + return null;
352 }
353
354 export function prepareForCommit(containerInfo: Container): null | Object {
packages/react-native-renderer/src/ReactNativeFiberHostComponent.js
+1 -5
@@ -30,7 +30,7 @@ import {
30 warnForStyleProps,
31 } from './NativeMethodsMixinUtils';
32
33 -class ReactNativeFiberHostComponent {
33 +class ReactNativeFiberHostComponent implements NativeMethods {
34 _children: Array<Instance | number>;
35 _nativeTag: number;
36 _internalFiberInstanceHandleDEV: Object;
@@ -127,8 +127,4 @@ class ReactNativeFiberHostComponent {
127 }
128 }
129
130 -// $FlowFixMe[class-object-subtyping] found when upgrading Flow
131 -// $FlowFixMe[method-unbinding] found when upgrading Flow
132 -(ReactNativeFiberHostComponent.prototype: $ReadOnly<{...NativeMethods, ...}>);
133 -
130 export default ReactNativeFiberHostComponent;
packages/react-native-renderer/src/ReactNativeHostConfig.js
+5
@@ -217,6 +217,11 @@ export function getChildHostContext(
217 }
218
219 export function getPublicInstance(instance: Instance): * {
220 + // $FlowExpectedError[prop-missing] For compatibility with Fabric
221 + if (instance.canonical) {
222 + return instance.canonical;
223 + }
224 +
225 return instance;
226 }
227
packages/react-native-renderer/src/ReactNativeRenderer.js
+1 -13
@@ -89,15 +89,6 @@ function findHostInstance_DEPRECATED(
89 hostInstance = findHostInstance(componentOrHandle);
90 }
91
92 - if (hostInstance == null) {
93 - return hostInstance;
94 - }
95 - if ((hostInstance: any).canonical) {
96 - // Fabric
97 - return (hostInstance: any).canonical;
98 - }
99 - // $FlowFixMe[incompatible-return]
100 - // $FlowFixMe[incompatible-exact]
92 return hostInstance;
93 }
94
@@ -145,10 +136,7 @@ function findNodeHandle(componentOrHandle: any): ?number {
136 if (hostInstance == null) {
137 return hostInstance;
138 }
148 - if ((hostInstance: any).canonical) {
149 - // Fabric
150 - return (hostInstance: any).canonical._nativeTag;
151 - }
139 +
140 return hostInstance._nativeTag;
141 }
142
packages/react-native-renderer/src/ReactNativeTypes.js
+8 -8
@@ -95,18 +95,18 @@ export type PartialViewConfig = $ReadOnly<{
95 validAttributes?: PartialAttributeConfiguration,
96 }>;
97
98 -export type NativeMethods = $ReadOnly<{
99 - blur(): void,
100 - focus(): void,
101 - measure(callback: MeasureOnSuccessCallback): void,
102 - measureInWindow(callback: MeasureInWindowOnSuccessCallback): void,
98 +export interface NativeMethods {
99 + blur(): void;
100 + focus(): void;
101 + measure(callback: MeasureOnSuccessCallback): void;
102 + measureInWindow(callback: MeasureInWindowOnSuccessCallback): void;
103 measureLayout(
104 relativeToNativeNode: number | ElementRef<HostComponent<mixed>>,
105 onSuccess: MeasureLayoutOnSuccessCallback,
106 onFail?: () => void,
107 - ): void,
108 - setNativeProps(nativeProps: {...}): void,
109 -}>;
107 + ): void;
108 + setNativeProps(nativeProps: {...}): void;
109 +}
110
111 export type HostComponent<T> = AbstractComponent<T, $ReadOnly<NativeMethods>>;
112
packages/react-reconciler/src/ReactFiberReconciler.js
+3 -3
@@ -175,7 +175,7 @@ function findHostInstance(component: Object): PublicInstance | null {
175 if (hostFiber === null) {
176 return null;
177 }
178 - return hostFiber.stateNode;
178 + return getPublicInstance(hostFiber.stateNode);
179 }
180
181 function findHostInstanceWithWarning(
@@ -240,7 +240,7 @@ function findHostInstanceWithWarning(
240 }
241 }
242 }
243 - return hostFiber.stateNode;
243 + return getPublicInstance(hostFiber.stateNode);
244 }
245 return findHostInstance(component);
246 }
@@ -524,7 +524,7 @@ export function findHostInstanceWithNoPortals(
524 if (hostFiber === null) {
525 return null;
526 }
527 - return hostFiber.stateNode;
527 + return getPublicInstance(hostFiber.stateNode);
528 }
529
530 let shouldErrorImpl: Fiber => ?boolean = fiber => null;