Skip to content

Commit effe6b5

Browse files
yungstersyungsters
committed
Fix Ref Lifecycles in Hidden Subtrees (#31379)
## Summary We're seeing certain situations in React Native development where ref callbacks in `<Activity mode="hidden">` are sometimes invoked exactly once with `null` without ever being called with a "current" value. This violates the contract for refs because refs are expected to always attach before detach (and to always eventually detach after attach). This is *particularly* bad for refs that return cleanup functions, because refs that return cleanup functions expect to never be invoked with `null`. This bug causes such refs to be invoked with `null` (because since `safelyAttachRef` was never called, `safelyDetachRef` thinks the ref does not return a cleanup function and invokes it with `null`). This fix makes use of `offscreenSubtreeWasHidden` in `commitDeletionEffectsOnFiber`, similar to how ec52a56 did this for `commitDeletionEffectsOnFiber`. ## How did you test this change? We were able to isolate the repro steps to isolate the React Native experimental changes. However, the repro steps depend on Fast Refresh. ``` function callbackRef(current) { // Called once with `current` as null, upon triggering Fast Refresh. } <Activity mode="hidden"> <View ref={callbackRef} />; </Activity> ``` Ideally, we would have a unit test that verifies this behavior without Fast Refresh. (We have evidence that this bug occurs without Fast Refresh in real product implementations. However, we have not successfully deduced the root cause, yet.) This PR currently includes a unit test that reproduces the Fast Refresh scenario, which is also demonstrated in this CodeSandbox: https://codesandbox.io/p/sandbox/hungry-darkness-33wxy7 Verified unit tests pass: ``` $ yarn $ yarn test # Run with `-r=www-classic` for `enableScopeAPI` tests. $ yarn test -r=www-classic ``` Verified on the internal React Native development branch that the bug no longer repros. --------- Co-authored-by: Rick Hanlon <[email protected]> DiffTrain build for [ea3ac58](ea3ac58)
1 parent 84a3f07 commit effe6b5

34 files changed

+476
-314
lines changed

‎compiled/facebook-www/REVISION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
0bc30748730063e561d87a24a4617526fdd38349
1+
ea3ac586693014e882655728fc8396ecb1d6cf6e

‎compiled/facebook-www/REVISION_TRANSFORMS

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
0bc30748730063e561d87a24a4617526fdd38349
1+
ea3ac586693014e882655728fc8396ecb1d6cf6e

‎compiled/facebook-www/React-dev.classic.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2021,7 +2021,7 @@ __DEV__ &&
20212021
exports.useTransition = function () {
20222022
return resolveDispatcher().useTransition();
20232023
};
2024-
exports.version = "19.0.0-www-classic-0bc30748-20241028";
2024+
exports.version = "19.0.0-www-classic-ea3ac586-20241031";
20252025
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
20262026
"function" ===
20272027
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

‎compiled/facebook-www/React-dev.modern.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2001,7 +2001,7 @@ __DEV__ &&
20012001
exports.useTransition = function () {
20022002
return resolveDispatcher().useTransition();
20032003
};
2004-
exports.version = "19.0.0-www-modern-0bc30748-20241028";
2004+
exports.version = "19.0.0-www-modern-ea3ac586-20241031";
20052005
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
20062006
"function" ===
20072007
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

‎compiled/facebook-www/React-prod.classic.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -824,4 +824,4 @@ exports.useSyncExternalStore = function (
824824
exports.useTransition = function () {
825825
return ReactSharedInternals.H.useTransition();
826826
};
827-
exports.version = "19.0.0-www-classic-0bc30748-20241028";
827+
exports.version = "19.0.0-www-classic-ea3ac586-20241031";

‎compiled/facebook-www/React-prod.modern.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -820,4 +820,4 @@ exports.useSyncExternalStore = function (
820820
exports.useTransition = function () {
821821
return ReactSharedInternals.H.useTransition();
822822
};
823-
exports.version = "19.0.0-www-modern-0bc30748-20241028";
823+
exports.version = "19.0.0-www-modern-ea3ac586-20241031";

‎compiled/facebook-www/React-profiling.classic.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -828,7 +828,7 @@ exports.useSyncExternalStore = function (
828828
exports.useTransition = function () {
829829
return ReactSharedInternals.H.useTransition();
830830
};
831-
exports.version = "19.0.0-www-classic-0bc30748-20241028";
831+
exports.version = "19.0.0-www-classic-ea3ac586-20241031";
832832
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
833833
"function" ===
834834
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

‎compiled/facebook-www/React-profiling.modern.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -824,7 +824,7 @@ exports.useSyncExternalStore = function (
824824
exports.useTransition = function () {
825825
return ReactSharedInternals.H.useTransition();
826826
};
827-
exports.version = "19.0.0-www-modern-0bc30748-20241028";
827+
exports.version = "19.0.0-www-modern-ea3ac586-20241031";
828828
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
829829
"function" ===
830830
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

‎compiled/facebook-www/ReactART-dev.classic.js

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10917,15 +10917,17 @@ __DEV__ &&
1091710917
);
1091810918
break;
1091910919
case 21:
10920-
safelyDetachRef(deletedFiber, nearestMountedAncestor);
10920+
offscreenSubtreeWasHidden ||
10921+
safelyDetachRef(deletedFiber, nearestMountedAncestor);
1092110922
recursivelyTraverseDeletionEffects(
1092210923
finishedRoot,
1092310924
nearestMountedAncestor,
1092410925
deletedFiber
1092510926
);
1092610927
break;
1092710928
case 22:
10928-
safelyDetachRef(deletedFiber, nearestMountedAncestor);
10929+
offscreenSubtreeWasHidden ||
10930+
safelyDetachRef(deletedFiber, nearestMountedAncestor);
1092910931
disableLegacyMode || deletedFiber.mode & 1
1093010932
? ((offscreenSubtreeWasHidden =
1093110933
(_prevHostParent = offscreenSubtreeWasHidden) ||
@@ -11093,8 +11095,9 @@ __DEV__ &&
1109311095
recursivelyTraverseMutationEffects(root, finishedWork);
1109411096
commitReconciliationEffects(finishedWork);
1109511097
flags & 512 &&
11096-
null !== current &&
11097-
safelyDetachRef(current, current.return);
11098+
(offscreenSubtreeWasHidden ||
11099+
null === current ||
11100+
safelyDetachRef(current, current.return));
1109811101
flags & 64 &&
1109911102
offscreenSubtreeIsHidden &&
1110011103
((finishedWork = finishedWork.updateQueue),
@@ -11111,8 +11114,9 @@ __DEV__ &&
1111111114
recursivelyTraverseMutationEffects(root, finishedWork);
1111211115
commitReconciliationEffects(finishedWork);
1111311116
flags & 512 &&
11114-
null !== current &&
11115-
safelyDetachRef(current, current.return);
11117+
(offscreenSubtreeWasHidden ||
11118+
null === current ||
11119+
safelyDetachRef(current, current.return));
1111611120
if (finishedWork.flags & 32) {
1111711121
var instance = finishedWork.stateNode;
1111811122
try {
@@ -11235,8 +11239,9 @@ __DEV__ &&
1123511239
break;
1123611240
case 22:
1123711241
flags & 512 &&
11238-
null !== current &&
11239-
safelyDetachRef(current, current.return);
11242+
(offscreenSubtreeWasHidden ||
11243+
null === current ||
11244+
safelyDetachRef(current, current.return));
1124011245
suspenseCallback = null !== finishedWork.memoizedState;
1124111246
retryQueue = null !== current && null !== current.memoizedState;
1124211247
if (disableLegacyMode || finishedWork.mode & 1) {
@@ -11359,9 +11364,11 @@ __DEV__ &&
1135911364
recursivelyTraverseMutationEffects(root, finishedWork);
1136011365
commitReconciliationEffects(finishedWork);
1136111366
flags & 512 &&
11362-
(null !== current &&
11367+
(offscreenSubtreeWasHidden ||
11368+
null === current ||
1136311369
safelyDetachRef(finishedWork, finishedWork.return),
11364-
safelyAttachRef(finishedWork, finishedWork.return));
11370+
offscreenSubtreeIsHidden ||
11371+
safelyAttachRef(finishedWork, finishedWork.return));
1136511372
flags & 4 && prepareScopeUpdate();
1136611373
break;
1136711374
default:
@@ -17056,11 +17063,11 @@ __DEV__ &&
1705617063
(function () {
1705717064
var internals = {
1705817065
bundleType: 1,
17059-
version: "19.0.0-www-classic-0bc30748-20241028",
17066+
version: "19.0.0-www-classic-ea3ac586-20241031",
1706017067
rendererPackageName: "react-art",
1706117068
currentDispatcherRef: ReactSharedInternals,
1706217069
findFiberByHostInstance: getInstanceFromNode,
17063-
reconcilerVersion: "19.0.0-www-classic-0bc30748-20241028"
17070+
reconcilerVersion: "19.0.0-www-classic-ea3ac586-20241031"
1706417071
};
1706517072
internals.overrideHookState = overrideHookState;
1706617073
internals.overrideHookStateDeletePath = overrideHookStateDeletePath;
@@ -17094,7 +17101,7 @@ __DEV__ &&
1709417101
exports.Shape = Shape;
1709517102
exports.Surface = Surface;
1709617103
exports.Text = Text;
17097-
exports.version = "19.0.0-www-classic-0bc30748-20241028";
17104+
exports.version = "19.0.0-www-classic-ea3ac586-20241031";
1709817105
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
1709917106
"function" ===
1710017107
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

‎compiled/facebook-www/ReactART-dev.modern.js

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10534,15 +10534,17 @@ __DEV__ &&
1053410534
);
1053510535
break;
1053610536
case 21:
10537-
safelyDetachRef(deletedFiber, nearestMountedAncestor);
10537+
offscreenSubtreeWasHidden ||
10538+
safelyDetachRef(deletedFiber, nearestMountedAncestor);
1053810539
recursivelyTraverseDeletionEffects(
1053910540
finishedRoot,
1054010541
nearestMountedAncestor,
1054110542
deletedFiber
1054210543
);
1054310544
break;
1054410545
case 22:
10545-
safelyDetachRef(deletedFiber, nearestMountedAncestor);
10546+
offscreenSubtreeWasHidden ||
10547+
safelyDetachRef(deletedFiber, nearestMountedAncestor);
1054610548
offscreenSubtreeWasHidden =
1054710549
(_prevHostParent = offscreenSubtreeWasHidden) ||
1054810550
null !== deletedFiber.memoizedState;
@@ -10704,8 +10706,9 @@ __DEV__ &&
1070410706
recursivelyTraverseMutationEffects(root, finishedWork);
1070510707
commitReconciliationEffects(finishedWork);
1070610708
flags & 512 &&
10707-
null !== current &&
10708-
safelyDetachRef(current, current.return);
10709+
(offscreenSubtreeWasHidden ||
10710+
null === current ||
10711+
safelyDetachRef(current, current.return));
1070910712
flags & 64 &&
1071010713
offscreenSubtreeIsHidden &&
1071110714
((finishedWork = finishedWork.updateQueue),
@@ -10722,8 +10725,9 @@ __DEV__ &&
1072210725
recursivelyTraverseMutationEffects(root, finishedWork);
1072310726
commitReconciliationEffects(finishedWork);
1072410727
flags & 512 &&
10725-
null !== current &&
10726-
safelyDetachRef(current, current.return);
10728+
(offscreenSubtreeWasHidden ||
10729+
null === current ||
10730+
safelyDetachRef(current, current.return));
1072710731
if (finishedWork.flags & 32) {
1072810732
var instance = finishedWork.stateNode;
1072910733
try {
@@ -10846,8 +10850,9 @@ __DEV__ &&
1084610850
break;
1084710851
case 22:
1084810852
flags & 512 &&
10849-
null !== current &&
10850-
safelyDetachRef(current, current.return);
10853+
(offscreenSubtreeWasHidden ||
10854+
null === current ||
10855+
safelyDetachRef(current, current.return));
1085110856
suspenseCallback = null !== finishedWork.memoizedState;
1085210857
retryQueue = null !== current && null !== current.memoizedState;
1085310858
var prevOffscreenSubtreeIsHidden = offscreenSubtreeIsHidden,
@@ -10967,9 +10972,11 @@ __DEV__ &&
1096710972
recursivelyTraverseMutationEffects(root, finishedWork);
1096810973
commitReconciliationEffects(finishedWork);
1096910974
flags & 512 &&
10970-
(null !== current &&
10975+
(offscreenSubtreeWasHidden ||
10976+
null === current ||
1097110977
safelyDetachRef(finishedWork, finishedWork.return),
10972-
safelyAttachRef(finishedWork, finishedWork.return));
10978+
offscreenSubtreeIsHidden ||
10979+
safelyAttachRef(finishedWork, finishedWork.return));
1097310980
flags & 4 && prepareScopeUpdate();
1097410981
break;
1097510982
default:
@@ -16499,11 +16506,11 @@ __DEV__ &&
1649916506
(function () {
1650016507
var internals = {
1650116508
bundleType: 1,
16502-
version: "19.0.0-www-modern-0bc30748-20241028",
16509+
version: "19.0.0-www-modern-ea3ac586-20241031",
1650316510
rendererPackageName: "react-art",
1650416511
currentDispatcherRef: ReactSharedInternals,
1650516512
findFiberByHostInstance: getInstanceFromNode,
16506-
reconcilerVersion: "19.0.0-www-modern-0bc30748-20241028"
16513+
reconcilerVersion: "19.0.0-www-modern-ea3ac586-20241031"
1650716514
};
1650816515
internals.overrideHookState = overrideHookState;
1650916516
internals.overrideHookStateDeletePath = overrideHookStateDeletePath;
@@ -16537,7 +16544,7 @@ __DEV__ &&
1653716544
exports.Shape = Shape;
1653816545
exports.Surface = Surface;
1653916546
exports.Text = Text;
16540-
exports.version = "19.0.0-www-modern-0bc30748-20241028";
16547+
exports.version = "19.0.0-www-modern-ea3ac586-20241031";
1654116548
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
1654216549
"function" ===
1654316550
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

‎compiled/facebook-www/ReactART-prod.classic.js

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7879,15 +7879,17 @@ function commitDeletionEffectsOnFiber(
78797879
);
78807880
break;
78817881
case 21:
7882-
safelyDetachRef(deletedFiber, nearestMountedAncestor);
7882+
offscreenSubtreeWasHidden ||
7883+
safelyDetachRef(deletedFiber, nearestMountedAncestor);
78837884
recursivelyTraverseDeletionEffects(
78847885
finishedRoot,
78857886
nearestMountedAncestor,
78867887
deletedFiber
78877888
);
78887889
break;
78897890
case 22:
7890-
safelyDetachRef(deletedFiber, nearestMountedAncestor);
7891+
offscreenSubtreeWasHidden ||
7892+
safelyDetachRef(deletedFiber, nearestMountedAncestor);
78917893
disableLegacyMode || deletedFiber.mode & 1
78927894
? ((offscreenSubtreeWasHidden =
78937895
(child = offscreenSubtreeWasHidden) ||
@@ -8018,8 +8020,9 @@ function commitMutationEffectsOnFiber(finishedWork, root) {
80188020
recursivelyTraverseMutationEffects(root, finishedWork);
80198021
commitReconciliationEffects(finishedWork);
80208022
flags & 512 &&
8021-
null !== current &&
8022-
safelyDetachRef(current, current.return);
8023+
(offscreenSubtreeWasHidden ||
8024+
null === current ||
8025+
safelyDetachRef(current, current.return));
80238026
if (
80248027
flags & 64 &&
80258028
offscreenSubtreeIsHidden &&
@@ -8040,8 +8043,9 @@ function commitMutationEffectsOnFiber(finishedWork, root) {
80408043
recursivelyTraverseMutationEffects(root, finishedWork);
80418044
commitReconciliationEffects(finishedWork);
80428045
flags & 512 &&
8043-
null !== current &&
8044-
safelyDetachRef(current, current.return);
8046+
(offscreenSubtreeWasHidden ||
8047+
null === current ||
8048+
safelyDetachRef(current, current.return));
80458049
if (flags & 4 && null != finishedWork.stateNode) {
80468050
flags = finishedWork.memoizedProps;
80478051
existingHiddenCallbacks =
@@ -8106,8 +8110,9 @@ function commitMutationEffectsOnFiber(finishedWork, root) {
81068110
break;
81078111
case 22:
81088112
flags & 512 &&
8109-
null !== current &&
8110-
safelyDetachRef(current, current.return);
8113+
(offscreenSubtreeWasHidden ||
8114+
null === current ||
8115+
safelyDetachRef(current, current.return));
81118116
instance = null !== finishedWork.memoizedState;
81128117
suspenseCallback = null !== current && null !== current.memoizedState;
81138118
if (disableLegacyMode || finishedWork.mode & 1) {
@@ -8205,8 +8210,11 @@ function commitMutationEffectsOnFiber(finishedWork, root) {
82058210
recursivelyTraverseMutationEffects(root, finishedWork);
82068211
commitReconciliationEffects(finishedWork);
82078212
flags & 512 &&
8208-
(null !== current && safelyDetachRef(finishedWork, finishedWork.return),
8209-
safelyAttachRef(finishedWork, finishedWork.return));
8213+
(offscreenSubtreeWasHidden ||
8214+
null === current ||
8215+
safelyDetachRef(finishedWork, finishedWork.return),
8216+
offscreenSubtreeIsHidden ||
8217+
safelyAttachRef(finishedWork, finishedWork.return));
82108218
flags & 4 && shim$1();
82118219
break;
82128220
default:
@@ -10862,13 +10870,13 @@ var slice = Array.prototype.slice,
1086210870
})(React.Component);
1086310871
var internals$jscomp$inline_1468 = {
1086410872
bundleType: 0,
10865-
version: "19.0.0-www-classic-0bc30748-20241028",
10873+
version: "19.0.0-www-classic-ea3ac586-20241031",
1086610874
rendererPackageName: "react-art",
1086710875
currentDispatcherRef: ReactSharedInternals,
1086810876
findFiberByHostInstance: function () {
1086910877
return null;
1087010878
},
10871-
reconcilerVersion: "19.0.0-www-classic-0bc30748-20241028"
10879+
reconcilerVersion: "19.0.0-www-classic-ea3ac586-20241031"
1087210880
};
1087310881
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
1087410882
var hook$jscomp$inline_1469 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
@@ -10894,4 +10902,4 @@ exports.RadialGradient = RadialGradient;
1089410902
exports.Shape = TYPES.SHAPE;
1089510903
exports.Surface = Surface;
1089610904
exports.Text = Text;
10897-
exports.version = "19.0.0-www-classic-0bc30748-20241028";
10905+
exports.version = "19.0.0-www-classic-ea3ac586-20241031";

0 commit comments

Comments
 (0)