[React Native] measure calls will now call FabricUIManager

The Fabric renderer was previously calling the paper UIManager's measure calls and passing the react tag. This PR changes the renderer to now call FabricUIManager passing the node instead.

One of the parts of this that feels more controversial is making NativeMethodsMixin and ReactNative.NativeComponent warn when calling measureLayout in Fabric. As Seb and I decided in #15126, it doesn't make sense for a component created with one of these methods to require a native ref but not work the other way around. For example: a.measureLayout(b) might work but b.measureLayout(a) wouldn't. We figure we should keep these consistent and continue migrating things off of NativeMethodsMixin and NativeComponent.

If this becomes problematic for the Fabric rollout then we should revisit this.

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

@@ -18,6 +18,7 @@ import type {
 import invariant from 'shared/invariant';
 // Modules provided by RN:
 import TextInputState from 'TextInputState';
+import FabricUIManager from 'FabricUIManager';
 import UIManager from 'UIManager';
 
 import {create} from './ReactNativeAttributePayload';
@@ -68,10 +69,33 @@ export default function(
      * prop](docs/view.html#onlayout) instead.
      */
     measure: function(callback: MeasureOnSuccessCallback) {
-      UIManager.measure(
-        findNodeHandle(this),
-        mountSafeCallback_NOT_REALLY_SAFE(this, callback),
-      );
+      let maybeInstance;
+
+      // Fiber errors if findNodeHandle is called for an umounted component.
+      // Tests using ReactTestRenderer will trigger this case indirectly.
+      // Mimicking stack behavior, we should silently ignore this case.
+      // TODO Fix ReactTestRenderer so we can remove this try/catch.
+      try {
+        maybeInstance = findHostInstance(this);
+      } catch (error) {}
+
+      // If there is no host component beneath this we should fail silently.
+      // This is not an error; it could mean a class component rendered null.
+      if (maybeInstance == null) {
+        return;
+      }
+
+      if (maybeInstance.canonical) {
+        FabricUIManager.measure(
+          maybeInstance.node,
+          mountSafeCallback_NOT_REALLY_SAFE(this, callback),
+        );
+      } else {
+        UIManager.measure(
+          findNodeHandle(this),
+          mountSafeCallback_NOT_REALLY_SAFE(this, callback),
+        );
+      }
     },
 
     /**
@@ -90,10 +114,33 @@ export default function(
      * has been completed in native.
      */
     measureInWindow: function(callback: MeasureInWindowOnSuccessCallback) {
-      UIManager.measureInWindow(
-        findNodeHandle(this),
-        mountSafeCallback_NOT_REALLY_SAFE(this, callback),
-      );
+      let maybeInstance;
+
+      // Fiber errors if findNodeHandle is called for an umounted component.
+      // Tests using ReactTestRenderer will trigger this case indirectly.
+      // Mimicking stack behavior, we should silently ignore this case.
+      // TODO Fix ReactTestRenderer so we can remove this try/catch.
+      try {
+        maybeInstance = findHostInstance(this);
+      } catch (error) {}
+
+      // If there is no host component beneath this we should fail silently.
+      // This is not an error; it could mean a class component rendered null.
+      if (maybeInstance == null) {
+        return;
+      }
+
+      if (maybeInstance.canonical) {
+        FabricUIManager.measureInWindow(
+          maybeInstance.node,
+          mountSafeCallback_NOT_REALLY_SAFE(this, callback),
+        );
+      } else {
+        UIManager.measureInWindow(
+          findNodeHandle(this),
+          mountSafeCallback_NOT_REALLY_SAFE(this, callback),
+        );
+      }
     },
 
     /**
@@ -109,12 +156,38 @@ export default function(
       onSuccess: MeasureLayoutOnSuccessCallback,
       onFail: () => void /* currently unused */,
     ) {
-      UIManager.measureLayout(
-        findNodeHandle(this),
-        relativeToNativeNode,
-        mountSafeCallback_NOT_REALLY_SAFE(this, onFail),
-        mountSafeCallback_NOT_REALLY_SAFE(this, onSuccess),
-      );
+      let maybeInstance;
+
+      // Fiber errors if findNodeHandle is called for an umounted component.
+      // Tests using ReactTestRenderer will trigger this case indirectly.
+      // Mimicking stack behavior, we should silently ignore this case.
+      // TODO Fix ReactTestRenderer so we can remove this try/catch.
+      try {
+        maybeInstance = findHostInstance(this);
+      } catch (error) {}
+
+      // If there is no host component beneath this we should fail silently.
+      // This is not an error; it could mean a class component rendered null.
+      if (maybeInstance == null) {
+        return;
+      }
+
+      if (maybeInstance.canonical) {
+        warningWithoutStack(
+          false,
+          'Warning: measureLayout on components using NativeMethodsMixin '+
+          'or ReactNative.NativeComponent is not currently supported in Fabric. ' +
+          'measureLayout must be called on a native ref. Consider using forwardRef.',
+        );
+        return;
+      } else {
+        UIManager.measureLayout(
+          findNodeHandle(this),
+          relativeToNativeNode,
+          mountSafeCallback_NOT_REALLY_SAFE(this, onFail),
+          mountSafeCallback_NOT_REALLY_SAFE(this, onSuccess),
+        );
+      }
     },
 
     /**

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

@@ -39,6 +39,9 @@ import {
   appendChildToSet as appendChildNodeToSet,
   completeRoot,
   registerEventHandler,
+  measure as fabricMeasure,
+  measureInWindow as fabricMeasureInWindow,
+  measureLayout as fabricMeasureLayout,
 } from 'FabricUIManager';
 import UIManager from 'UIManager';
 
@@ -85,15 +88,18 @@ class ReactFabricHostComponent {
   _nativeTag: number;
   viewConfig: ReactNativeBaseComponentViewConfig<>;
   currentProps: Props;
+  _internalInstanceHandle: Instance;
 
   constructor(
     tag: number,
     viewConfig: ReactNativeBaseComponentViewConfig<>,
     props: Props,
+    internalInstanceHandle: Instance,
   ) {
     this._nativeTag = tag;
     this.viewConfig = viewConfig;
     this.currentProps = props;
+    this._internalInstanceHandle = internalInstanceHandle;
   }
 
   blur() {
@@ -105,50 +111,36 @@ class ReactFabricHostComponent {
   }
 
   measure(callback: MeasureOnSuccessCallback) {
-    UIManager.measure(
-      this._nativeTag,
+    fabricMeasure(
+      this._internalInstanceHandle.stateNode.node,
       mountSafeCallback_NOT_REALLY_SAFE(this, callback),
     );
   }
 
   measureInWindow(callback: MeasureInWindowOnSuccessCallback) {
-    UIManager.measureInWindow(
-      this._nativeTag,
+    fabricMeasureInWindow(
+      this._internalInstanceHandle.stateNode.node,
       mountSafeCallback_NOT_REALLY_SAFE(this, callback),
     );
   }
 
   measureLayout(
-    relativeToNativeNode: number | Object,
+    relativeToNativeNode: Object,
     onSuccess: MeasureLayoutOnSuccessCallback,
     onFail: () => void /* currently unused */,
   ) {
-    let relativeNode;
-
-    if (typeof relativeToNativeNode === 'number') {
-      // Already a node handle
-      relativeNode = relativeToNativeNode;
-    } else if (relativeToNativeNode._nativeTag) {
-      relativeNode = relativeToNativeNode._nativeTag;
-    } else if (
-      relativeToNativeNode.canonical &&
-      relativeToNativeNode.canonical._nativeTag
-    ) {
-      relativeNode = relativeToNativeNode.canonical._nativeTag;
-    }
-
-    if (relativeNode == null) {
+    if (relativeToNativeNode._internalInstanceHandle == null) {
       warningWithoutStack(
         false,
-        'Warning: ref.measureLayout must be called with a node handle or a ref to a native component.',
+        'Warning: ref.measureLayout must be called with a ref to a native component.',
       );
 
       return;
     }
 
-    UIManager.measureLayout(
-      this._nativeTag,
-      relativeNode,
+    fabricMeasureLayout(
+      this._internalInstanceHandle.stateNode.node,
+      relativeToNativeNode._internalInstanceHandle.stateNode.node,
       mountSafeCallback_NOT_REALLY_SAFE(this, onFail),
       mountSafeCallback_NOT_REALLY_SAFE(this, onSuccess),
     );
@@ -212,7 +204,12 @@ export function createInstance(
     internalInstanceHandle, // internalInstanceHandle
   );
 
-  const component = new ReactFabricHostComponent(tag, viewConfig, props);
+  const component = new ReactFabricHostComponent(
+    tag,
+    viewConfig,
+    props,
+    internalInstanceHandle,
+  );
 
   return {
     node: node,

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

@@ -19,6 +19,7 @@ import type {
 import React from 'react';
 // Modules provided by RN:
 import TextInputState from 'TextInputState';
+import FabricUIManager from 'FabricUIManager';
 import UIManager from 'UIManager';
 
 import {create} from './ReactNativeAttributePayload';
@@ -83,10 +84,33 @@ export default function(
      * [`onLayout` prop](docs/view.html#onlayout) instead.
      */
     measure(callback: MeasureOnSuccessCallback): void {
-      UIManager.measure(
-        findNodeHandle(this),
-        mountSafeCallback_NOT_REALLY_SAFE(this, callback),
-      );
+      let maybeInstance;
+
+      // Fiber errors if findNodeHandle is called for an umounted component.
+      // Tests using ReactTestRenderer will trigger this case indirectly.
+      // Mimicking stack behavior, we should silently ignore this case.
+      // TODO Fix ReactTestRenderer so we can remove this try/catch.
+      try {
+        maybeInstance = findHostInstance(this);
+      } catch (error) {}
+
+      // If there is no host component beneath this we should fail silently.
+      // This is not an error; it could mean a class component rendered null.
+      if (maybeInstance == null) {
+        return;
+      }
+
+      if (maybeInstance.canonical) {
+        FabricUIManager.measure(
+          maybeInstance.node,
+          mountSafeCallback_NOT_REALLY_SAFE(this, callback),
+        );
+      } else {
+        UIManager.measure(
+          findNodeHandle(this),
+          mountSafeCallback_NOT_REALLY_SAFE(this, callback),
+        );
+      }
     }
 
     /**
@@ -103,10 +127,33 @@ export default function(
      * These values are not available until after natives rendering completes.
      */
     measureInWindow(callback: MeasureInWindowOnSuccessCallback): void {
-      UIManager.measureInWindow(
-        findNodeHandle(this),
-        mountSafeCallback_NOT_REALLY_SAFE(this, callback),
-      );
+      let maybeInstance;
+
+      // Fiber errors if findNodeHandle is called for an umounted component.
+      // Tests using ReactTestRenderer will trigger this case indirectly.
+      // Mimicking stack behavior, we should silently ignore this case.
+      // TODO Fix ReactTestRenderer so we can remove this try/catch.
+      try {
+        maybeInstance = findHostInstance(this);
+      } catch (error) {}
+
+      // If there is no host component beneath this we should fail silently.
+      // This is not an error; it could mean a class component rendered null.
+      if (maybeInstance == null) {
+        return;
+      }
+
+      if (maybeInstance.canonical) {
+        FabricUIManager.measureInWindow(
+          maybeInstance.node,
+          mountSafeCallback_NOT_REALLY_SAFE(this, callback),
+        );
+      } else {
+        UIManager.measureInWindow(
+          findNodeHandle(this),
+          mountSafeCallback_NOT_REALLY_SAFE(this, callback),
+        );
+      }
     }
 
     /**
@@ -120,12 +167,38 @@ export default function(
       onSuccess: MeasureLayoutOnSuccessCallback,
       onFail: () => void /* currently unused */,
     ): void {
-      UIManager.measureLayout(
-        findNodeHandle(this),
-        relativeToNativeNode,
-        mountSafeCallback_NOT_REALLY_SAFE(this, onFail),
-        mountSafeCallback_NOT_REALLY_SAFE(this, onSuccess),
-      );
+      let maybeInstance;
+
+      // Fiber errors if findNodeHandle is called for an umounted component.
+      // Tests using ReactTestRenderer will trigger this case indirectly.
+      // Mimicking stack behavior, we should silently ignore this case.
+      // TODO Fix ReactTestRenderer so we can remove this try/catch.
+      try {
+        maybeInstance = findHostInstance(this);
+      } catch (error) {}
+
+      // If there is no host component beneath this we should fail silently.
+      // This is not an error; it could mean a class component rendered null.
+      if (maybeInstance == null) {
+        return;
+      }
+
+      if (maybeInstance.canonical) {
+        warningWithoutStack(
+          false,
+          'Warning: measureLayout on components using NativeMethodsMixin '+
+          'or ReactNative.NativeComponent is not currently supported in Fabric. ' +
+          'measureLayout must be called on a native ref. Consider using forwardRef.',
+        );
+        return;
+      } else {
+        UIManager.measureLayout(
+          findNodeHandle(this),
+          relativeToNativeNode,
+          mountSafeCallback_NOT_REALLY_SAFE(this, onFail),
+          mountSafeCallback_NOT_REALLY_SAFE(this, onSuccess),
+        );
+      }
     }
 
     /**

packages/react-native-renderer/src/__mocks__/FabricUIManager.js

@@ -119,6 +119,10 @@ const RCTFabricUIManager = {
   }),
 
   registerEventHandler: jest.fn(function registerEventHandler(callback) {}),
+
+  measure: jest.fn(),
+  measureLayout: jest.fn(),
+  measureInWindow: jest.fn(),
 };
 
 module.exports = RCTFabricUIManager;