ReactNative upstream sync (#9197)

* Addresses a circular dependency between NativeMethodsMixin and ReactNativeFiber:

* Created new ReactNativeFiberHostComponent with similar interface but without unnecessary call to findNodeHandle.
* Created new Flow interface for mixins to ensure ReactNativeFiberHostComponent and NativeMethodsMixinUtils stay synced.
* Moved helper methods to NativeMethodsMixinUtils to be shared between the 2 wrappers.

This diff was submitted and reviewed internally along with some other ReactNative changes. This is the React-only portion.

* Removed unused Flow type import

* Copied a Flow fix from internal

src/renderers/native/NativeMethodsMixin.js

@@ -12,50 +12,28 @@
 'use strict';
 
 var ReactNative = require('ReactNative');
+var ReactNativeFeatureFlags = require('ReactNativeFeatureFlags');
 var ReactNativeAttributePayload = require('ReactNativeAttributePayload');
 var TextInputState = require('TextInputState');
 var UIManager = require('UIManager');
 
 var invariant = require('fbjs/lib/invariant');
+var findNodeHandle = require('findNodeHandle');
 
-type MeasureOnSuccessCallback = (
-  x: number,
-  y: number,
-  width: number,
-  height: number,
-  pageX: number,
-  pageY: number,
-) => void;
+var {
+  mountSafeCallback,
+  throwOnStylesProp,
+  warnForStyleProps,
+} = require('NativeMethodsMixinUtils');
 
-type MeasureInWindowOnSuccessCallback = (
-  x: number,
-  y: number,
-  width: number,
-  height: number,
-) => void;
-
-type MeasureLayoutOnSuccessCallback = (
-  left: number,
-  top: number,
-  width: number,
-  height: number,
-) => void;
-
-function warnForStyleProps(props, validAttributes) {
-  for (var key in validAttributes.style) {
-    if (!(validAttributes[key] || props[key] === undefined)) {
-      console.error(
-        'You are setting the style `{ ' +
-          key +
-          ': ... }` as a prop. You ' +
-          'should nest it in a style object. ' +
-          'E.g. `{ style: { ' +
-          key +
-          ': ... } }`',
-      );
-    }
-  }
-}
+import type {
+  MeasureInWindowOnSuccessCallback,
+  MeasureLayoutOnSuccessCallback,
+  MeasureOnSuccessCallback,
+} from 'NativeMethodsMixinUtils';
+import type {
+  ReactNativeBaseComponentViewConfig,
+} from 'ReactNativeViewConfigRegistry';
 
 /**
  * `NativeMethodsMixin` provides methods to access the underlying native
@@ -69,6 +47,10 @@ function warnForStyleProps(props, validAttributes) {
  * information, see [Direct
  * Manipulation](docs/direct-manipulation.html).
  */
+// TODO (bvaughn) Figure out how to use the NativeMethodsInterface type to-
+// ensure that these mixins and ReactNativeFiberHostComponent stay in sync.
+// Unfortunately, using it causes Flow to complain WRT createClass mixins:
+// "call of method `createClass`. Expected an exact object instead of ..."
 var NativeMethodsMixin = {
   /**
    * Determines the location on screen, width, and height of the given view and
@@ -144,20 +126,15 @@ var NativeMethodsMixin = {
    * Manipulation](docs/direct-manipulation.html)).
    */
   setNativeProps: function(nativeProps: Object) {
-    if (__DEV__) {
-      warnForStyleProps(nativeProps, this.viewConfig.validAttributes);
-    }
+    // Ensure ReactNative factory function has configured findNodeHandle.
+    // Requiring it won't execute the factory function until first referenced.
+    // It's possible for tests that use ReactTestRenderer to reach this point,
+    // Without having executed ReactNative.
+    // Defer the factory function until now to avoid a cycle with UIManager.
+    // TODO (bvaughn) Remove this once ReactNativeStack is dropped.
+    require('ReactNative');
 
-    var updatePayload = ReactNativeAttributePayload.create(
-      nativeProps,
-      this.viewConfig.validAttributes,
-    );
-
-    UIManager.updateView(
-      (ReactNative.findNodeHandle(this): any),
-      this.viewConfig.uiViewClassName,
-      updatePayload,
-    );
+    injectedSetNativeProps(this, nativeProps);
   },
 
   /**
@@ -176,23 +153,113 @@ var NativeMethodsMixin = {
   },
 };
 
-function throwOnStylesProp(component, props) {
-  if (props.styles !== undefined) {
-    var owner = component._owner || null;
-    var name = component.constructor.displayName;
-    var msg = '`styles` is not a supported property of `' +
-      name +
-      '`, did ' +
-      'you mean `style` (singular)?';
-    if (owner && owner.constructor && owner.constructor.displayName) {
-      msg += '\n\nCheck the `' +
-        owner.constructor.displayName +
-        '` parent ' +
-        ' component.';
-    }
-    throw new Error(msg);
+// TODO (bvaughn) Inline this once ReactNativeStack is dropped.
+function setNativePropsFiber(componentOrHandle: any, nativeProps: Object) {
+  // Class components don't have viewConfig -> validateAttributes.
+  // Nor does it make sense to set native props on a non-native component.
+  // Instead, find the nearest host component and set props on it.
+  // Use findNodeHandle() rather than ReactNative.findNodeHandle() because
+  // We want the instance/wrapper (not the native tag).
+  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 = findNodeHandle(componentOrHandle);
+  } 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;
   }
+
+  const viewConfig: ReactNativeBaseComponentViewConfig = maybeInstance.viewConfig;
+
+  if (__DEV__) {
+    warnForStyleProps(nativeProps, viewConfig.validAttributes);
+  }
+
+  var updatePayload = ReactNativeAttributePayload.create(
+    nativeProps,
+    viewConfig.validAttributes,
+  );
+
+  UIManager.updateView(
+    maybeInstance._nativeTag,
+    viewConfig.uiViewClassName,
+    updatePayload,
+  );
 }
+
+// TODO (bvaughn) Remove this once ReactNativeStack is dropped.
+function setNativePropsStack(componentOrHandle: any, nativeProps: Object) {
+  // Class components don't have viewConfig -> validateAttributes.
+  // Nor does it make sense to set native props on a non-native component.
+  // Instead, find the nearest host component and set props on it.
+  // Use findNodeHandle() rather than ReactNative.findNodeHandle() because
+  // We want the instance/wrapper (not the native tag).
+  let maybeInstance = findNodeHandle(componentOrHandle);
+
+  // 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;
+  }
+
+  let viewConfig: ReactNativeBaseComponentViewConfig;
+  if (maybeInstance.viewConfig !== undefined) {
+    // ReactNativeBaseComponent
+    viewConfig = maybeInstance.viewConfig;
+  } else if (
+    maybeInstance._instance !== undefined &&
+    maybeInstance._instance.viewConfig !== undefined
+  ) {
+    // ReactCompositeComponentWrapper
+    // Some instances (eg Text) define their own viewConfig
+    viewConfig = maybeInstance._instance.viewConfig;
+  } else {
+    // ReactCompositeComponentWrapper
+    // Other instances (eg TextInput) defer to their children's viewConfig
+    while (maybeInstance._renderedComponent !== undefined) {
+      maybeInstance = maybeInstance._renderedComponent;
+    }
+    viewConfig = maybeInstance.viewConfig;
+  }
+
+  const tag: number = typeof maybeInstance.getHostNode === 'function'
+    ? maybeInstance.getHostNode()
+    : maybeInstance._rootNodeID;
+
+  if (__DEV__) {
+    warnForStyleProps(nativeProps, viewConfig.validAttributes);
+  }
+
+  var updatePayload = ReactNativeAttributePayload.create(
+    nativeProps,
+    viewConfig.validAttributes,
+  );
+
+  UIManager.updateView(tag, viewConfig.uiViewClassName, updatePayload);
+}
+
+// Switching based on fiber vs stack to avoid a lot of inline checks at runtime.
+// HACK Normally this injection would be done by the renderer, but in this case
+// that would result in a cycle between ReactNative and NativeMethodsMixin.
+// We avoid requiring additional code for this injection so it's probably ok?
+// TODO (bvaughn) Remove this once ReactNativeStack is gone.
+let injectedSetNativeProps: (
+  componentOrHandle: any,
+  nativeProps: Object,
+) => void;
+if (ReactNativeFeatureFlags.useFiber) {
+  injectedSetNativeProps = setNativePropsFiber;
+} else {
+  injectedSetNativeProps = setNativePropsStack;
+}
+
 if (__DEV__) {
   // hide this from Flow since we can't define these properties outside of
   // __DEV__ without actually implementing them (setting them to undefined
@@ -211,23 +278,4 @@ if (__DEV__) {
   };
 }
 
-/**
- * In the future, we should cleanup callbacks by cancelling them instead of
- * using this.
- */
-function mountSafeCallback(
-  context: ReactComponent<any, any, any>,
-  callback: ?Function,
-): any {
-  return function() {
-    if (
-      !callback ||
-      (typeof context.isMounted === 'function' && !context.isMounted())
-    ) {
-      return undefined;
-    }
-    return callback.apply(context, arguments);
-  };
-}
-
 module.exports = NativeMethodsMixin;

src/renderers/native/NativeMethodsMixinUtils.js

@@ -0,0 +1,108 @@
+/**
+ * Copyright (c) 2015-present, Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD-style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ *
+ * @providesModule NativeMethodsMixinUtils
+ * @flow
+ */
+'use strict';
+
+export type MeasureOnSuccessCallback = (
+  x: number,
+  y: number,
+  width: number,
+  height: number,
+  pageX: number,
+  pageY: number,
+) => void;
+
+export type MeasureInWindowOnSuccessCallback = (
+  x: number,
+  y: number,
+  width: number,
+  height: number,
+) => void;
+
+export type MeasureLayoutOnSuccessCallback = (
+  left: number,
+  top: number,
+  width: number,
+  height: number,
+) => void;
+
+/**
+ * Shared between ReactNativeFiberHostComponent and NativeMethodsMixin to keep
+ * API in sync.
+ */
+export interface NativeMethodsInterface {
+  blur(): void,
+  focus(): void,
+  measure(callback: MeasureOnSuccessCallback): void,
+  measureInWindow(callback: MeasureInWindowOnSuccessCallback): void,
+  measureLayout(
+    relativeToNativeNode: number,
+    onSuccess: MeasureLayoutOnSuccessCallback,
+    onFail: () => void,
+  ): void,
+  setNativeProps(nativeProps: Object): void,
+}
+
+/**
+ * In the future, we should cleanup callbacks by cancelling them instead of
+ * using this.
+ */
+function mountSafeCallback(context: any, callback: ?Function): any {
+  return function() {
+    if (
+      !callback ||
+      (typeof context.isMounted === 'function' && !context.isMounted())
+    ) {
+      return undefined;
+    }
+    return callback.apply(context, arguments);
+  };
+}
+
+function throwOnStylesProp(component: any, props: any) {
+  if (props.styles !== undefined) {
+    var owner = component._owner || null;
+    var name = component.constructor.displayName;
+    var msg = '`styles` is not a supported property of `' +
+      name +
+      '`, did ' +
+      'you mean `style` (singular)?';
+    if (owner && owner.constructor && owner.constructor.displayName) {
+      msg += '\n\nCheck the `' +
+        owner.constructor.displayName +
+        '` parent ' +
+        ' component.';
+    }
+    throw new Error(msg);
+  }
+}
+
+function warnForStyleProps(props: any, validAttributes: any) {
+  for (var key in validAttributes.style) {
+    if (!(validAttributes[key] || props[key] === undefined)) {
+      console.error(
+        'You are setting the style `{ ' +
+          key +
+          ': ... }` as a prop. You ' +
+          'should nest it in a style object. ' +
+          'E.g. `{ style: { ' +
+          key +
+          ': ... } }`',
+      );
+    }
+  }
+}
+
+module.exports = {
+  mountSafeCallback,
+  throwOnStylesProp,
+  warnForStyleProps,
+};

src/renderers/native/ReactNative.js

@@ -11,5 +11,8 @@
  */
 'use strict';
 
-// TODO (bvaughn) Enable Fiber experiement via ReactNativeFeatureFlags
-module.exports = require('ReactNativeStack');
+const ReactNativeFeatureFlags = require('ReactNativeFeatureFlags');
+
+module.exports = ReactNativeFeatureFlags.useFiber
+  ? require('ReactNativeFiber')
+  : require('ReactNativeStack');

src/renderers/native/ReactNativeFiber.js

@@ -12,18 +12,11 @@
 
 'use strict';
 
-import type {Element} from 'React';
-import type {Fiber} from 'ReactFiber';
-import type {ReactNodeList} from 'ReactTypes';
-import type {
-  ReactNativeBaseComponentViewConfig,
-} from 'ReactNativeViewConfigRegistry';
-
-const NativeMethodsMixin = require('NativeMethodsMixin');
 const ReactFiberReconciler = require('ReactFiberReconciler');
 const ReactGenericBatching = require('ReactGenericBatching');
 const ReactNativeAttributePayload = require('ReactNativeAttributePayload');
 const ReactNativeComponentTree = require('ReactNativeComponentTree');
+const ReactNativeFiberHostComponent = require('ReactNativeFiberHostComponent');
 const ReactNativeInjection = require('ReactNativeInjection');
 const ReactNativeTagHandles = require('ReactNativeTagHandles');
 const ReactNativeViewConfigRegistry = require('ReactNativeViewConfigRegistry');
@@ -36,6 +29,13 @@ const findNodeHandle = require('findNodeHandle');
 const invariant = require('fbjs/lib/invariant');
 
 const {injectInternals} = require('ReactFiberDevToolsHook');
+
+import type {Element} from 'React';
+import type {Fiber} from 'ReactFiber';
+import type {
+  ReactNativeBaseComponentViewConfig,
+} from 'ReactNativeViewConfigRegistry';
+import type {ReactNodeList} from 'ReactTypes';
 const {
   precacheFiberNode,
   uncacheFiberNode,
@@ -45,7 +45,7 @@ const {
 ReactNativeInjection.inject();
 
 type Container = number;
-type Instance = {
+export type Instance = {
   _children: Array<Instance | number>,
   _nativeTag: number,
   viewConfig: ReactNativeBaseComponentViewConfig,
@@ -53,13 +53,6 @@ type Instance = {
 type Props = Object;
 type TextInstance = number;
 
-function NativeHostComponent(tag, viewConfig) {
-  this._nativeTag = tag;
-  this._children = [];
-  this.viewConfig = viewConfig;
-}
-Object.assign(NativeHostComponent.prototype, NativeMethodsMixin);
-
 function recursivelyUncacheFiberNode(node: Instance | TextInstance) {
   if (typeof node === 'number') {
     // Leaf node (eg text)
@@ -162,7 +155,7 @@ const NativeRenderer = ReactFiberReconciler({
     const viewConfig = ReactNativeViewConfigRegistry.get(type);
 
     if (__DEV__) {
-      for (let key in viewConfig.validAttributes) {
+      for (const key in viewConfig.validAttributes) {
         if (props.hasOwnProperty(key)) {
           deepFreezeAndThrowOnMutationInDev(props[key]);
         }
@@ -181,12 +174,14 @@ const NativeRenderer = ReactFiberReconciler({
       updatePayload, // props
     );
 
-    const component = new NativeHostComponent(tag, viewConfig);
+    const component = new ReactNativeFiberHostComponent(tag, viewConfig);
 
     precacheFiberNode(internalInstanceHandle, tag);
     updateFiberProps(tag, props);
 
-    return component;
+    // Not sure how to avoid this cast. Flow is okay if the component is defined
+    // in the same file but if it's external it can't see the types.
+    return ((component: any): Instance);
   },
 
   createTextInstance(
@@ -377,14 +372,18 @@ ReactGenericBatching.injection.injectFiberBatchedUpdates(
 
 const roots = new Map();
 
-findNodeHandle.injection.injectFindNode((fiber: Fiber) => {
-  const instance: any = NativeRenderer.findHostInstance(fiber);
-  return instance ? instance._nativeTag : null;
-});
-findNodeHandle.injection.injectFindRootNodeID(instance => instance._nativeTag);
+findNodeHandle.injection.injectFindNode((fiber: Fiber) =>
+  NativeRenderer.findHostInstance(fiber));
+findNodeHandle.injection.injectFindRootNodeID(instance => instance);
 
 const ReactNative = {
-  findNodeHandle,
+  // External users of findNodeHandle() expect the host tag number return type.
+  // The injected findNodeHandle() strategy returns the instance wrapper though.
+  // See NativeMethodsMixin#setNativeProps for more info on why this is done.
+  findNodeHandle(componentOrHandle: any): ?number {
+    const instance: any = findNodeHandle(componentOrHandle);
+    return instance ? instance._nativeTag : null;
+  },
 
   render(element: Element<any>, containerTag: any, callback: ?Function) {
     let root = roots.get(containerTag);

src/renderers/native/ReactNativeFiberHostComponent.js

@@ -0,0 +1,104 @@
+/**
+ * Copyright 2013-present, Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD-style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ *
+ * @providesModule ReactNativeFiberHostComponent
+ * @flow
+ * @preventMunge
+ */
+
+'use strict';
+
+var ReactNativeAttributePayload = require('ReactNativeAttributePayload');
+var TextInputState = require('TextInputState');
+var UIManager = require('UIManager');
+
+var {
+  mountSafeCallback,
+  warnForStyleProps,
+} = require('NativeMethodsMixinUtils');
+
+import type {
+  MeasureInWindowOnSuccessCallback,
+  MeasureLayoutOnSuccessCallback,
+  MeasureOnSuccessCallback,
+  NativeMethodsInterface,
+} from 'NativeMethodsMixinUtils';
+import type {Instance} from 'ReactNativeFiber';
+import type {
+  ReactNativeBaseComponentViewConfig,
+} from 'ReactNativeViewConfigRegistry';
+
+/**
+ * This component defines the same methods as NativeMethodsMixin but without the
+ * findNodeHandle wrapper. This wrapper is unnecessary for HostComponent views
+ * and would also result in a circular require.js dependency (since
+ * ReactNativeFiber depends on this component and NativeMethodsMixin depends on
+ * ReactNativeFiber).
+ */
+class ReactNativeFiberHostComponent implements NativeMethodsInterface {
+  _children: Array<Instance | number>;
+  _nativeTag: number;
+  viewConfig: ReactNativeBaseComponentViewConfig;
+
+  constructor(tag: number, viewConfig: ReactNativeBaseComponentViewConfig) {
+    this._nativeTag = tag;
+    this._children = [];
+    this.viewConfig = viewConfig;
+  }
+
+  blur() {
+    TextInputState.blurTextInput(this._nativeTag);
+  }
+
+  focus() {
+    TextInputState.focusTextInput(this._nativeTag);
+  }
+
+  measure(callback: MeasureOnSuccessCallback) {
+    UIManager.measure(this._nativeTag, mountSafeCallback(this, callback));
+  }
+
+  measureInWindow(callback: MeasureInWindowOnSuccessCallback) {
+    UIManager.measureInWindow(
+      this._nativeTag,
+      mountSafeCallback(this, callback),
+    );
+  }
+
+  measureLayout(
+    relativeToNativeNode: number,
+    onSuccess: MeasureLayoutOnSuccessCallback,
+    onFail: () => void /* currently unused */,
+  ) {
+    UIManager.measureLayout(
+      this._nativeTag,
+      relativeToNativeNode,
+      mountSafeCallback(this, onFail),
+      mountSafeCallback(this, onSuccess),
+    );
+  }
+
+  setNativeProps(nativeProps: Object) {
+    if (__DEV__) {
+      warnForStyleProps(nativeProps, this.viewConfig.validAttributes);
+    }
+
+    var updatePayload = ReactNativeAttributePayload.create(
+      nativeProps,
+      this.viewConfig.validAttributes,
+    );
+
+    UIManager.updateView(
+      this._nativeTag,
+      this.viewConfig.uiViewClassName,
+      updatePayload,
+    );
+  }
+}
+
+module.exports = ReactNativeFiberHostComponent;

src/renderers/native/ReactNativeStack.js

@@ -13,8 +13,8 @@
 
 var ReactNativeComponentTree = require('ReactNativeComponentTree');
 var ReactNativeInjection = require('ReactNativeInjection');
-var ReactNativeStackInjection = require('ReactNativeStackInjection');
 var ReactNativeMount = require('ReactNativeMount');
+var ReactNativeStackInjection = require('ReactNativeStackInjection');
 var ReactUpdates = require('ReactUpdates');
 
 var findNodeHandle = require('findNodeHandle');
@@ -30,12 +30,16 @@ var render = function(
   return ReactNativeMount.renderComponent(element, mountInto, callback);
 };
 
-findNodeHandle.injection.injectFindNode(instance => instance.getHostNode());
-findNodeHandle.injection.injectFindRootNodeID(instance => instance._rootNodeID);
-
 var ReactNative = {
   hasReactNativeInitialized: false,
-  findNodeHandle: findNodeHandle,
+
+  // External users of findNodeHandle() expect the host tag number return type.
+  // The injected findNodeHandle() strategy returns the instance wrapper though.
+  // See NativeMethodsMixin#setNativeProps for more info on why this is done.
+  findNodeHandle(componentOrHandle: any): ?number {
+    return findNodeHandle(componentOrHandle).getHostNode();
+  },
+
   render: render,
   unmountComponentAtNode: ReactNativeMount.unmountComponentAtNode,
 

src/renderers/native/ReactNativeStackInjection.js

@@ -60,10 +60,8 @@ function inject() {
     );
   };
 
-  findNodeHandle.injection.injectFindNode(instance => instance.getHostNode());
-  findNodeHandle.injection.injectFindRootNodeID(
-    instance => instance._rootNodeID,
-  );
+  findNodeHandle.injection.injectFindNode(instance => instance);
+  findNodeHandle.injection.injectFindRootNodeID(instance => instance);
 
   ReactEmptyComponent.injection.injectEmptyComponentFactory(EmptyComponent);
 

src/renderers/native/createReactNativeComponentClass.js

@@ -13,8 +13,8 @@
 'use strict';
 
 const ReactNativeBaseComponent = require('ReactNativeBaseComponent');
-const ReactNativeViewConfigRegistry = require('ReactNativeViewConfigRegistry');
 const ReactNativeFeatureFlags = require('ReactNativeFeatureFlags');
+const ReactNativeViewConfigRegistry = require('ReactNativeViewConfigRegistry');
 
 // See also ReactNativeBaseComponent
 type ReactNativeBaseComponentViewConfig = {

src/renderers/native/findNodeHandle.js

@@ -53,7 +53,10 @@ import type {ReactInstance} from 'ReactInstanceType';
 let injectedFindNode;
 let injectedFindRootNodeID;
 
-function findNodeHandle(componentOrHandle: any): ?number {
+// TODO (bvaughn) Rename the findNodeHandle module to something more descriptive
+// eg findInternalHostInstance. This will reduce the likelihood of someone
+// accidentally deep-requiring this version.
+function findNodeHandle(componentOrHandle: any): any {
   if (__DEV__) {
     // TODO: fix this unsafe cast to work with Fiber.
     var owner = ((ReactCurrentOwner.current: any): ReactInstance | null);