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 <rickhanlonii@fb.com> DiffTrain build for [ea3ac58](ea3ac58)
facebook · Oct 31, 2024 · eeb3328 · eeb3328
1 parent 0fb655b
commit eeb3328
Show file tree

Hide file tree

Showing 23 changed files with 269 additions and 203 deletions.
diff --git a/compiled-rn/VERSION_NATIVE_FB b/compiled-rn/VERSION_NATIVE_FB
@@ -1 +1 @@
-19.0.0-native-fb-0bc30748-20241028
+19.0.0-native-fb-ea3ac586-20241031
diff --git a/...iled-rn/facebook-fbsource/xplat/js/RKJSModules/vendor/react/react-dom/cjs/ReactDOM-dev.js b/...iled-rn/facebook-fbsource/xplat/js/RKJSModules/vendor/react/react-dom/cjs/ReactDOM-dev.js
@@ -7,7 +7,7 @@
  * @noflow
  * @nolint
  * @preventMunge
- * @generated SignedSource<<4186c5a616e166d9a549e0b6bbda0e76>>
+ * @generated SignedSource<<fe2a5887890fd9ac0c7e12bbbe65fde3>>
  */
 
 "use strict";
@@ -420,5 +420,5 @@ __DEV__ &&
     exports.useFormStatus = function () {
       return resolveDispatcher().useHostTransitionStatus();
     };
-    exports.version = "19.0.0-native-fb-0bc30748-20241028";
+    exports.version = "19.0.0-native-fb-ea3ac586-20241031";
   })();
diff --git a/...led-rn/facebook-fbsource/xplat/js/RKJSModules/vendor/react/react-dom/cjs/ReactDOM-prod.js b/...led-rn/facebook-fbsource/xplat/js/RKJSModules/vendor/react/react-dom/cjs/ReactDOM-prod.js
@@ -7,7 +7,7 @@
  * @noflow
  * @nolint
  * @preventMunge
- * @generated SignedSource<<fff3c4f1df329db5a3686b9c95687e91>>
+ * @generated SignedSource<<1cb77e72c96250d4d97fa3f50697ea38>>
  */
 
 "use strict";
