Skip to content

Commit b72ed69

Browse files
authored
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.
1 parent 25a8b97 commit b72ed69

File tree

7 files changed

+31
-51
lines changed

7 files changed

+31
-51
lines changed

packages/react-native-renderer/src/ReactFabric.js

+1-15
Original file line numberDiff line numberDiff line change
@@ -90,15 +90,6 @@ function findHostInstance_DEPRECATED<TElementType: ElementType>(
9090
hostInstance = findHostInstance(componentOrHandle);
9191
}
9292

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]
10293
return hostInstance;
10394
}
10495

@@ -146,12 +137,7 @@ function findNodeHandle(componentOrHandle: any): ?number {
146137
if (hostInstance == null) {
147138
return hostInstance;
148139
}
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+
155141
return hostInstance._nativeTag;
156142
}
157143

packages/react-native-renderer/src/ReactFabricHostConfig.js

+12-7
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ if (registerEventHandler) {
110110
/**
111111
* This is used for refs on host components.
112112
*/
113-
class ReactFabricHostComponent {
113+
class ReactFabricHostComponent implements NativeMethods {
114114
_nativeTag: number;
115115
viewConfig: ViewConfig;
116116
currentProps: Props;
@@ -215,10 +215,6 @@ class ReactFabricHostComponent {
215215
}
216216
}
217217

218-
// $FlowFixMe[class-object-subtyping] found when upgrading Flow
219-
// $FlowFixMe[method-unbinding] found when upgrading Flow
220-
(ReactFabricHostComponent.prototype: $ReadOnly<{...NativeMethods, ...}>);
221-
222218
export * from 'react-reconciler/src/ReactFiberHostConfigWithNoMutation';
223219
export * from 'react-reconciler/src/ReactFiberHostConfigWithNoHydration';
224220
export * from 'react-reconciler/src/ReactFiberHostConfigWithNoScopes';
@@ -342,8 +338,17 @@ export function getChildHostContext(
342338
}
343339
}
344340

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;
347352
}
348353

349354
export function prepareForCommit(containerInfo: Container): null | Object {

packages/react-native-renderer/src/ReactNativeFiberHostComponent.js

+1-5
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import {
3030
warnForStyleProps,
3131
} from './NativeMethodsMixinUtils';
3232

33-
class ReactNativeFiberHostComponent {
33+
class ReactNativeFiberHostComponent implements NativeMethods {
3434
_children: Array<Instance | number>;
3535
_nativeTag: number;
3636
_internalFiberInstanceHandleDEV: Object;
@@ -127,8 +127,4 @@ class ReactNativeFiberHostComponent {
127127
}
128128
}
129129

130-
// $FlowFixMe[class-object-subtyping] found when upgrading Flow
131-
// $FlowFixMe[method-unbinding] found when upgrading Flow
132-
(ReactNativeFiberHostComponent.prototype: $ReadOnly<{...NativeMethods, ...}>);
133-
134130
export default ReactNativeFiberHostComponent;

packages/react-native-renderer/src/ReactNativeHostConfig.js

+5
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,11 @@ export function getChildHostContext(
217217
}
218218

219219
export function getPublicInstance(instance: Instance): * {
220+
// $FlowExpectedError[prop-missing] For compatibility with Fabric
221+
if (instance.canonical) {
222+
return instance.canonical;
223+
}
224+
220225
return instance;
221226
}
222227

packages/react-native-renderer/src/ReactNativeRenderer.js

+1-13
Original file line numberDiff line numberDiff line change
@@ -89,15 +89,6 @@ function findHostInstance_DEPRECATED(
8989
hostInstance = findHostInstance(componentOrHandle);
9090
}
9191

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]
10192
return hostInstance;
10293
}
10394

@@ -145,10 +136,7 @@ function findNodeHandle(componentOrHandle: any): ?number {
145136
if (hostInstance == null) {
146137
return hostInstance;
147138
}
148-
if ((hostInstance: any).canonical) {
149-
// Fabric
150-
return (hostInstance: any).canonical._nativeTag;
151-
}
139+
152140
return hostInstance._nativeTag;
153141
}
154142

packages/react-native-renderer/src/ReactNativeTypes.js

+8-8
Original file line numberDiff line numberDiff line change
@@ -95,18 +95,18 @@ export type PartialViewConfig = $ReadOnly<{
9595
validAttributes?: PartialAttributeConfiguration,
9696
}>;
9797

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;
103103
measureLayout(
104104
relativeToNativeNode: number | ElementRef<HostComponent<mixed>>,
105105
onSuccess: MeasureLayoutOnSuccessCallback,
106106
onFail?: () => void,
107-
): void,
108-
setNativeProps(nativeProps: {...}): void,
109-
}>;
107+
): void;
108+
setNativeProps(nativeProps: {...}): void;
109+
}
110110

111111
export type HostComponent<T> = AbstractComponent<T, $ReadOnly<NativeMethods>>;
112112

packages/react-reconciler/src/ReactFiberReconciler.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ function findHostInstance(component: Object): PublicInstance | null {
175175
if (hostFiber === null) {
176176
return null;
177177
}
178-
return hostFiber.stateNode;
178+
return getPublicInstance(hostFiber.stateNode);
179179
}
180180

181181
function findHostInstanceWithWarning(
@@ -240,7 +240,7 @@ function findHostInstanceWithWarning(
240240
}
241241
}
242242
}
243-
return hostFiber.stateNode;
243+
return getPublicInstance(hostFiber.stateNode);
244244
}
245245
return findHostInstance(component);
246246
}
@@ -524,7 +524,7 @@ export function findHostInstanceWithNoPortals(
524524
if (hostFiber === null) {
525525
return null;
526526
}
527-
return hostFiber.stateNode;
527+
return getPublicInstance(hostFiber.stateNode);
528528
}
529529

530530
let shouldErrorImpl: Fiber => ?boolean = fiber => null;

0 commit comments

Comments
 (0)