Defer reading native view config from UIManager until view is used

[bvaughn]

Reading them from UIManager during registration forces a Prepack deopt.

Although a handful of React Native views define their configs in JavaScript (eg RCTText, ReactNativeART) the majority require them to be loaded from UIManager (eg RCTView). For consistency/simplicity, all view registrations have been updated to use this new lazy-config approach.

Downstream I have also updated requireNativeComponent. While testing, I noticed that this method fell back to returning an UnimplementedView component if a view config could not be found in UIManager. Apparently this was a short-term solution in place for Ads Manager at a time when Android didn't support many of the RN view types. After discussion with the React Native team, I've added an invariant here instead. This simplifies our Flow typing and avoids unnecessary runtime checks.

[sophiebits]

I don't think I fully understand how lazilyCreateReactNativeComponentClass will get used and how it relates to the non-lazy version. (For example, why not make them all lazy?)

[bvaughn]

Fair question! I left a few inline comments about this but probably didn't go into enough detail.

Some RN views (eg RCTText or ReactNativeART, anything that uses createReactNativeComponentClass directly) define their view configurations entirely in JavaScript. It didn't seem necessary to make them lazy since it would introduce an unnecessary function call. There's only ~10 of these in fbsource though, and we don't expose this creation method externally so...maybe it's better to just convert everything to be lazy?

I was kind of on the fence about it so your thoughts are welcome. 😄

[sophiebits]

I think it makes sense to just have the one API. Can you post the react-native diff too?

[bvaughn]

It's posted internally, on my Phabricator.

[bvaughn]

Okay. I've removed the non-lazy version of createReactNativeComponentClass and tested it via a quick fbsource sync. Seems to require pretty minimal changes.

[bvaughn]

I think it makes sense to just have the one API. Can you post the react-native diff too?

The full diff is at 5626312. Beyond the rote changes to replace sync createReactNativeComponentClass calls with lazy ones, the changes are limited to requireNativeComponent:

const ReactNativeStyleAttributes = require('ReactNativeStyleAttributes');
 const UIManager = require('UIManager');
-const UnimplementedView = require('UnimplementedView');
 
 const createReactNativeComponentClass = require('createReactNativeComponentClass');
 const insetsDiffer = require('insetsDiffer');
@@ -23,6 +22,7 @@
 const resolveAssetSource = require('resolveAssetSource');
 const sizesDiffer = require('sizesDiffer');
 const verifyPropTypes = require('verifyPropTypes');
