From 7f665da0eee37383cd2b50c82f490170d2979e10 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 20 Mar 2017 13:50:31 -0700 Subject: [PATCH] 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 | 216 +++++++++++------- .../native/NativeMethodsMixinUtils.js | 108 +++++++++ src/renderers/native/ReactNative.js | 7 +- src/renderers/native/ReactNativeFiber.js | 49 ++-- .../native/ReactNativeFiberHostComponent.js | 104 +++++++++ src/renderers/native/ReactNativeStack.js | 14 +- .../native/ReactNativeStackInjection.js | 6 +- .../native/createReactNativeComponentClass.js | 2 +- src/renderers/native/findNodeHandle.js | 5 +- 9 files changed, 389 insertions(+), 122 deletions(-) create mode 100644 src/renderers/native/NativeMethodsMixinUtils.js create mode 100644 src/renderers/native/ReactNativeFiberHostComponent.js diff --git a/src/renderers/native/NativeMethodsMixin.js b/src/renderers/native/NativeMethodsMixin.js index 25062816e3..893668fb25 100644 --- a/src/renderers/native/NativeMethodsMixin.js +++ b/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, - callback: ?Function, -): any { - return function() { - if ( - !callback || - (typeof context.isMounted === 'function' && !context.isMounted()) - ) { - return undefined; - } - return callback.apply(context, arguments); - }; -} - module.exports = NativeMethodsMixin; diff --git a/src/renderers/native/NativeMethodsMixinUtils.js b/src/renderers/native/NativeMethodsMixinUtils.js new file mode 100644 index 0000000000..68e4150869 --- /dev/null +++ b/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, +}; diff --git a/src/renderers/native/ReactNative.js b/src/renderers/native/ReactNative.js index 27c56d54ba..82b12de9a1 100644 --- a/src/renderers/native/ReactNative.js +++ b/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'); diff --git a/src/renderers/native/ReactNativeFiber.js b/src/renderers/native/ReactNativeFiber.js index d617e02691..783ae91397 100644 --- a/src/renderers/native/ReactNativeFiber.js +++ b/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, _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, containerTag: any, callback: ?Function) { let root = roots.get(containerTag); diff --git a/src/renderers/native/ReactNativeFiberHostComponent.js b/src/renderers/native/ReactNativeFiberHostComponent.js new file mode 100644 index 0000000000..0c0c69a65d --- /dev/null +++ b/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; + _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; diff --git a/src/renderers/native/ReactNativeStack.js b/src/renderers/native/ReactNativeStack.js index 2781d2e622..37d8b89f56 100644 --- a/src/renderers/native/ReactNativeStack.js +++ b/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, diff --git a/src/renderers/native/ReactNativeStackInjection.js b/src/renderers/native/ReactNativeStackInjection.js index d1127b86b3..5aa3d3d879 100644 --- a/src/renderers/native/ReactNativeStackInjection.js +++ b/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); diff --git a/src/renderers/native/createReactNativeComponentClass.js b/src/renderers/native/createReactNativeComponentClass.js index 9e4039a9f2..f216325828 100644 --- a/src/renderers/native/createReactNativeComponentClass.js +++ b/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 = { diff --git a/src/renderers/native/findNodeHandle.js b/src/renderers/native/findNodeHandle.js index 5b02c1054c..68ec7e503d 100644 --- a/src/renderers/native/findNodeHandle.js +++ b/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);