@@ -203,4 +203,4 @@ exports.useFormState = function (action, initialState, permalink) {
 exports.useFormStatus = function () {
   return ReactSharedInternals.H.useHostTransitionStatus();
 };
-exports.version = "19.0.0-native-fb-0bc30748-20241028";
+exports.version = "19.0.0-native-fb-ea3ac586-20241031";
diff --git a/...n/facebook-fbsource/xplat/js/RKJSModules/vendor/react/react-dom/cjs/ReactDOM-profiling.js b/...n/facebook-fbsource/xplat/js/RKJSModules/vendor/react/react-dom/cjs/ReactDOM-profiling.js
@@ -7,7 +7,7 @@
  * @noflow
  * @nolint
  * @preventMunge
- * @generated SignedSource<<fff3c4f1df329db5a3686b9c95687e91>>
+ * @generated SignedSource<<1cb77e72c96250d4d97fa3f50697ea38>>
  */
 
 "use strict";
@@ -203,4 +203,4 @@ exports.useFormState = function (action, initialState, permalink) {
 exports.useFormStatus = function () {
   return ReactSharedInternals.H.useHostTransitionStatus();
 };
-exports.version = "19.0.0-native-fb-0bc30748-20241028";
+exports.version = "19.0.0-native-fb-ea3ac586-20241031";
diff --git a/...n/facebook-fbsource/xplat/js/RKJSModules/vendor/react/react-dom/cjs/ReactDOMClient-dev.js b/...n/facebook-fbsource/xplat/js/RKJSModules/vendor/react/react-dom/cjs/ReactDOMClient-dev.js
@@ -7,7 +7,7 @@
  * @noflow
  * @nolint
  * @preventMunge
- * @generated SignedSource<<cdd9b867ddcd339bddad99558118041c>>
+ * @generated SignedSource<<5c3370bf9d23dea134827436a14c995a>>
  */
 
 /*
@@ -12275,7 +12275,8 @@ __DEV__ &&
           );
           break;
         case 22:
-          safelyDetachRef(deletedFiber, nearestMountedAncestor);
+          offscreenSubtreeWasHidden ||
+            safelyDetachRef(deletedFiber, nearestMountedAncestor);
           deletedFiber.mode & ConcurrentMode
             ? ((offscreenSubtreeWasHidden =
                 (prevHostParent = offscreenSubtreeWasHidden) ||
@@ -12458,8 +12459,9 @@ __DEV__ &&
           recursivelyTraverseMutationEffects(root, finishedWork);
           commitReconciliationEffects(finishedWork);
           flags & 512 &&
-            null !== current &&
-            safelyDetachRef(current, current.return);
+            (offscreenSubtreeWasHidden ||
+              null === current ||
+              safelyDetachRef(current, current.return));
           flags & 64 &&
             offscreenSubtreeIsHidden &&
             ((finishedWork = finishedWork.updateQueue),
@@ -12475,8 +12477,9 @@ __DEV__ &&
           recursivelyTraverseMutationEffects(root, finishedWork);
           commitReconciliationEffects(finishedWork);
           flags & 512 &&
-            null !== current &&
-            safelyDetachRef(current, current.return);
+            (offscreenSubtreeWasHidden ||
+              null === current ||
+              safelyDetachRef(current, current.return));
           if (flags & 4)
             if (
               ((root = null !== current ? current.memoizedState : null),
@@ -12670,8 +12673,9 @@ __DEV__ &&
           recursivelyTraverseMutationEffects(root, finishedWork);
           commitReconciliationEffects(finishedWork);
           flags & 512 &&
-            null !== current &&
-            safelyDetachRef(current, current.return);
+            (offscreenSubtreeWasHidden ||
+              null === current ||
+              safelyDetachRef(current, current.return));
           if (finishedWork.flags & 32) {
             root = finishedWork.stateNode;
             try {
@@ -12809,8 +12813,9 @@ __DEV__ &&
           break;
         case 22:
           flags & 512 &&
-            null !== current &&
-            safelyDetachRef(current, current.return);
+            (offscreenSubtreeWasHidden ||
+              null === current ||
+              safelyDetachRef(current, current.return));
           i = null !== finishedWork.memoizedState;
           nextNode = null !== current && null !== current.memoizedState;
           finishedWork.mode & ConcurrentMode
@@ -25342,11 +25347,11 @@ __DEV__ &&
     };
     (function () {
       var isomorphicReactPackageVersion = React.version;
-      if ("19.0.0-native-fb-0bc30748-20241028" !== isomorphicReactPackageVersion)
+      if ("19.0.0-native-fb-ea3ac586-20241031" !== isomorphicReactPackageVersion)
         throw Error(
           'Incompatible React versions: The "react" and "react-dom" packages must have the exact same version. Instead got:\n  - react:      ' +
             (isomorphicReactPackageVersion +
-              "\n  - react-dom:  19.0.0-native-fb-0bc30748-20241028\nLearn more: https://react.dev/warnings/version-mismatch")
+              "\n  - react-dom:  19.0.0-native-fb-ea3ac586-20241031\nLearn more: https://react.dev/warnings/version-mismatch")
         );
     })();
     ("function" === typeof Map &&
@@ -25383,11 +25388,11 @@ __DEV__ &&
       !(function () {
         var internals = {
           bundleType: 1,
-          version: "19.0.0-native-fb-0bc30748-20241028",
+          version: "19.0.0-native-fb-ea3ac586-20241031",
           rendererPackageName: "react-dom",
           currentDispatcherRef: ReactSharedInternals,
           findFiberByHostInstance: getClosestInstanceFromNode,
-          reconcilerVersion: "19.0.0-native-fb-0bc30748-20241028"
+          reconcilerVersion: "19.0.0-native-fb-ea3ac586-20241031"
         };
         internals.overrideHookState = overrideHookState;
         internals.overrideHookStateDeletePath = overrideHookStateDeletePath;
@@ -25531,5 +25536,5 @@ __DEV__ &&
       listenToAllSupportedEvents(container);
       return new ReactDOMHydrationRoot(initialChildren);
     };
-    exports.version = "19.0.0-native-fb-0bc30748-20241028";
+    exports.version = "19.0.0-native-fb-ea3ac586-20241031";
   })();
diff --git a/.../facebook-fbsource/xplat/js/RKJSModules/vendor/react/react-dom/cjs/ReactDOMClient-prod.js b/.../facebook-fbsource/xplat/js/RKJSModules/vendor/react/react-dom/cjs/ReactDOMClient-prod.js
@@ -7,7 +7,7 @@
  * @noflow
  * @nolint
  * @preventMunge
- * @generated SignedSource<<5e5146d6bcd48109a990c272366ec551>>
+ * @generated SignedSource<<9227b55c2ff9afd498253f61ab8ef951>>
  */
 
 /*
@@ -8576,7 +8576,8 @@ function commitDeletionEffectsOnFiber(
       );
       break;
     case 22:
-      safelyDetachRef(deletedFiber, nearestMountedAncestor);
+      offscreenSubtreeWasHidden ||
+        safelyDetachRef(deletedFiber, nearestMountedAncestor);
       deletedFiber.mode & 1
         ? ((offscreenSubtreeWasHidden =
             (prevHostParent = offscreenSubtreeWasHidden) ||
@@ -8715,8 +8716,9 @@ function commitMutationEffectsOnFiber(finishedWork, root) {
       recursivelyTraverseMutationEffects(root, finishedWork);
       commitReconciliationEffects(finishedWork);
       flags & 512 &&
-        null !== current &&
-        safelyDetachRef(current, current.return);
+        (offscreenSubtreeWasHidden ||
+          null === current ||
+          safelyDetachRef(current, current.return));
       flags & 64 &&
         offscreenSubtreeIsHidden &&
         ((finishedWork = finishedWork.updateQueue),
@@ -8732,8 +8734,9 @@ function commitMutationEffectsOnFiber(finishedWork, root) {
       recursivelyTraverseMutationEffects(root, finishedWork);
       commitReconciliationEffects(finishedWork);
       flags & 512 &&
-        null !== current &&
-        safelyDetachRef(current, current.return);
+        (offscreenSubtreeWasHidden ||
+          null === current ||
+          safelyDetachRef(current, current.return));
       if (flags & 4) {
         var currentResource = null !== current ? current.memoizedState : null;
         flags = finishedWork.memoizedState;
@@ -8913,8 +8916,9 @@ function commitMutationEffectsOnFiber(finishedWork, root) {
       recursivelyTraverseMutationEffects(root, finishedWork);
       commitReconciliationEffects(finishedWork);
       flags & 512 &&
-        null !== current &&
-        safelyDetachRef(current, current.return);
+        (offscreenSubtreeWasHidden ||
+          null === current ||
+          safelyDetachRef(current, current.return));
       if (finishedWork.flags & 32) {
         hoistableRoot = finishedWork.stateNode;
         try {
@@ -9008,8 +9012,9 @@ function commitMutationEffectsOnFiber(finishedWork, root) {
       break;
     case 22:
       flags & 512 &&
-        null !== current &&
-        safelyDetachRef(current, current.return);
+        (offscreenSubtreeWasHidden ||
+          null === current ||
+          safelyDetachRef(current, current.return));
       node = null !== finishedWork.memoizedState;
       nextNode = null !== current && null !== current.memoizedState;
       finishedWork.mode & 1
@@ -15631,14 +15636,14 @@ ReactDOMHydrationRoot.prototype.unstable_scheduleHydration = function (target) {
 };
 var isomorphicReactPackageVersion$jscomp$inline_1691 = React.version;
 if (
-  "19.0.0-native-fb-0bc30748-20241028" !==
+  "19.0.0-native-fb-ea3ac586-20241031" !==
   isomorphicReactPackageVersion$jscomp$inline_1691
 )
   throw Error(
     formatProdErrorMessage(
       527,
       isomorphicReactPackageVersion$jscomp$inline_1691,
-      "19.0.0-native-fb-0bc30748-20241028"
+      "19.0.0-native-fb-ea3ac586-20241031"
     )
   );
 ReactDOMSharedInternals.findDOMNode = function (componentOrElement) {
@@ -15660,11 +15665,11 @@ ReactDOMSharedInternals.findDOMNode = function (componentOrElement) {
 };
 var internals$jscomp$inline_2145 = {
   bundleType: 0,
-  version: "19.0.0-native-fb-0bc30748-20241028",
+  version: "19.0.0-native-fb-ea3ac586-20241031",
   rendererPackageName: "react-dom",
   currentDispatcherRef: ReactSharedInternals,
   findFiberByHostInstance: getClosestInstanceFromNode,
-  reconcilerVersion: "19.0.0-native-fb-0bc30748-20241028"
+  reconcilerVersion: "19.0.0-native-fb-ea3ac586-20241031"
 };
 if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
   var hook$jscomp$inline_2146 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
@@ -15768,4 +15773,4 @@ exports.hydrateRoot = function (container, initialChildren, options) {
   listenToAllSupportedEvents(container);
   return new ReactDOMHydrationRoot(initialChildren);
 };
-exports.version = "19.0.0-native-fb-0bc30748-20241028";
+exports.version = "19.0.0-native-fb-ea3ac586-20241031";
diff --git a/...book-fbsource/xplat/js/RKJSModules/vendor/react/react-dom/cjs/ReactDOMClient-profiling.js b/...book-fbsource/xplat/js/RKJSModules/vendor/react/react-dom/cjs/ReactDOMClient-profiling.js
@@ -7,7 +7,7 @@
  * @noflow
  * @nolint
  * @preventMunge
- * @generated SignedSource<<72d89d3d6bfbf0efcb833b3c2b4137ad>>
+ * @generated SignedSource<<9ef4621d4d4d2360978a8053c835ed45>>
  */
 
 /*
@@ -8983,7 +8983,8 @@ function commitDeletionEffectsOnFiber(
       );
       break;
     case 22:
-      safelyDetachRef(deletedFiber, nearestMountedAncestor);
+      offscreenSubtreeWasHidden ||
+        safelyDetachRef(deletedFiber, nearestMountedAncestor);
       deletedFiber.mode & 1
         ? ((offscreenSubtreeWasHidden =
             (prevHostParent = offscreenSubtreeWasHidden) ||
@@ -9134,8 +9135,9 @@ function commitMutationEffectsOnFiber(finishedWork, root) {
       recursivelyTraverseMutationEffects(root, finishedWork);
       commitReconciliationEffects(finishedWork);
       flags & 512 &&
-        null !== current &&
-        safelyDetachRef(current, current.return);
+        (offscreenSubtreeWasHidden ||
+          null === current ||
+          safelyDetachRef(current, current.return));
       flags & 64 &&
         offscreenSubtreeIsHidden &&
         ((finishedWork = finishedWork.updateQueue),
@@ -9151,8 +9153,9 @@ function commitMutationEffectsOnFiber(finishedWork, root) {
       recursivelyTraverseMutationEffects(root, finishedWork);
       commitReconciliationEffects(finishedWork);
       flags & 512 &&
-        null !== current &&
-        safelyDetachRef(current, current.return);
+        (offscreenSubtreeWasHidden ||
+          null === current ||
+          safelyDetachRef(current, current.return));
       if (flags & 4)
         if (
           ((root = null !== current ? current.memoizedState : null),
@@ -9332,8 +9335,9 @@ function commitMutationEffectsOnFiber(finishedWork, root) {
       recursivelyTraverseMutationEffects(root, finishedWork);
       commitReconciliationEffects(finishedWork);
       flags & 512 &&
-        null !== current &&
-        safelyDetachRef(current, current.return);
+        (offscreenSubtreeWasHidden ||
+          null === current ||
+          safelyDetachRef(current, current.return));
       if (finishedWork.flags & 32) {
         root = finishedWork.stateNode;
         try {
@@ -9430,8 +9434,9 @@ function commitMutationEffectsOnFiber(finishedWork, root) {
       break;
     case 22:
       flags & 512 &&
-        null !== current &&
-        safelyDetachRef(current, current.return);
+        (offscreenSubtreeWasHidden ||
+          null === current ||
+          safelyDetachRef(current, current.return));
       node = null !== finishedWork.memoizedState;
       nextNode = null !== current && null !== current.memoizedState;
       finishedWork.mode & 1
@@ -16288,14 +16293,14 @@ ReactDOMHydrationRoot.prototype.unstable_scheduleHydration = function (target) {
 };
 var isomorphicReactPackageVersion$jscomp$inline_1781 = React.version;
 if (
-  "19.0.0-native-fb-0bc30748-20241028" !==
+  "19.0.0-native-fb-ea3ac586-20241031" !==
   isomorphicReactPackageVersion$jscomp$inline_1781
 )
   throw Error(
     formatProdErrorMessage(
       527,
       isomorphicReactPackageVersion$jscomp$inline_1781,
-      "19.0.0-native-fb-0bc30748-20241028"
+      "19.0.0-native-fb-ea3ac586-20241031"
     )
   );
 ReactDOMSharedInternals.findDOMNode = function (componentOrElement) {
@@ -16317,11 +16322,11 @@ ReactDOMSharedInternals.findDOMNode = function (componentOrElement) {
 };
 var internals$jscomp$inline_1788 = {
   bundleType: 0,
-  version: "19.0.0-native-fb-0bc30748-20241028",
+  version: "19.0.0-native-fb-ea3ac586-20241031",
   rendererPackageName: "react-dom",
   currentDispatcherRef: ReactSharedInternals,
   findFiberByHostInstance: getClosestInstanceFromNode,
-  reconcilerVersion: "19.0.0-native-fb-0bc30748-20241028",
+  reconcilerVersion: "19.0.0-native-fb-ea3ac586-20241031",
   getLaneLabelMap: function () {
     for (
       var map = new Map(), lane = 1, index$286 = 0;
@@ -16440,4 +16445,4 @@ exports.hydrateRoot = function (container, initialChildren, options) {
   listenToAllSupportedEvents(container);
   return new ReactDOMHydrationRoot(initialChildren);
 };
-exports.version = "19.0.0-native-fb-0bc30748-20241028";
+exports.version = "19.0.0-native-fb-ea3ac586-20241031";
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		19.0.0-native-fb-0bc30748-20241028
		19.0.0-native-fb-ea3ac586-20241031