Skip to content

Commit

Permalink
Guard against unmounted components when accessing public instances on…
Browse files Browse the repository at this point in the history
… Fabric (#27687)

## Summary

This fixes an error in `getPublicInstanceFromInstanceHandle` where we
throw an error when trying to access the public instance from the fiber
of an unmounted component. This shouldn't throw but return `null`
instead.

## How did you test this change?

Updated unit tests.
Before:
<img width="969" alt="Screenshot 2023-11-10 at 15 26 14"
src="https://github.com/facebook/react/assets/117921/ea161616-2775-4fab-8d74-da4bef48d09a">

After:
<img width="1148" alt="Screenshot 2023-11-10 at 15 28 37"
src="https://github.com/facebook/react/assets/117921/db18b918-b6b6-4925-9cfc-3b4b2f3ab92d">

DiffTrain build for commit 6b3834a.
  • Loading branch information
rubennorte committed Nov 10, 2023
1 parent 372f016 commit 6dbeb12
Show file tree
Hide file tree
Showing 11 changed files with 51 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25474,7 +25474,7 @@ if (__DEV__) {
return root;
}

var ReactVersion = "18.3.0-canary-6a7f3aa85-20231110";
var ReactVersion = "18.3.0-canary-6b3834a45-20231110";

// Might add PROFILE later.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9083,7 +9083,7 @@ var devToolsConfig$jscomp$inline_1033 = {
throw Error("TestRenderer does not support findFiberByHostInstance()");
},
bundleType: 0,
version: "18.3.0-canary-6a7f3aa85-20231110",
version: "18.3.0-canary-6b3834a45-20231110",
rendererPackageName: "react-test-renderer"
};
var internals$jscomp$inline_1226 = {
Expand Down Expand Up @@ -9114,7 +9114,7 @@ var internals$jscomp$inline_1226 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-canary-6a7f3aa85-20231110"
reconcilerVersion: "18.3.0-canary-6b3834a45-20231110"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_1227 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9509,7 +9509,7 @@ var devToolsConfig$jscomp$inline_1075 = {
throw Error("TestRenderer does not support findFiberByHostInstance()");
},
bundleType: 0,
version: "18.3.0-canary-6a7f3aa85-20231110",
version: "18.3.0-canary-6b3834a45-20231110",
rendererPackageName: "react-test-renderer"
};
var internals$jscomp$inline_1267 = {
Expand Down Expand Up @@ -9540,7 +9540,7 @@ var internals$jscomp$inline_1267 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-canary-6a7f3aa85-20231110"
reconcilerVersion: "18.3.0-canary-6b3834a45-20231110"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_1268 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ if (__DEV__) {
) {
__REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStart(new Error());
}
var ReactVersion = "18.3.0-canary-6a7f3aa85-20231110";
var ReactVersion = "18.3.0-canary-6b3834a45-20231110";

// ATTENTION
// When adding new symbols to this file,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -580,4 +580,4 @@ exports.useSyncExternalStore = function (
exports.useTransition = function () {
return ReactCurrentDispatcher.current.useTransition();
};
exports.version = "18.3.0-canary-6a7f3aa85-20231110";
exports.version = "18.3.0-canary-6b3834a45-20231110";
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,7 @@ exports.useSyncExternalStore = function (
exports.useTransition = function () {
return ReactCurrentDispatcher.current.useTransition();
};
exports.version = "18.3.0-canary-6a7f3aa85-20231110";
exports.version = "18.3.0-canary-6b3834a45-20231110";
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
"function" ===
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
6a7f3aa858b3a8670d6a4861e30f248b335e55bd
6b3834a45b585e4340734139841ae81dc1b1a75d
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* @noflow
* @nolint
* @preventMunge
* @generated SignedSource<<82de3c20128bc9cf9a3e000de26414a9>>
* @generated SignedSource<<35562a820289897947536c61435e3dcd>>
*/

"use strict";
Expand Down Expand Up @@ -5111,13 +5111,20 @@ to return true:wantsResponderID| |
function getPublicInstanceFromInternalInstanceHandle(
internalInstanceHandle
) {
var instance = internalInstanceHandle.stateNode; // React resets all the fields in the fiber when the component is unmounted
// to prevent memory leaks.

if (instance == null) {
return null;
}

if (internalInstanceHandle.tag === HostText) {
var textInstance = internalInstanceHandle.stateNode;
var textInstance = instance;
return getPublicTextInstance(textInstance, internalInstanceHandle);
}

var instance = internalInstanceHandle.stateNode;
return getPublicInstance(instance);
var elementInstance = internalInstanceHandle.stateNode;
return getPublicInstance(elementInstance);
}
function shouldSetTextContent(type, props) {
// TODO (bvaughn) Revisit this decision.
Expand Down Expand Up @@ -27805,7 +27812,7 @@ to return true:wantsResponderID| |
return root;
}

var ReactVersion = "18.3.0-canary-8403dc9a";
var ReactVersion = "18.3.0-canary-884b3517";

function createPortal$1(
children,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* @noflow
* @nolint
* @preventMunge
* @generated SignedSource<<215b578b886f3eeea52b84f72bb15ec6>>
* @generated SignedSource<<c45396dc1efd10a6033e0b1852b0fe7f>>
*/

"use strict";
Expand Down Expand Up @@ -9541,7 +9541,7 @@ var roots = new Map(),
devToolsConfig$jscomp$inline_1048 = {
findFiberByHostInstance: getInstanceFromNode,
bundleType: 0,
version: "18.3.0-canary-1b86ce9f",
version: "18.3.0-canary-8f9ce9e2",
rendererPackageName: "react-native-renderer",
rendererConfig: {
getInspectorDataForInstance: getInspectorDataForInstance,
Expand Down Expand Up @@ -9584,7 +9584,7 @@ var internals$jscomp$inline_1290 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-canary-1b86ce9f"
reconcilerVersion: "18.3.0-canary-8f9ce9e2"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_1291 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down Expand Up @@ -9643,16 +9643,17 @@ exports.getNodeFromInternalInstanceHandle = function (internalInstanceHandle) {
exports.getPublicInstanceFromInternalInstanceHandle = function (
internalInstanceHandle
) {
if (6 === internalInstanceHandle.tag) {
var textInstance = internalInstanceHandle.stateNode;
null == textInstance.publicInstance &&
(textInstance.publicInstance =
ReactNativePrivateInterface.createPublicTextInstance(
internalInstanceHandle
));
return textInstance.publicInstance;
}
return getPublicInstance(internalInstanceHandle.stateNode);
var instance = internalInstanceHandle.stateNode;
return null == instance
? null
: 6 === internalInstanceHandle.tag
? (null == instance.publicInstance &&
(instance.publicInstance =
ReactNativePrivateInterface.createPublicTextInstance(
internalInstanceHandle
)),
instance.publicInstance)
: getPublicInstance(internalInstanceHandle.stateNode);
};
exports.render = function (element, containerTag, callback, concurrentRoot) {
var root = roots.get(containerTag);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* @noflow
* @nolint
* @preventMunge
* @generated SignedSource<<443d6fa46938bdd4041f48305e367a13>>
* @generated SignedSource<<cb300ed80629ca9a73c08e9fcad07bd0>>
*/

"use strict";
Expand Down Expand Up @@ -10243,7 +10243,7 @@ var roots = new Map(),
devToolsConfig$jscomp$inline_1126 = {
findFiberByHostInstance: getInstanceFromNode,
bundleType: 0,
version: "18.3.0-canary-5ad064e7",
version: "18.3.0-canary-8a2d4f06",
rendererPackageName: "react-native-renderer",
rendererConfig: {
getInspectorDataForInstance: getInspectorDataForInstance,
Expand Down Expand Up @@ -10299,7 +10299,7 @@ var roots = new Map(),
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-canary-5ad064e7"
reconcilerVersion: "18.3.0-canary-8a2d4f06"
});
exports.createPortal = function (children, containerTag) {
return createPortal$1(
Expand Down Expand Up @@ -10345,16 +10345,17 @@ exports.getNodeFromInternalInstanceHandle = function (internalInstanceHandle) {
exports.getPublicInstanceFromInternalInstanceHandle = function (
internalInstanceHandle
) {
if (6 === internalInstanceHandle.tag) {
var textInstance = internalInstanceHandle.stateNode;
null == textInstance.publicInstance &&
(textInstance.publicInstance =
ReactNativePrivateInterface.createPublicTextInstance(
internalInstanceHandle
));
return textInstance.publicInstance;
}
return getPublicInstance(internalInstanceHandle.stateNode);
var instance = internalInstanceHandle.stateNode;
return null == instance
? null
: 6 === internalInstanceHandle.tag
? (null == instance.publicInstance &&
(instance.publicInstance =
ReactNativePrivateInterface.createPublicTextInstance(
internalInstanceHandle
)),
instance.publicInstance)
: getPublicInstance(internalInstanceHandle.stateNode);
};
exports.render = function (element, containerTag, callback, concurrentRoot) {
var root = roots.get(containerTag);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* @noformat
* @flow strict
* @nolint
* @generated SignedSource<<652b117c94307244bcf5e4af18928903>>
* @generated SignedSource<<1836a1b6639552dce12199ef2c85f63d>>
*/

import type {ElementRef, ElementType, Element, AbstractComponent} from 'react';
Expand Down Expand Up @@ -247,7 +247,7 @@ export type ReactFabricType = {
): ?Node,
getPublicInstanceFromInternalInstanceHandle(
internalInstanceHandle: InternalInstanceHandle,
): PublicInstance | PublicTextInstance,
): PublicInstance | PublicTextInstance | null,
...
};

Expand Down

0 comments on commit 6dbeb12

Please sign in to comment.