From 8bd5b489b4cfc080aca8f93e5487abbeb7fc879d Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 31 Aug 2017 13:42:27 -0700 Subject: [PATCH] Defer reading native view config from UIManager until view is used (#10569) * ReactNative supports lazy view registration Don't react viewConfig from UIManager when a view is registered, but only when it is used for the first time. This will help unblock Prepack optimizations for RN views. * Added new lazy-creation method for native views Also exposed the native view config registry to allow invalid/unsupported host components to fall back to RCTView * Removed ReactNativeViewConfigRegistry from RN __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED After discussion with Spencer Ahrens and Andy Street, it seems the early fallback behavior was mostly for the early days of Ads Manager when Android didn't yet support a lot of the view types. It should be okay now to use an invarient/redbox instead for unsupported views. * Removed non-lazy createReactNativeComponentClass impl There are only a handful of components with JavaScript-defined view configs. It's probably worth making them use the new lazy/async interface as well to be more consistent. --- src/renderers/native/NativeMethodsMixin.js | 4 +- src/renderers/native/ReactNativeComponent.js | 4 +- src/renderers/native/ReactNativeFiberEntry.js | 2 +- .../native/ReactNativeFiberHostComponent.js | 4 +- .../native/ReactNativeFiberRenderer.js | 4 +- src/renderers/native/ReactNativeTypes.js | 17 ++++--- .../native/ReactNativeViewConfigRegistry.js | 50 ++++++++++++++----- .../__tests__/ReactNativeEvents-test.js | 16 +++--- .../native/__tests__/ReactNativeMount-test.js | 24 ++++----- .../createReactNativeComponentClass-test.js | 20 ++++++-- .../native/createReactNativeComponentClass.js | 16 +++--- 11 files changed, 96 insertions(+), 65 deletions(-) diff --git a/src/renderers/native/NativeMethodsMixin.js b/src/renderers/native/NativeMethodsMixin.js index 977111b08a..a9028b9304 100644 --- a/src/renderers/native/NativeMethodsMixin.js +++ b/src/renderers/native/NativeMethodsMixin.js @@ -30,10 +30,8 @@ import type { MeasureLayoutOnSuccessCallback, MeasureOnSuccessCallback, NativeMethodsMixinType, -} from 'ReactNativeTypes'; -import type { ReactNativeBaseComponentViewConfig, -} from 'ReactNativeViewConfigRegistry'; +} from 'ReactNativeTypes'; /** * `NativeMethodsMixin` provides methods to access the underlying native diff --git a/src/renderers/native/ReactNativeComponent.js b/src/renderers/native/ReactNativeComponent.js index 1e2e2a01e3..65b2767599 100644 --- a/src/renderers/native/ReactNativeComponent.js +++ b/src/renderers/native/ReactNativeComponent.js @@ -28,10 +28,8 @@ import type { MeasureLayoutOnSuccessCallback, MeasureOnSuccessCallback, NativeMethodsMixinType, -} from 'ReactNativeTypes'; -import type { ReactNativeBaseComponentViewConfig, -} from 'ReactNativeViewConfigRegistry'; +} from 'ReactNativeTypes'; /** * Superclass that provides methods to access the underlying native component. diff --git a/src/renderers/native/ReactNativeFiberEntry.js b/src/renderers/native/ReactNativeFiberEntry.js index 99ef774daf..d0f119cbde 100644 --- a/src/renderers/native/ReactNativeFiberEntry.js +++ b/src/renderers/native/ReactNativeFiberEntry.js @@ -100,7 +100,7 @@ const ReactNativeFiber: ReactNativeType = { ReactNativeComponentTree: require('ReactNativeComponentTree'), // InspectorUtils, ScrollResponder ReactNativePropRegistry: require('ReactNativePropRegistry'), // flattenStyle, Stylesheet TouchHistoryMath: require('TouchHistoryMath'), // PanResponder - createReactNativeComponentClass: require('createReactNativeComponentClass'), // eg Text + createReactNativeComponentClass: require('createReactNativeComponentClass'), // eg RCTText, RCTView, ReactNativeART takeSnapshot: require('takeSnapshot'), // react-native-implementation }, }; diff --git a/src/renderers/native/ReactNativeFiberHostComponent.js b/src/renderers/native/ReactNativeFiberHostComponent.js index 67e9af67d7..c7842c9384 100644 --- a/src/renderers/native/ReactNativeFiberHostComponent.js +++ b/src/renderers/native/ReactNativeFiberHostComponent.js @@ -24,11 +24,9 @@ import type { MeasureLayoutOnSuccessCallback, MeasureOnSuccessCallback, NativeMethodsMixinType, + ReactNativeBaseComponentViewConfig, } from 'ReactNativeTypes'; import type {Instance} from 'ReactNativeFiberRenderer'; -import type { - ReactNativeBaseComponentViewConfig, -} from 'ReactNativeViewConfigRegistry'; /** * This component defines the same methods as NativeMethodsMixin but without the diff --git a/src/renderers/native/ReactNativeFiberRenderer.js b/src/renderers/native/ReactNativeFiberRenderer.js index ee1dbccb4c..03b02acdfd 100644 --- a/src/renderers/native/ReactNativeFiberRenderer.js +++ b/src/renderers/native/ReactNativeFiberRenderer.js @@ -24,9 +24,7 @@ const deepFreezeAndThrowOnMutationInDev = require('deepFreezeAndThrowOnMutationI const emptyObject = require('fbjs/lib/emptyObject'); const invariant = require('fbjs/lib/invariant'); -import type { - ReactNativeBaseComponentViewConfig, -} from 'ReactNativeViewConfigRegistry'; +import type {ReactNativeBaseComponentViewConfig} from 'ReactNativeTypes'; export type Container = number; export type Instance = { diff --git a/src/renderers/native/ReactNativeTypes.js b/src/renderers/native/ReactNativeTypes.js index 4d9da467cd..2f5bea4048 100644 --- a/src/renderers/native/ReactNativeTypes.js +++ b/src/renderers/native/ReactNativeTypes.js @@ -34,6 +34,14 @@ export type MeasureLayoutOnSuccessCallback = ( height: number, ) => void; +export type ReactNativeBaseComponentViewConfig = { + validAttributes: Object, + uiViewClassName: string, + propTypes?: Object, +}; + +export type ViewConfigGetter = () => ReactNativeBaseComponentViewConfig; + /** * This type keeps ReactNativeFiberHostComponent and NativeMethodsMixin in sync. * It can also provide types for ReactNative applications that use NMM or refs. @@ -51,16 +59,11 @@ export type NativeMethodsMixinType = { setNativeProps(nativeProps: Object): void, }; -type ReactNativeBaseComponentViewConfig = { - validAttributes: Object, - uiViewClassName: string, - propTypes?: Object, -}; - type SecretInternalsType = { NativeMethodsMixin: NativeMethodsMixinType, createReactNativeComponentClass( - viewConfig: ReactNativeBaseComponentViewConfig, + name: string, + callback: ViewConfigGetter, ): any, ReactNativeComponentTree: any, ReactNativePropRegistry: any, diff --git a/src/renderers/native/ReactNativeViewConfigRegistry.js b/src/renderers/native/ReactNativeViewConfigRegistry.js index fa6d379924..370a933706 100644 --- a/src/renderers/native/ReactNativeViewConfigRegistry.js +++ b/src/renderers/native/ReactNativeViewConfigRegistry.js @@ -14,29 +14,53 @@ const invariant = require('fbjs/lib/invariant'); -export type ReactNativeBaseComponentViewConfig = { - validAttributes: Object, - uiViewClassName: string, - propTypes?: Object, -}; +import type { + ReactNativeBaseComponentViewConfig, + ViewConfigGetter, +} from 'ReactNativeTypes'; +const viewConfigCallbacks = new Map(); const viewConfigs = new Map(); const ReactNativeViewConfigRegistry = { - register(viewConfig: ReactNativeBaseComponentViewConfig) { - const name = viewConfig.uiViewClassName; + /** + * Registers a native view/component by name. + * A callback is provided to load the view config from UIManager. + * The callback is deferred until the view is actually rendered. + * This is done to avoid causing Prepack deopts. + */ + register(name: string, callback: ViewConfigGetter): string { invariant( - !viewConfigs.has(name), + !viewConfigCallbacks.has(name), 'Tried to register two views with the same name %s', name, ); - viewConfigs.set(name, viewConfig); + viewConfigCallbacks.set(name, callback); return name; }, - get(name: string) { - const config = viewConfigs.get(name); - invariant(config, 'View config not found for name %s', name); - return config; + + /** + * Retrieves a config for the specified view. + * If this is the first time the view has been used, + * This configuration will be lazy-loaded from UIManager. + */ + get(name: string): ReactNativeBaseComponentViewConfig { + let viewConfig; + if (!viewConfigs.has(name)) { + const callback = viewConfigCallbacks.get(name); + invariant( + typeof callback === 'function', + 'View config not found for name %s', + name, + ); + viewConfigCallbacks.set(name, null); + viewConfig = callback(); + viewConfigs.set(name, viewConfig); + } else { + viewConfig = viewConfigs.get(name); + } + invariant(viewConfig, 'View config not found for name %s', name); + return viewConfig; }, }; diff --git a/src/renderers/native/__tests__/ReactNativeEvents-test.js b/src/renderers/native/__tests__/ReactNativeEvents-test.js index 5a43e47c8d..c46a818298 100644 --- a/src/renderers/native/__tests__/ReactNativeEvents-test.js +++ b/src/renderers/native/__tests__/ReactNativeEvents-test.js @@ -34,10 +34,10 @@ beforeEach(() => { it('handles events', () => { expect(RCTEventEmitter.register.mock.calls.length).toBe(1); var EventEmitter = RCTEventEmitter.register.mock.calls[0][0]; - var View = createReactNativeComponentClass({ + var View = createReactNativeComponentClass('View', () => ({ validAttributes: {foo: true}, uiViewClassName: 'View', - }); + })); var log = []; ReactNative.render( @@ -94,10 +94,10 @@ it('handles events on text nodes', () => { expect(RCTEventEmitter.register.mock.calls.length).toBe(1); var EventEmitter = RCTEventEmitter.register.mock.calls[0][0]; - var Text = createReactNativeComponentClass({ + var Text = createReactNativeComponentClass('Text', () => ({ validAttributes: {foo: true}, uiViewClassName: 'Text', - }); + })); class ContextHack extends React.Component { static childContextTypes = {isInAParentText: PropTypes.bool}; @@ -179,10 +179,10 @@ it('handles events on text nodes', () => { it('handles when a responder is unmounted while a touch sequence is in progress', () => { var EventEmitter = RCTEventEmitter.register.mock.calls[0][0]; - var View = createReactNativeComponentClass({ + var View = createReactNativeComponentClass('View', () => ({ validAttributes: {id: true}, uiViewClassName: 'View', - }); + })); function getViewById(id) { return UIManager.createView.mock.calls.find( @@ -273,10 +273,10 @@ it('handles when a responder is unmounted while a touch sequence is in progress' it('handles events without target', () => { var EventEmitter = RCTEventEmitter.register.mock.calls[0][0]; - var View = createReactNativeComponentClass({ + var View = createReactNativeComponentClass('View', () => ({ validAttributes: {id: true}, uiViewClassName: 'View', - }); + })); function getViewById(id) { return UIManager.createView.mock.calls.find( diff --git a/src/renderers/native/__tests__/ReactNativeMount-test.js b/src/renderers/native/__tests__/ReactNativeMount-test.js index 5c434a7e49..d9e0414855 100644 --- a/src/renderers/native/__tests__/ReactNativeMount-test.js +++ b/src/renderers/native/__tests__/ReactNativeMount-test.js @@ -27,10 +27,10 @@ describe('ReactNative', () => { }); it('should be able to create and render a native component', () => { - var View = createReactNativeComponentClass({ + var View = createReactNativeComponentClass('View', () => ({ validAttributes: {foo: true}, uiViewClassName: 'View', - }); + })); ReactNative.render(, 1); expect(UIManager.createView).toBeCalled(); @@ -40,10 +40,10 @@ describe('ReactNative', () => { }); it('should be able to create and update a native component', () => { - var View = createReactNativeComponentClass({ + var View = createReactNativeComponentClass('View', () => ({ validAttributes: {foo: true}, uiViewClassName: 'View', - }); + })); ReactNative.render(, 11); @@ -61,10 +61,10 @@ describe('ReactNative', () => { }); it('should not call UIManager.updateView after render for properties that have not changed', () => { - const Text = createReactNativeComponentClass({ + const Text = createReactNativeComponentClass('Text', () => ({ validAttributes: {foo: true}, uiViewClassName: 'Text', - }); + })); ReactNative.render(1, 11); expect(UIManager.updateView).not.toBeCalled(); @@ -87,10 +87,10 @@ describe('ReactNative', () => { }); it('should not call UIManager.updateView from setNativeProps for properties that have not changed', () => { - const View = createReactNativeComponentClass({ + const View = createReactNativeComponentClass('View', () => ({ validAttributes: {foo: true}, uiViewClassName: 'View', - }); + })); class Subclass extends ReactNative.NativeComponent { render() { @@ -122,10 +122,10 @@ describe('ReactNative', () => { }); it('returns the correct instance and calls it in the callback', () => { - var View = createReactNativeComponentClass({ + var View = createReactNativeComponentClass('View', () => ({ validAttributes: {foo: true}, uiViewClassName: 'View', - }); + })); var a; var b; @@ -143,10 +143,10 @@ describe('ReactNative', () => { }); it('renders and reorders children', () => { - var View = createReactNativeComponentClass({ + var View = createReactNativeComponentClass('View', () => ({ validAttributes: {title: true}, uiViewClassName: 'View', - }); + })); class Component extends React.Component { render() { diff --git a/src/renderers/native/__tests__/createReactNativeComponentClass-test.js b/src/renderers/native/__tests__/createReactNativeComponentClass-test.js index 58470bdca4..7a225fb187 100644 --- a/src/renderers/native/__tests__/createReactNativeComponentClass-test.js +++ b/src/renderers/native/__tests__/createReactNativeComponentClass-test.js @@ -34,8 +34,14 @@ describe('createReactNativeComponentClass', () => { uiViewClassName: 'View', }; - const Text = createReactNativeComponentClass(textViewConfig); - const View = createReactNativeComponentClass(viewViewConfig); + const Text = createReactNativeComponentClass( + textViewConfig.uiViewClassName, + () => textViewConfig, + ); + const View = createReactNativeComponentClass( + viewViewConfig.uiViewClassName, + () => viewViewConfig, + ); expect(Text).not.toBe(View); @@ -53,10 +59,16 @@ describe('createReactNativeComponentClass', () => { uiViewClassName: 'Text', // Same }; - createReactNativeComponentClass(textViewConfig); + createReactNativeComponentClass( + textViewConfig.uiViewClassName, + () => textViewConfig, + ); expect(() => { - createReactNativeComponentClass(altTextViewConfig); + createReactNativeComponentClass( + altTextViewConfig.uiViewClassName, + () => altTextViewConfig, + ); }).toThrow('Tried to register two views with the same name Text'); }); }); diff --git a/src/renderers/native/createReactNativeComponentClass.js b/src/renderers/native/createReactNativeComponentClass.js index 3742475902..c9ed0d7d0d 100644 --- a/src/renderers/native/createReactNativeComponentClass.js +++ b/src/renderers/native/createReactNativeComponentClass.js @@ -14,21 +14,21 @@ const ReactNativeViewConfigRegistry = require('ReactNativeViewConfigRegistry'); -// See also ReactNativeBaseComponent -export type ReactNativeBaseComponentViewConfig = { - validAttributes: Object, - uiViewClassName: string, - propTypes?: Object, -}; +import type {ViewConfigGetter} from 'ReactNativeTypes'; /** + * Creates a renderable ReactNative host component. + * Use this method for view configs that are loaded from UIManager. + * Use createReactNativeComponentClass() for view configs defined within JavaScript. + * * @param {string} config iOS View configuration. * @private */ const createReactNativeComponentClass = function( - viewConfig: ReactNativeBaseComponentViewConfig, + name: string, + callback: ViewConfigGetter, ): string { - return ReactNativeViewConfigRegistry.register(viewConfig); + return ReactNativeViewConfigRegistry.register(name, callback); }; module.exports = createReactNativeComponentClass;