+const invariant = require('fbjs/lib/invariant');
 const warning = require('fbjs/lib/warning');
 
 /**
@@ -47,77 +47,86 @@
   componentInterface?: ?ComponentInterface,
   extraConfig?: ?{nativeOnly?: Object},
 ): React$ComponentType<any> | string {
-  const viewConfig = UIManager[viewName];
-  if (!viewConfig || !viewConfig.NativeProps) {
-    warning(false, 'Native component for "%s" does not exist', viewName);
-    return UnimplementedView;
-  }
+  // Don't load the ViewConfig from UIManager until it's needed for rendering.
+  // Lazy-loading this can help avoid Prepack deopts.
+  function getViewConfig() {
+    const viewConfig = UIManager[viewName];
+
+    invariant(
+      viewConfig != null &&
+      !viewConfig.NativeProps != null,
+      'Native component for "%s" does not exist',
+      viewName
+    );
 
-  viewConfig.uiViewClassName = viewName;
-  viewConfig.validAttributes = {};
+    viewConfig.uiViewClassName = viewName;
+    viewConfig.validAttributes = {};
+
+    // ReactNative `View.propTypes` have been deprecated in favor of
+    // `ViewPropTypes`. In their place a temporary getter has been added with a
+    // deprecated warning message. Avoid triggering that warning here by using
+    // temporary workaround, __propTypesSecretDontUseThesePlease.
+    // TODO (bvaughn) Revert this particular change any time after April 1
+    if (componentInterface) {
+      viewConfig.propTypes =
+        typeof componentInterface.__propTypesSecretDontUseThesePlease === 'object'
+          ? componentInterface.__propTypesSecretDontUseThesePlease
+          : componentInterface.propTypes;
+    } else {
+      viewConfig.propTypes = null;
+    }
 
-  // ReactNative `View.propTypes` have been deprecated in favor of
-  // `ViewPropTypes`. In their place a temporary getter has been added with a
-  // deprecated warning message. Avoid triggering that warning here by using
-  // temporary workaround, __propTypesSecretDontUseThesePlease.
-  // TODO (bvaughn) Revert this particular change any time after April 1
-  if (componentInterface) {
-    viewConfig.propTypes =
-      typeof componentInterface.__propTypesSecretDontUseThesePlease === 'object'
-        ? componentInterface.__propTypesSecretDontUseThesePlease
-        : componentInterface.propTypes;
-  } else {
-    viewConfig.propTypes = null;
-  }
+    let baseModuleName = viewConfig.baseModuleName;
+    let nativeProps = { ...viewConfig.NativeProps };
+    while (baseModuleName) {
+      const baseModule = UIManager[baseModuleName];
+      if (!baseModule) {
+        warning(false, 'Base module "%s" does not exist', baseModuleName);
+        baseModuleName = null;
+      } else {
+        nativeProps = { ...nativeProps, ...baseModule.NativeProps };
+        baseModuleName = baseModule.baseModuleName;
+      }
+    }
+
+    for (const key in nativeProps) {
+      let useAttribute = false;
+      const attribute = {};
+
+      const differ = TypeToDifferMap[nativeProps[key]];
+      if (differ) {
+        attribute.diff = differ;
+        useAttribute = true;
+      }
 
-  let baseModuleName = viewConfig.baseModuleName;
-  let nativeProps = { ...viewConfig.NativeProps };
-  while (baseModuleName) {
-    const baseModule = UIManager[baseModuleName];
-    if (!baseModule) {
-      warning(false, 'Base module "%s" does not exist', baseModuleName);
-      baseModuleName = null;
-    } else {
-      nativeProps = { ...nativeProps, ...baseModule.NativeProps };
-      baseModuleName = baseModule.baseModuleName;
+      const processor = TypeToProcessorMap[nativeProps[key]];
+      if (processor) {
+        attribute.process = processor;
+        useAttribute = true;
+      }
+
+      viewConfig.validAttributes[key] = useAttribute ? attribute : true;
     }
+
+    // Unfortunately, the current set up puts the style properties on the top
+    // level props object. We also need to add the nested form for API
+    // compatibility. This allows these props on both the top level and the
+    // nested style level. TODO: Move these to nested declarations on the
+    // native side.
+    viewConfig.validAttributes.style = ReactNativeStyleAttributes;
+
+    if (__DEV__) {
+      componentInterface && verifyPropTypes(
+        componentInterface,
+        viewConfig,
+        extraConfig && extraConfig.nativeOnly
+      );
+    }
+
+    return viewConfig;
   }
 
-  for (const key in nativeProps) {
-    let useAttribute = false;
-    const attribute = {};
-
-    const differ = TypeToDifferMap[nativeProps[key]];
-    if (differ) {
-      attribute.diff = differ;
-      useAttribute = true;
-    }
-
-    const processor = TypeToProcessorMap[nativeProps[key]];
-    if (processor) {
-      attribute.process = processor;
-      useAttribute = true;
-    }
-
-    viewConfig.validAttributes[key] = useAttribute ? attribute : true;
-  }
-
-  // Unfortunately, the current set up puts the style properties on the top
-  // level props object. We also need to add the nested form for API
-  // compatibility. This allows these props on both the top level and the
-  // nested style level. TODO: Move these to nested declarations on the
-  // native side.
-  viewConfig.validAttributes.style = ReactNativeStyleAttributes;
-
-  if (__DEV__) {
-    componentInterface && verifyPropTypes(
-      componentInterface,
-      viewConfig,
-      extraConfig && extraConfig.nativeOnly
-    );
-  }
-
-  return createReactNativeComponentClass(viewConfig);
+  return createReactNativeComponentClass(viewName, getViewConfig);
 }
 
 const TypeToDifferMap = {

[sophiebits]

lgtm, thanks!

[sophiebits]

Any reason this isn't .delete? Either way.

[bvaughn]

I use viewConfigCallbacks to check for view name uniqueness. Nulling out prevents us from blocking GC while avoiding having to check both viewConfigs and viewConfigCallbacks maps.

[sophiebits]

👍