[RN] Remove isMounted() false positive warning (#13511)

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

@@ -22,7 +22,7 @@ import UIManager from 'UIManager';
 
 import * as ReactNativeAttributePayload from './ReactNativeAttributePayload';
 import {
-  mountSafeCallback,
+  mountSafeCallback_NOT_REALLY_SAFE,
   throwOnStylesProp,
   warnForStyleProps,
 } from './NativeMethodsMixinUtils';
@@ -67,7 +67,7 @@ export default function(
     measure: function(callback: MeasureOnSuccessCallback) {
       UIManager.measure(
         findNodeHandle(this),
-        mountSafeCallback(this, callback),
+        mountSafeCallback_NOT_REALLY_SAFE(this, callback),
       );
     },
 
@@ -89,7 +89,7 @@ export default function(
     measureInWindow: function(callback: MeasureInWindowOnSuccessCallback) {
       UIManager.measureInWindow(
         findNodeHandle(this),
-        mountSafeCallback(this, callback),
+        mountSafeCallback_NOT_REALLY_SAFE(this, callback),
       );
     },
 
@@ -109,8 +109,8 @@ export default function(
       UIManager.measureLayout(
         findNodeHandle(this),
         relativeToNativeNode,
-        mountSafeCallback(this, onFail),
-        mountSafeCallback(this, onSuccess),
+        mountSafeCallback_NOT_REALLY_SAFE(this, onFail),
+        mountSafeCallback_NOT_REALLY_SAFE(this, onSuccess),
       );
     },
 

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

@@ -11,26 +11,36 @@
  * In the future, we should cleanup callbacks by cancelling them instead of
  * using this.
  */
-export function mountSafeCallback(context: any, callback: ?Function): any {
+export function mountSafeCallback_NOT_REALLY_SAFE(
+  context: any,
+  callback: ?Function,
+): any {
   return function() {
     if (!callback) {
       return undefined;
     }
+    // This protects against createClass() components.
+    // We don't know if there is code depending on it.
+    // We intentionally don't use isMounted() because even accessing
+    // isMounted property on a React ES6 class will trigger a warning.
     if (typeof context.__isMounted === 'boolean') {
-      // TODO(gaearon): this is gross and should be removed.
-      // It is currently necessary because View uses createClass,
-      // and so any measure() calls on View (which are done by React
-      // DevTools) trigger the isMounted() deprecation warning.
       if (!context.__isMounted) {
         return undefined;
       }
-      // The else branch is important so that we don't
-      // trigger the deprecation warning by calling isMounted.
-    } else if (typeof context.isMounted === 'function') {
-      if (!context.isMounted()) {
-        return undefined;
-      }
     }
+
+    // FIXME: there used to be other branches that protected
+    // against unmounted host components. But RN host components don't
+    // define isMounted() anymore, so those checks didn't do anything.
+
+    // They caused false positive warning noise so we removed them:
+    // https://github.com/facebook/react-native/issues/18868#issuecomment-413579095
+
+    // However, this means that the callback is NOT guaranteed to be safe
+    // for host components. The solution we should implement is to make
+    // UIManager.measure() and similar calls truly cancelable. Then we
+    // can change our own code calling them to cancel when something unmounts.
+
     return callback.apply(context, arguments);
   };
 }

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

@@ -15,7 +15,10 @@ import type {
   ReactNativeBaseComponentViewConfig,
 } from './ReactNativeTypes';
 
-import {mountSafeCallback, warnForStyleProps} from './NativeMethodsMixinUtils';
+import {
+  mountSafeCallback_NOT_REALLY_SAFE,
+  warnForStyleProps,
+} from './NativeMethodsMixinUtils';
 import * as ReactNativeAttributePayload from './ReactNativeAttributePayload';
 import * as ReactNativeFrameScheduling from './ReactNativeFrameScheduling';
 import * as ReactNativeViewConfigRegistry from 'ReactNativeViewConfigRegistry';
@@ -104,13 +107,16 @@ class ReactFabricHostComponent {
   }
 
   measure(callback: MeasureOnSuccessCallback) {
-    UIManager.measure(this._nativeTag, mountSafeCallback(this, callback));
+    UIManager.measure(
+      this._nativeTag,
+      mountSafeCallback_NOT_REALLY_SAFE(this, callback),
+    );
   }
 
   measureInWindow(callback: MeasureInWindowOnSuccessCallback) {
     UIManager.measureInWindow(
       this._nativeTag,
-      mountSafeCallback(this, callback),
+      mountSafeCallback_NOT_REALLY_SAFE(this, callback),
     );
   }
 
@@ -122,8 +128,8 @@ class ReactFabricHostComponent {
     UIManager.measureLayout(
       this._nativeTag,
       relativeToNativeNode,
-      mountSafeCallback(this, onFail),
-      mountSafeCallback(this, onSuccess),
+      mountSafeCallback_NOT_REALLY_SAFE(this, onFail),
+      mountSafeCallback_NOT_REALLY_SAFE(this, onSuccess),
     );
   }
 

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

@@ -22,7 +22,7 @@ import TextInputState from 'TextInputState';
 import UIManager from 'UIManager';
 
 import * as ReactNativeAttributePayload from './ReactNativeAttributePayload';
-import {mountSafeCallback} from './NativeMethodsMixinUtils';
+import {mountSafeCallback_NOT_REALLY_SAFE} from './NativeMethodsMixinUtils';
 
 export default function(
   findNodeHandle: any => ?number,
@@ -82,7 +82,7 @@ export default function(
     measure(callback: MeasureOnSuccessCallback): void {
       UIManager.measure(
         findNodeHandle(this),
-        mountSafeCallback(this, callback),
+        mountSafeCallback_NOT_REALLY_SAFE(this, callback),
       );
     }
 
@@ -102,7 +102,7 @@ export default function(
     measureInWindow(callback: MeasureInWindowOnSuccessCallback): void {
       UIManager.measureInWindow(
         findNodeHandle(this),
-        mountSafeCallback(this, callback),
+        mountSafeCallback_NOT_REALLY_SAFE(this, callback),
       );
     }
 
@@ -120,8 +120,8 @@ export default function(
       UIManager.measureLayout(
         findNodeHandle(this),
         relativeToNativeNode,
-        mountSafeCallback(this, onFail),
-        mountSafeCallback(this, onSuccess),
+        mountSafeCallback_NOT_REALLY_SAFE(this, onFail),
+        mountSafeCallback_NOT_REALLY_SAFE(this, onSuccess),
       );
     }
 

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

@@ -21,7 +21,10 @@ import TextInputState from 'TextInputState';
 import UIManager from 'UIManager';
 
 import * as ReactNativeAttributePayload from './ReactNativeAttributePayload';
-import {mountSafeCallback, warnForStyleProps} from './NativeMethodsMixinUtils';
+import {
+  mountSafeCallback_NOT_REALLY_SAFE,
+  warnForStyleProps,
+} from './NativeMethodsMixinUtils';
 
 /**
  * This component defines the same methods as NativeMethodsMixin but without the
@@ -50,13 +53,16 @@ class ReactNativeFiberHostComponent {
   }
 
   measure(callback: MeasureOnSuccessCallback) {
-    UIManager.measure(this._nativeTag, mountSafeCallback(this, callback));
+    UIManager.measure(
+      this._nativeTag,
+      mountSafeCallback_NOT_REALLY_SAFE(this, callback),
+    );
   }
 
   measureInWindow(callback: MeasureInWindowOnSuccessCallback) {
     UIManager.measureInWindow(
       this._nativeTag,
-      mountSafeCallback(this, callback),
+      mountSafeCallback_NOT_REALLY_SAFE(this, callback),
     );
   }
 
@@ -68,8 +74,8 @@ class ReactNativeFiberHostComponent {
     UIManager.measureLayout(
       this._nativeTag,
       relativeToNativeNode,
-      mountSafeCallback(this, onFail),
-      mountSafeCallback(this, onSuccess),
+      mountSafeCallback_NOT_REALLY_SAFE(this, onFail),
+      mountSafeCallback_NOT_REALLY_SAFE(this, onSuccess),
     );
   }