Support string return type from RN createReactNativeFiberComponentClass()

Reviewed By: sebmarkbage

Differential Revision: D4607283

fbshipit-source-id: 466d2373dd570f77ebcced306d2f20a3f72d79c6

Author: Brian Vaughn

Libraries/Components/TextInput/TextInput.js

@@ -540,13 +540,6 @@ const TextInput = React.createClass({
    */
   mixins: [NativeMethodsMixin, TimerMixin],
 
-  viewConfig:
-    ((Platform.OS === 'ios' && RCTTextField ?
-      RCTTextField.viewConfig :
-      (Platform.OS === 'android' && AndroidTextInput ?
-        AndroidTextInput.viewConfig :
-        {})) : Object),
-
   /**
    * Returns `true` if the input is currently focused; `false` otherwise.
    */

Libraries/Components/View/View.js

@@ -16,6 +16,7 @@ const NativeMethodsMixin = require('NativeMethodsMixin');
 const NativeModules = require('NativeModules');
 const Platform = require('Platform');
 const React = require('React');
+const ReactNativeFeatureFlags = require('ReactNativeFeatureFlags');
 const ReactNativeStyleAttributes = require('ReactNativeStyleAttributes');
 const ReactNativeViewAttributes = require('ReactNativeViewAttributes');
 const StyleSheetPropType = require('StyleSheetPropType');
@@ -119,6 +120,9 @@ const View = React.createClass({
     ...statics,
   },
 
+  // TODO (bvaughn) Replace this with a deprecated getter warning. This object
+  // should be accessible via a separate import. It will not be available in
+  // production mode in the future and so should not be directly accessed.
   propTypes: {
     ...TVViewPropTypes,
 
@@ -536,11 +540,20 @@ if (__DEV__) {
   }
 }
 
+// TODO (bvaughn) Remove feature flags once all static View accessors are gone.
+// We temporarily wrap fiber native views with the create-class View above,
+// Because external code sometimes accesses static properties of this view.
 let ViewToExport = RCTView;
-if (__DEV__) {
+if (
+  __DEV__ ||
+  ReactNativeFeatureFlags.useFiber
+) {
   ViewToExport = View;
 } else {
-  Object.assign(RCTView, statics);
+  // TODO (bvaughn) Remove this mixin once all static View accessors are gone.
+  Object.assign((RCTView : any), statics);
 }
 
-module.exports = ViewToExport;
+// TODO (bvaughn) Temporarily mask Flow warnings for View property accesses.
+// We're wrapping the string type (Fiber) for now to avoid any actual problems.
+module.exports = ((ViewToExport : any) : typeof View);

Libraries/ReactNative/UIManager.js

@@ -26,6 +26,27 @@ invariant(UIManager, 'UIManager is undefined. The native module config is probab
 
 const _takeSnapshot = UIManager.takeSnapshot;
 
+// findNodeHandle() returns a reference to a wrapper component with viewConfig.
+// This wrapper is required for NativeMethodsMixin.setNativeProps, but most
+// callers want the native tag (number) and not the wrapper. For this purpose,
+// the ReactNative renderer decorates findNodeHandle() and extracts the tag.
+// However UIManager can't require ReactNative without introducing a cycle, and
+// deferring the require causes a significant performance regression in Wilde
+// (along the lines of 17% regression in RN Bridge startup). So as a temporary
+// workaround, this wrapper method mimics what the native renderer does.
+// TODO (bvaughn) Remove this and use findNodeHandle directly once stack is gone
+function findNodeHandleWrapper(componentOrHandle : any) : ?number {
+  const instance: any = findNodeHandle(componentOrHandle);
+
+  if (instance) {
+    return typeof instance._nativeTag === 'number'
+      ? instance._nativeTag
+      : instance.getHostNode();
+  } else {
+    return null;
+  }
+}
+
 /**
  * Capture an image of the screen, window or an individual view. The image
  * will be stored in a temporary file that will only exist for as long as the
@@ -57,7 +78,7 @@ UIManager.takeSnapshot = async function(
     return;
   }
   if (typeof view !== 'number' && view !== 'window') {
-    view = findNodeHandle(view) || 'window';
+    view = findNodeHandleWrapper(view) || 'window';
   }
   return _takeSnapshot(view, options);
 };

Libraries/ReactNative/requireNativeComponent.js

@@ -46,7 +46,7 @@ function requireNativeComponent(
   viewName: string,
   componentInterface?: ?ComponentInterface,
   extraConfig?: ?{nativeOnly?: Object},
-): Function {
+): ReactClass<any> | string {
   const viewConfig = UIManager[viewName];
   if (!viewConfig || !viewConfig.NativeProps) {
     warning(false, 'Native component for "%s" does not exist', viewName);

Libraries/Renderer/src/renderers/native/NativeMethodsMixin.js

@@ -13,45 +13,25 @@
 
 var ReactNative = require('ReactNative');
 var ReactNativeAttributePayload = require('ReactNativeAttributePayload');
+var ReactNativeFeatureFlags = require('ReactNativeFeatureFlags');
 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
-
-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 + ': ... } }`'
-      );
-    }
-  }
-}
+var {
+  mountSafeCallback,
+  throwOnStylesProp,
+  warnForStyleProps,
+} = require('NativeMethodsMixinUtils');
+
+import type {
+  MeasureInWindowOnSuccessCallback,
+  MeasureLayoutOnSuccessCallback,
+  MeasureOnSuccessCallback,
+} from 'NativeMethodsMixinUtils';
+import type { ReactNativeBaseComponentViewConfig } from 'ReactNativeViewConfigRegistry';
 
 /**
  * `NativeMethodsMixin` provides methods to access the underlying native
@@ -65,6 +45,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
@@ -140,20 +124,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);
   },
 
   /**
@@ -172,19 +151,116 @@ 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.';
+// 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;
     }
-    throw new Error(msg);
+    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
@@ -203,20 +279,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;