mirror of
https://github.com/facebook/react-native.git
synced 2025-11-01 09:14:26 +00:00
Force accesses to ReactFabric to always go through proxy (#45822)
Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/45822 Changelog: [internal] ## Context `react-native/Libraries/Renderer/shims/ReactFabric` is the default module to load the RN renderer and loading it has side-effects (among other things it sets `global.RN$stopSurface`. We introduced a proxy module (`RendererProxy`) so we could use dependency injection to overwrite the renderer module with a custom implementation (the original goal was to be able to use a renderer version that didn't pull paper and only used Fabric). Unfortunately, using both the proxy and the module directly in some places leads to race conditions setting `global.RN$stopSurface`, which causes some screens to be rendered with one renderer and unmounted/disposed with a different one (because we accessed `ReactFabric` later and set `RN$stopSurface` from a different renderer implementation). When this happens, the unmount request in the other renderer is a no-op because no surface was renderer in it. This leads to surfaces not being disposed. ## Changes This modifies the proxy to add additional functions and modifies all other modules in the package to make sure that all the accesses to the renderer go through the proxy. Reviewed By: sammy-SC Differential Revision: D60452544 fbshipit-source-id: 1b17a95ed9b1c529718f22983dde1f00f1b2adae
This commit is contained in:
committed by
Facebook GitHub Bot
parent
f8f85be8b5
commit
c3dbebff5a
Vendored
+1
-1
@@ -20,7 +20,7 @@ import type {
|
||||
import type {ElementRef} from 'react';
|
||||
|
||||
import TextInputState from '../../Components/TextInput/TextInputState';
|
||||
import {getNodeFromInternalInstanceHandle} from '../../Renderer/shims/ReactFabric';
|
||||
import {getNodeFromInternalInstanceHandle} from '../../ReactNative/RendererProxy';
|
||||
import {getFabricUIManager} from '../FabricUIManager';
|
||||
import {create} from './ReactNativeAttributePayload';
|
||||
import warnForStyleProps from './warnForStyleProps';
|
||||
|
||||
Vendored
+5
-5
@@ -15,7 +15,7 @@
|
||||
|
||||
import type ReactNativeElement from '../../../src/private/webapis/dom/nodes/ReactNativeElement';
|
||||
import type ReadOnlyText from '../../../src/private/webapis/dom/nodes/ReadOnlyText';
|
||||
import typeof ReactFabricType from '../../Renderer/shims/ReactFabric';
|
||||
import typeof * as RendererProxyT from '../../ReactNative/RendererProxy';
|
||||
import type {
|
||||
InternalInstanceHandle,
|
||||
Node,
|
||||
@@ -32,7 +32,7 @@ let PublicInstanceClass:
|
||||
let ReadOnlyTextClass: Class<ReadOnlyText>;
|
||||
|
||||
// Lazy loaded to avoid evaluating the module when using the legacy renderer.
|
||||
let ReactFabric: ReactFabricType;
|
||||
let RendererProxy: RendererProxyT;
|
||||
|
||||
export function createPublicInstance(
|
||||
tag: number,
|
||||
@@ -78,10 +78,10 @@ export function getNodeFromPublicInstance(
|
||||
return null;
|
||||
}
|
||||
|
||||
if (ReactFabric == null) {
|
||||
ReactFabric = require('../../Renderer/shims/ReactFabric');
|
||||
if (RendererProxy == null) {
|
||||
RendererProxy = require('../../ReactNative/RendererProxy');
|
||||
}
|
||||
return ReactFabric.getNodeFromInternalInstanceHandle(
|
||||
return RendererProxy.getNodeFromInternalInstanceHandle(
|
||||
publicInstance.__internalInstanceHandle,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -8,7 +8,11 @@
|
||||
* @flow strict-local
|
||||
*/
|
||||
|
||||
import type {HostComponent} from '../Renderer/shims/ReactNativeTypes';
|
||||
import type {
|
||||
HostComponent,
|
||||
InternalInstanceHandle,
|
||||
Node,
|
||||
} from '../Renderer/shims/ReactNativeTypes';
|
||||
import type ReactFabricHostComponent from './ReactFabricPublicInstance/ReactFabricHostComponent';
|
||||
import type {Element, ElementRef, ElementType} from 'react';
|
||||
|
||||
@@ -139,3 +143,21 @@ export function isChildPublicInstance(
|
||||
childInstance,
|
||||
);
|
||||
}
|
||||
|
||||
export function getNodeFromInternalInstanceHandle(
|
||||
internalInstanceHandle: InternalInstanceHandle,
|
||||
): ?Node {
|
||||
// This is only available in Fabric
|
||||
return require('../Renderer/shims/ReactFabric').getNodeFromInternalInstanceHandle(
|
||||
internalInstanceHandle,
|
||||
);
|
||||
}
|
||||
|
||||
export function getPublicInstanceFromInternalInstanceHandle(
|
||||
internalInstanceHandle: InternalInstanceHandle,
|
||||
): mixed /*PublicInstance | PublicTextInstance | null*/ {
|
||||
// This is only available in Fabric
|
||||
return require('../Renderer/shims/ReactFabric').getPublicInstanceFromInternalInstanceHandle(
|
||||
internalInstanceHandle,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -7548,6 +7548,12 @@ declare export function isChildPublicInstance(
|
||||
parentInstance: ReactFabricHostComponent | HostComponent<mixed>,
|
||||
childInstance: ReactFabricHostComponent | HostComponent<mixed>
|
||||
): boolean;
|
||||
declare export function getNodeFromInternalInstanceHandle(
|
||||
internalInstanceHandle: InternalInstanceHandle
|
||||
): ?Node;
|
||||
declare export function getPublicInstanceFromInternalInstanceHandle(
|
||||
internalInstanceHandle: InternalInstanceHandle
|
||||
): mixed;
|
||||
"
|
||||
`;
|
||||
|
||||
|
||||
@@ -304,8 +304,10 @@ function setInstanceHandle(
|
||||
export function getShadowNode(node: ReadOnlyNode): ?ShadowNode {
|
||||
// Lazy import Fabric here to avoid DOM Node APIs classes from having side-effects.
|
||||
// With a static import we can't use these classes for Paper-only variants.
|
||||
const ReactFabric = require('../../../../../Libraries/Renderer/shims/ReactFabric');
|
||||
return ReactFabric.getNodeFromInternalInstanceHandle(getInstanceHandle(node));
|
||||
const RendererProxy = require('../../../../../Libraries/ReactNative/RendererProxy');
|
||||
return RendererProxy.getNodeFromInternalInstanceHandle(
|
||||
getInstanceHandle(node),
|
||||
);
|
||||
}
|
||||
|
||||
export function getChildNodes(
|
||||
@@ -349,9 +351,9 @@ export function getPublicInstanceFromInternalInstanceHandle(
|
||||
): ?ReadOnlyNode {
|
||||
// Lazy import Fabric here to avoid DOM Node APIs classes from having side-effects.
|
||||
// With a static import we can't use these classes for Paper-only variants.
|
||||
const ReactFabric = require('../../../../../Libraries/Renderer/shims/ReactFabric');
|
||||
const RendererProxy = require('../../../../../Libraries/ReactNative/RendererProxy');
|
||||
const mixedPublicInstance =
|
||||
ReactFabric.getPublicInstanceFromInternalInstanceHandle(instanceHandle);
|
||||
RendererProxy.getPublicInstanceFromInternalInstanceHandle(instanceHandle);
|
||||
// $FlowExpectedError[incompatible-return] React defines public instances as "mixed" because it can't access the definition from React Native.
|
||||
return mixedPublicInstance;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user