From 5f90178cd5f82b2d02ac799315317d95253ed305 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 8 Apr 2018 00:05:45 -0700 Subject: [PATCH 1/3] Move findNodeHandle into the renderers and use instantiation This is just like ReactDOM does it. This also lets us get rid of injection for findNodeHandle. Instead I move NativeMethodsMixin and ReactNativeComponent to use instantiation. --- .../src/__tests__/findDOMNode-test.js | 4 +- .../src/NativeMethodsMixin.js | 369 +++++++++--------- .../react-native-renderer/src/ReactFabric.js | 62 ++- .../src/ReactNativeComponent.js | 285 +++++++------- .../src/ReactNativeRenderer.js | 62 ++- .../src/findNodeHandle.js | 104 ----- .../src/findNumericNodeHandle.js | 28 -- 7 files changed, 450 insertions(+), 464 deletions(-) delete mode 100644 packages/react-native-renderer/src/findNodeHandle.js delete mode 100644 packages/react-native-renderer/src/findNumericNodeHandle.js diff --git a/packages/react-dom/src/__tests__/findDOMNode-test.js b/packages/react-dom/src/__tests__/findDOMNode-test.js index a6f494e9b7ac0..471880d81a407 100644 --- a/packages/react-dom/src/__tests__/findDOMNode-test.js +++ b/packages/react-dom/src/__tests__/findDOMNode-test.js @@ -65,9 +65,7 @@ describe('findDOMNode', () => { it('findDOMNode should reject random objects', () => { expect(function() { ReactDOM.findDOMNode({foo: 'bar'}); - }).toThrowError( - 'Element appears to be neither ReactComponent nor DOMNode. Keys: foo', - ); + }).toThrowError('Argument appears to not be a ReactComponent. Keys: foo'); }); it('findDOMNode should reject unmounted objects with render func', () => { diff --git a/packages/react-native-renderer/src/NativeMethodsMixin.js b/packages/react-native-renderer/src/NativeMethodsMixin.js index aec0442934831..d6b3cdfe7c5c6 100644 --- a/packages/react-native-renderer/src/NativeMethodsMixin.js +++ b/packages/react-native-renderer/src/NativeMethodsMixin.js @@ -26,192 +26,203 @@ import { throwOnStylesProp, warnForStyleProps, } from './NativeMethodsMixinUtils'; -import findNodeHandle from './findNodeHandle'; -import findNumericNodeHandle from './findNumericNodeHandle'; +import * as ReactInstanceMap from 'shared/ReactInstanceMap'; -/** - * `NativeMethodsMixin` provides methods to access the underlying native - * component directly. This can be useful in cases when you want to focus - * a view or measure its on-screen dimensions, for example. - * - * The methods described here are available on most of the default components - * provided by React Native. Note, however, that they are *not* available on - * composite components that aren't directly backed by a native view. This will - * generally include most components that you define in your own app. For more - * information, see [Direct - * Manipulation](docs/direct-manipulation.html). - * - * Note the Flow $Exact<> syntax is required to support mixins. - * React createClass mixins can only be used with exact types. - */ -const NativeMethodsMixin: $Exact = { +export default function( + findNodeHandle: any => ?number, + findHostInstance: any => any, +) { /** - * Determines the location on screen, width, and height of the given view and - * returns the values via an async callback. If successful, the callback will - * be called with the following arguments: - * - * - x - * - y - * - width - * - height - * - pageX - * - pageY + * `NativeMethodsMixin` provides methods to access the underlying native + * component directly. This can be useful in cases when you want to focus + * a view or measure its on-screen dimensions, for example. * - * Note that these measurements are not available until after the rendering - * has been completed in native. If you need the measurements as soon as - * possible, consider using the [`onLayout` - * prop](docs/view.html#onlayout) instead. - */ - measure: function(callback: MeasureOnSuccessCallback) { - UIManager.measure( - findNumericNodeHandle(this), - mountSafeCallback(this, callback), - ); - }, - - /** - * Determines the location of the given view in the window and returns the - * values via an async callback. If the React root view is embedded in - * another native view, this will give you the absolute coordinates. If - * successful, the callback will be called with the following - * arguments: + * The methods described here are available on most of the default components + * provided by React Native. Note, however, that they are *not* available on + * composite components that aren't directly backed by a native view. This will + * generally include most components that you define in your own app. For more + * information, see [Direct + * Manipulation](docs/direct-manipulation.html). * - * - x - * - y - * - width - * - height - * - * Note that these measurements are not available until after the rendering - * has been completed in native. - */ - measureInWindow: function(callback: MeasureInWindowOnSuccessCallback) { - UIManager.measureInWindow( - findNumericNodeHandle(this), - mountSafeCallback(this, callback), - ); - }, - - /** - * Like [`measure()`](#measure), but measures the view relative an ancestor, - * specified as `relativeToNativeNode`. This means that the returned x, y - * are relative to the origin x, y of the ancestor view. - * - * As always, to obtain a native node handle for a component, you can use - * `findNumericNodeHandle(component)`. - */ - measureLayout: function( - relativeToNativeNode: number, - onSuccess: MeasureLayoutOnSuccessCallback, - onFail: () => void /* currently unused */, - ) { - UIManager.measureLayout( - findNumericNodeHandle(this), - relativeToNativeNode, - mountSafeCallback(this, onFail), - mountSafeCallback(this, onSuccess), - ); - }, - - /** - * This function sends props straight to native. They will not participate in - * future diff process - this means that if you do not include them in the - * next render, they will remain active (see [Direct - * Manipulation](docs/direct-manipulation.html)). + * Note the Flow $Exact<> syntax is required to support mixins. + * React createClass mixins can only be used with exact types. */ - setNativeProps: function(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 findNumericNodeHandle() 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(this); - } 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); - } - - const updatePayload = ReactNativeAttributePayload.create( - nativeProps, - viewConfig.validAttributes, - ); - - // Avoid the overhead of bridge calls if there's no update. - // This is an expensive no-op for Android, and causes an unnecessary - // view invalidation for certain components (eg RCTTextInput) on iOS. - if (updatePayload != null) { - UIManager.updateView( - maybeInstance._nativeTag, - viewConfig.uiViewClassName, - updatePayload, + const NativeMethodsMixin: $Exact = { + /** + * Determines the location on screen, width, and height of the given view and + * returns the values via an async callback. If successful, the callback will + * be called with the following arguments: + * + * - x + * - y + * - width + * - height + * - pageX + * - pageY + * + * Note that these measurements are not available until after the rendering + * has been completed in native. If you need the measurements as soon as + * possible, consider using the [`onLayout` + * prop](docs/view.html#onlayout) instead. + */ + measure: function(callback: MeasureOnSuccessCallback) { + UIManager.measure( + findNodeHandle(this), + mountSafeCallback(this, callback), + ); + }, + + /** + * Determines the location of the given view in the window and returns the + * values via an async callback. If the React root view is embedded in + * another native view, this will give you the absolute coordinates. If + * successful, the callback will be called with the following + * arguments: + * + * - x + * - y + * - width + * - height + * + * Note that these measurements are not available until after the rendering + * has been completed in native. + */ + measureInWindow: function(callback: MeasureInWindowOnSuccessCallback) { + UIManager.measureInWindow( + findNodeHandle(this), + mountSafeCallback(this, callback), + ); + }, + + /** + * Like [`measure()`](#measure), but measures the view relative an ancestor, + * specified as `relativeToNativeNode`. This means that the returned x, y + * are relative to the origin x, y of the ancestor view. + * + * As always, to obtain a native node handle for a component, you can use + * `findNodeHandle(component)`. + */ + measureLayout: function( + relativeToNativeNode: number, + onSuccess: MeasureLayoutOnSuccessCallback, + onFail: () => void /* currently unused */, + ) { + UIManager.measureLayout( + findNodeHandle(this), + relativeToNativeNode, + mountSafeCallback(this, onFail), + mountSafeCallback(this, onSuccess), + ); + }, + + /** + * This function sends props straight to native. They will not participate in + * future diff process - this means that if you do not include them in the + * next render, they will remain active (see [Direct + * Manipulation](docs/direct-manipulation.html)). + */ + setNativeProps: function(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 findNodeHandle() because + // We want the instance/wrapper (not the native tag). + let maybeInstance; + + const fiber = ReactInstanceMap.get(this); + if (!fiber) { + return; + } + + // 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 = findHostInstance(fiber); + } 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); + } + + const updatePayload = ReactNativeAttributePayload.create( + nativeProps, + viewConfig.validAttributes, ); - } - }, - - /** - * Requests focus for the given input or view. The exact behavior triggered - * will depend on the platform and type of view. - */ - focus: function() { - TextInputState.focusTextInput(findNumericNodeHandle(this)); - }, - /** - * Removes focus from an input or view. This is the opposite of `focus()`. - */ - blur: function() { - TextInputState.blurTextInput(findNumericNodeHandle(this)); - }, -}; - -if (__DEV__) { - // hide this from Flow since we can't define these properties outside of - // __DEV__ without actually implementing them (setting them to undefined - // isn't allowed by ReactClass) - const NativeMethodsMixin_DEV = (NativeMethodsMixin: any); - invariant( - !NativeMethodsMixin_DEV.componentWillMount && - !NativeMethodsMixin_DEV.componentWillReceiveProps && - !NativeMethodsMixin_DEV.UNSAFE_componentWillMount && - !NativeMethodsMixin_DEV.UNSAFE_componentWillReceiveProps, - 'Do not override existing functions.', - ); - // TODO (bvaughn) Remove cWM and cWRP in a future version of React Native, - // Once these lifecycles have been remove from the reconciler. - NativeMethodsMixin_DEV.componentWillMount = function() { - throwOnStylesProp(this, this.props); - }; - NativeMethodsMixin_DEV.componentWillReceiveProps = function(newProps) { - throwOnStylesProp(this, newProps); - }; - NativeMethodsMixin_DEV.UNSAFE_componentWillMount = function() { - throwOnStylesProp(this, this.props); - }; - NativeMethodsMixin_DEV.UNSAFE_componentWillReceiveProps = function(newProps) { - throwOnStylesProp(this, newProps); + // Avoid the overhead of bridge calls if there's no update. + // This is an expensive no-op for Android, and causes an unnecessary + // view invalidation for certain components (eg RCTTextInput) on iOS. + if (updatePayload != null) { + UIManager.updateView( + maybeInstance._nativeTag, + viewConfig.uiViewClassName, + updatePayload, + ); + } + }, + + /** + * Requests focus for the given input or view. The exact behavior triggered + * will depend on the platform and type of view. + */ + focus: function() { + TextInputState.focusTextInput(findNodeHandle(this)); + }, + + /** + * Removes focus from an input or view. This is the opposite of `focus()`. + */ + blur: function() { + TextInputState.blurTextInput(findNodeHandle(this)); + }, }; - // React may warn about cWM/cWRP/cWU methods being deprecated. - // Add a flag to suppress these warnings for this special case. - // TODO (bvaughn) Remove this flag once the above methods have been removed. - NativeMethodsMixin_DEV.componentWillMount.__suppressDeprecationWarning = true; - NativeMethodsMixin_DEV.componentWillReceiveProps.__suppressDeprecationWarning = true; + if (__DEV__) { + // hide this from Flow since we can't define these properties outside of + // __DEV__ without actually implementing them (setting them to undefined + // isn't allowed by ReactClass) + const NativeMethodsMixin_DEV = (NativeMethodsMixin: any); + invariant( + !NativeMethodsMixin_DEV.componentWillMount && + !NativeMethodsMixin_DEV.componentWillReceiveProps && + !NativeMethodsMixin_DEV.UNSAFE_componentWillMount && + !NativeMethodsMixin_DEV.UNSAFE_componentWillReceiveProps, + 'Do not override existing functions.', + ); + // TODO (bvaughn) Remove cWM and cWRP in a future version of React Native, + // Once these lifecycles have been remove from the reconciler. + NativeMethodsMixin_DEV.componentWillMount = function() { + throwOnStylesProp(this, this.props); + }; + NativeMethodsMixin_DEV.componentWillReceiveProps = function(newProps) { + throwOnStylesProp(this, newProps); + }; + NativeMethodsMixin_DEV.UNSAFE_componentWillMount = function() { + throwOnStylesProp(this, this.props); + }; + NativeMethodsMixin_DEV.UNSAFE_componentWillReceiveProps = function( + newProps, + ) { + throwOnStylesProp(this, newProps); + }; + + // React may warn about cWM/cWRP/cWU methods being deprecated. + // Add a flag to suppress these warnings for this special case. + // TODO (bvaughn) Remove this flag once the above methods have been removed. + NativeMethodsMixin_DEV.componentWillMount.__suppressDeprecationWarning = true; + NativeMethodsMixin_DEV.componentWillReceiveProps.__suppressDeprecationWarning = true; + } + + return NativeMethodsMixin; } - -export default NativeMethodsMixin; diff --git a/packages/react-native-renderer/src/ReactFabric.js b/packages/react-native-renderer/src/ReactFabric.js index edd6047270989..3bac58d7fac00 100644 --- a/packages/react-native-renderer/src/ReactFabric.js +++ b/packages/react-native-renderer/src/ReactFabric.js @@ -21,19 +21,69 @@ import ReactNativeComponent from './ReactNativeComponent'; import * as ReactNativeComponentTree from './ReactNativeComponentTree'; import ReactFabricRenderer from './ReactFabricRenderer'; import {getInspectorDataForViewTag} from './ReactNativeFiberInspector'; -import {injectFindHostInstance} from './findNodeHandle'; -import findNumericNodeHandle from './findNumericNodeHandle'; -injectFindHostInstance(ReactFabricRenderer.findHostInstance); +import {ReactCurrentOwner} from 'shared/ReactGlobalSharedState'; +import type {Fiber} from 'react-reconciler/src/ReactFiber'; +import * as ReactInstanceMap from 'shared/ReactInstanceMap'; +import getComponentName from 'shared/getComponentName'; +import invariant from 'fbjs/lib/invariant'; +import warning from 'fbjs/lib/warning'; + +const findHostInstance = ReactFabricRenderer.findHostInstance; + +function findNodeHandle(componentOrHandle: any): ?number { + if (__DEV__) { + const owner = ReactCurrentOwner.current; + if (owner !== null && owner.stateNode !== null) { + warning( + owner.stateNode._warnedAboutRefsInRender, + '%s is accessing findNodeHandle inside its render(). ' + + 'render() should be a pure function of props and state. It should ' + + 'never access something that requires stale data from the previous ' + + 'render, such as refs. Move this logic to componentDidMount and ' + + 'componentDidUpdate instead.', + getComponentName(owner) || 'A component', + ); + + owner.stateNode._warnedAboutRefsInRender = true; + } + } + if (componentOrHandle == null) { + return null; + } + if (typeof componentOrHandle === 'number') { + // Already a node handle + return componentOrHandle; + } + if (componentOrHandle._nativeTag) { + return componentOrHandle._nativeTag; + } + if (componentOrHandle.canonical && componentOrHandle.canonical._nativeTag) { + return componentOrHandle.canonical._nativeTag; + } + const internalInstance: Fiber = ReactInstanceMap.get(componentOrHandle); + if (!internalInstance) { + return null; + } + const hostInstance = findHostInstance(internalInstance); + if (hostInstance == null) { + return hostInstance; + } + if (hostInstance.canonical) { + // Fabric + return hostInstance.canonical._nativeTag; + } + return hostInstance._nativeTag; +} ReactGenericBatching.injection.injectRenderer(ReactFabricRenderer); const roots = new Map(); const ReactFabric: ReactFabricType = { - NativeComponent: ReactNativeComponent, + NativeComponent: ReactNativeComponent(findNodeHandle, findHostInstance), - findNodeHandle: findNumericNodeHandle, + findNodeHandle, render(element: React$Element, containerTag: any, callback: ?Function) { let root = roots.get(containerTag); @@ -69,7 +119,7 @@ const ReactFabric: ReactFabricType = { __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED: { // Used as a mixin in many createClass-based components - NativeMethodsMixin, + NativeMethodsMixin: NativeMethodsMixin(findNodeHandle, findHostInstance), // Used by react-native-github/Libraries/ components ReactNativeComponentTree, // ScrollResponder }, diff --git a/packages/react-native-renderer/src/ReactNativeComponent.js b/packages/react-native-renderer/src/ReactNativeComponent.js index 4aefbd0282c49..a8f225aaadd84 100644 --- a/packages/react-native-renderer/src/ReactNativeComponent.js +++ b/packages/react-native-renderer/src/ReactNativeComponent.js @@ -23,156 +23,165 @@ import UIManager from 'UIManager'; import * as ReactNativeAttributePayload from './ReactNativeAttributePayload'; import {mountSafeCallback} from './NativeMethodsMixinUtils'; -import findNodeHandle from './findNodeHandle'; -import findNumericNodeHandle from './findNumericNodeHandle'; - -/** - * Superclass that provides methods to access the underlying native component. - * This can be useful when you want to focus a view or measure its dimensions. - * - * Methods implemented by this class are available on most default components - * provided by React Native. However, they are *not* available on composite - * components that are not directly backed by a native view. For more - * information, see [Direct Manipulation](docs/direct-manipulation.html). - * - * @abstract - */ -class ReactNativeComponent extends React.Component< - Props, - State, -> { - /** - * Due to bugs in Flow's handling of React.createClass, some fields already - * declared in the base class need to be redeclared below. - */ - props: Props; - state: State; +import * as ReactInstanceMap from 'shared/ReactInstanceMap'; +export default function( + findNodeHandle: any => ?number, + findHostInstance: any => any, +) { /** - * Removes focus. This is the opposite of `focus()`. - */ - blur(): void { - TextInputState.blurTextInput(findNumericNodeHandle(this)); - } - - /** - * Requests focus. The exact behavior depends on the platform and view. - */ - focus(): void { - TextInputState.focusTextInput(findNumericNodeHandle(this)); - } - - /** - * Measures the on-screen location and dimensions. If successful, the callback - * will be called asynchronously with the following arguments: + * Superclass that provides methods to access the underlying native component. + * This can be useful when you want to focus a view or measure its dimensions. * - * - x - * - y - * - width - * - height - * - pageX - * - pageY + * Methods implemented by this class are available on most default components + * provided by React Native. However, they are *not* available on composite + * components that are not directly backed by a native view. For more + * information, see [Direct Manipulation](docs/direct-manipulation.html). * - * These values are not available until after natives rendering completes. If - * you need the measurements as soon as possible, consider using the - * [`onLayout` prop](docs/view.html#onlayout) instead. + * @abstract */ - measure(callback: MeasureOnSuccessCallback): void { - UIManager.measure( - findNumericNodeHandle(this), - mountSafeCallback(this, callback), - ); - } + class ReactNativeComponent extends React.Component< + Props, + State, + > { + /** + * Due to bugs in Flow's handling of React.createClass, some fields already + * declared in the base class need to be redeclared below. + */ + props: Props; + state: State; + + /** + * Removes focus. This is the opposite of `focus()`. + */ + blur(): void { + TextInputState.blurTextInput(findNodeHandle(this)); + } - /** - * Measures the on-screen location and dimensions. Even if the React Native - * root view is embedded within another native view, this method will give you - * the absolute coordinates measured from the window. If successful, the - * callback will be called asynchronously with the following arguments: - * - * - x - * - y - * - width - * - height - * - * These values are not available until after natives rendering completes. - */ - measureInWindow(callback: MeasureInWindowOnSuccessCallback): void { - UIManager.measureInWindow( - findNumericNodeHandle(this), - mountSafeCallback(this, callback), - ); - } + /** + * Requests focus. The exact behavior depends on the platform and view. + */ + focus(): void { + TextInputState.focusTextInput(findNodeHandle(this)); + } - /** - * Similar to [`measure()`](#measure), but the resulting location will be - * relative to the supplied ancestor's location. - * - * Obtain a native node handle with `ReactNative.findNodeHandle(component)`. - */ - measureLayout( - relativeToNativeNode: number, - onSuccess: MeasureLayoutOnSuccessCallback, - onFail: () => void /* currently unused */, - ): void { - UIManager.measureLayout( - findNumericNodeHandle(this), - relativeToNativeNode, - mountSafeCallback(this, onFail), - mountSafeCallback(this, onSuccess), - ); - } + /** + * Measures the on-screen location and dimensions. If successful, the callback + * will be called asynchronously with the following arguments: + * + * - x + * - y + * - width + * - height + * - pageX + * - pageY + * + * These values are not available until after natives rendering completes. If + * you need the measurements as soon as possible, consider using the + * [`onLayout` prop](docs/view.html#onlayout) instead. + */ + measure(callback: MeasureOnSuccessCallback): void { + UIManager.measure( + findNodeHandle(this), + mountSafeCallback(this, callback), + ); + } - /** - * This function sends props straight to native. They will not participate in - * future diff process - this means that if you do not include them in the - * next render, they will remain active (see [Direct - * Manipulation](docs/direct-manipulation.html)). - */ - setNativeProps(nativeProps: Object): void { - // 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(this); - } 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; + /** + * Measures the on-screen location and dimensions. Even if the React Native + * root view is embedded within another native view, this method will give you + * the absolute coordinates measured from the window. If successful, the + * callback will be called asynchronously with the following arguments: + * + * - x + * - y + * - width + * - height + * + * These values are not available until after natives rendering completes. + */ + measureInWindow(callback: MeasureInWindowOnSuccessCallback): void { + UIManager.measureInWindow( + findNodeHandle(this), + mountSafeCallback(this, callback), + ); } - const viewConfig: ReactNativeBaseComponentViewConfig = - maybeInstance.viewConfig || maybeInstance.canonical.viewConfig; - - const updatePayload = ReactNativeAttributePayload.create( - nativeProps, - viewConfig.validAttributes, - ); - - // Avoid the overhead of bridge calls if there's no update. - // This is an expensive no-op for Android, and causes an unnecessary - // view invalidation for certain components (eg RCTTextInput) on iOS. - if (updatePayload != null) { - UIManager.updateView( - maybeInstance._nativeTag, - viewConfig.uiViewClassName, - updatePayload, + /** + * Similar to [`measure()`](#measure), but the resulting location will be + * relative to the supplied ancestor's location. + * + * Obtain a native node handle with `ReactNative.findNodeHandle(component)`. + */ + measureLayout( + relativeToNativeNode: number, + onSuccess: MeasureLayoutOnSuccessCallback, + onFail: () => void /* currently unused */, + ): void { + UIManager.measureLayout( + findNodeHandle(this), + relativeToNativeNode, + mountSafeCallback(this, onFail), + mountSafeCallback(this, onSuccess), ); } + + /** + * This function sends props straight to native. They will not participate in + * future diff process - this means that if you do not include them in the + * next render, they will remain active (see [Direct + * Manipulation](docs/direct-manipulation.html)). + */ + setNativeProps(nativeProps: Object): void { + // 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; + + const fiber = ReactInstanceMap.get(this); + if (!fiber) { + return; + } + + // 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 = findHostInstance(fiber); + } 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 || maybeInstance.canonical.viewConfig; + + const updatePayload = ReactNativeAttributePayload.create( + nativeProps, + viewConfig.validAttributes, + ); + + // Avoid the overhead of bridge calls if there's no update. + // This is an expensive no-op for Android, and causes an unnecessary + // view invalidation for certain components (eg RCTTextInput) on iOS. + if (updatePayload != null) { + UIManager.updateView( + maybeInstance._nativeTag, + viewConfig.uiViewClassName, + updatePayload, + ); + } + } } -} -// eslint-disable-next-line no-unused-expressions -(ReactNativeComponent.prototype: NativeMethodsMixinType); + // eslint-disable-next-line no-unused-expressions + (ReactNativeComponent.prototype: NativeMethodsMixinType); -export default ReactNativeComponent; + return ReactNativeComponent; +} diff --git a/packages/react-native-renderer/src/ReactNativeRenderer.js b/packages/react-native-renderer/src/ReactNativeRenderer.js index faef67d42993b..c988005bef74a 100644 --- a/packages/react-native-renderer/src/ReactNativeRenderer.js +++ b/packages/react-native-renderer/src/ReactNativeRenderer.js @@ -25,10 +25,60 @@ import ReactNativeComponent from './ReactNativeComponent'; import * as ReactNativeComponentTree from './ReactNativeComponentTree'; import ReactNativeFiberRenderer from './ReactNativeFiberRenderer'; import {getInspectorDataForViewTag} from './ReactNativeFiberInspector'; -import {injectFindHostInstance} from './findNodeHandle'; -import findNumericNodeHandle from './findNumericNodeHandle'; -injectFindHostInstance(ReactNativeFiberRenderer.findHostInstance); +import {ReactCurrentOwner} from 'shared/ReactGlobalSharedState'; +import type {Fiber} from 'react-reconciler/src/ReactFiber'; +import * as ReactInstanceMap from 'shared/ReactInstanceMap'; +import getComponentName from 'shared/getComponentName'; +import invariant from 'fbjs/lib/invariant'; +import warning from 'fbjs/lib/warning'; + +const findHostInstance = ReactNativeFiberRenderer.findHostInstance; + +function findNodeHandle(componentOrHandle: any): ?number { + if (__DEV__) { + const owner = ReactCurrentOwner.current; + if (owner !== null && owner.stateNode !== null) { + warning( + owner.stateNode._warnedAboutRefsInRender, + '%s is accessing findNodeHandle inside its render(). ' + + 'render() should be a pure function of props and state. It should ' + + 'never access something that requires stale data from the previous ' + + 'render, such as refs. Move this logic to componentDidMount and ' + + 'componentDidUpdate instead.', + getComponentName(owner) || 'A component', + ); + + owner.stateNode._warnedAboutRefsInRender = true; + } + } + if (componentOrHandle == null) { + return null; + } + if (typeof componentOrHandle === 'number') { + // Already a node handle + return componentOrHandle; + } + if (componentOrHandle._nativeTag) { + return componentOrHandle._nativeTag; + } + if (componentOrHandle.canonical && componentOrHandle.canonical._nativeTag) { + return componentOrHandle.canonical._nativeTag; + } + const internalInstance: Fiber = ReactInstanceMap.get(componentOrHandle); + if (!internalInstance) { + return null; + } + const hostInstance = findHostInstance(internalInstance); + if (hostInstance == null) { + return hostInstance; + } + if (hostInstance.canonical) { + // Fabric + return hostInstance.canonical._nativeTag; + } + return hostInstance._nativeTag; +} ReactGenericBatching.injection.injectRenderer(ReactNativeFiberRenderer); @@ -43,9 +93,9 @@ function computeComponentStackForErrorReporting(reactTag: number): string { const roots = new Map(); const ReactNativeRenderer: ReactNativeType = { - NativeComponent: ReactNativeComponent, + NativeComponent: ReactNativeComponent(findNodeHandle, findHostInstance), - findNodeHandle: findNumericNodeHandle, + findNodeHandle, render(element: React$Element, containerTag: any, callback: ?Function) { let root = roots.get(containerTag); @@ -94,7 +144,7 @@ const ReactNativeRenderer: ReactNativeType = { __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED: { // Used as a mixin in many createClass-based components - NativeMethodsMixin, + NativeMethodsMixin: NativeMethodsMixin(findNodeHandle, findHostInstance), // Used by react-native-github/Libraries/ components ReactNativeComponentTree, // ScrollResponder computeComponentStackForErrorReporting, diff --git a/packages/react-native-renderer/src/findNodeHandle.js b/packages/react-native-renderer/src/findNodeHandle.js deleted file mode 100644 index 883b45f919fbe..0000000000000 --- a/packages/react-native-renderer/src/findNodeHandle.js +++ /dev/null @@ -1,104 +0,0 @@ -/** - * Copyright (c) 2015-present, Facebook, Inc. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @flow - */ - -import type {Fiber} from 'react-reconciler/src/ReactFiber'; - -import * as ReactInstanceMap from 'shared/ReactInstanceMap'; -import {ReactCurrentOwner} from 'shared/ReactGlobalSharedState'; -import getComponentName from 'shared/getComponentName'; -import invariant from 'fbjs/lib/invariant'; -import warning from 'fbjs/lib/warning'; - -let findHostInstance = function(fiber: Fiber): any { - return null; -}; - -export function injectFindHostInstance(impl: (fiber: Fiber) => any) { - findHostInstance = impl; -} - -/** - * ReactNative vs ReactWeb - * ----------------------- - * React treats some pieces of data opaquely. This means that the information - * is first class (it can be passed around), but cannot be inspected. This - * allows us to build infrastructure that reasons about resources, without - * making assumptions about the nature of those resources, and this allows that - * infra to be shared across multiple platforms, where the resources are very - * different. General infra (such as `ReactMultiChild`) reasons opaquely about - * the data, but platform specific code (such as `ReactNativeBaseComponent`) can - * make assumptions about the data. - * - * - * `rootNodeID`, uniquely identifies a position in the generated native view - * tree. Many layers of composite components (created with `React.createClass`) - * can all share the same `rootNodeID`. - * - * `nodeHandle`: A sufficiently unambiguous way to refer to a lower level - * resource (dom node, native view etc). The `rootNodeID` is sufficient for web - * `nodeHandle`s, because the position in a tree is always enough to uniquely - * identify a DOM node (we never have nodes in some bank outside of the - * document). The same would be true for `ReactNative`, but we must maintain a - * mapping that we can send efficiently serializable - * strings across native boundaries. - * - * Opaque name TodaysWebReact FutureWebWorkerReact ReactNative - * ---------------------------------------------------------------------------- - * nodeHandle N/A rootNodeID tag - */ - -// 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__) { - const owner = ReactCurrentOwner.current; - if (owner !== null && owner.stateNode !== null) { - warning( - owner.stateNode._warnedAboutRefsInRender, - '%s is accessing findNodeHandle inside its render(). ' + - 'render() should be a pure function of props and state. It should ' + - 'never access something that requires stale data from the previous ' + - 'render, such as refs. Move this logic to componentDidMount and ' + - 'componentDidUpdate instead.', - getComponentName(owner) || 'A component', - ); - - owner.stateNode._warnedAboutRefsInRender = true; - } - } - if (componentOrHandle == null) { - return null; - } - if (typeof componentOrHandle === 'number') { - // Already a node handle - return componentOrHandle; - } - - const component = componentOrHandle; - - // TODO (sophiebits): Wrap iOS native components in a composite wrapper, then - // ReactInstanceMap.get here will always succeed for mounted components - const internalInstance: Fiber = ReactInstanceMap.get(component); - if (internalInstance) { - return findHostInstance(internalInstance); - } else { - if (component) { - return component; - } else { - invariant( - false, - 'findNodeHandle(...): Unable to find node handle for unmounted ' + - 'component.', - ); - } - } -} - -export default findNodeHandle; diff --git a/packages/react-native-renderer/src/findNumericNodeHandle.js b/packages/react-native-renderer/src/findNumericNodeHandle.js deleted file mode 100644 index 910950c880d90..0000000000000 --- a/packages/react-native-renderer/src/findNumericNodeHandle.js +++ /dev/null @@ -1,28 +0,0 @@ -/** - * Copyright (c) 2015-present, Facebook, Inc. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @flow - */ - -import findNodeHandle from './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. - */ -export default function findNumericNodeHandleFiber( - componentOrHandle: any, -): ?number { - const instance: any = findNodeHandle(componentOrHandle); - if (instance == null || typeof instance === 'number') { - return instance; - } - if (instance.canonical) { - return instance.canonical._nativeTag; - } - return instance._nativeTag; -} From 2e2fa9c1102c1172eadc2438571e9a371c60dd7a Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 8 Apr 2018 01:02:34 -0700 Subject: [PATCH 2/3] Refactor findHostInstance The reconciler shouldn't expose the Fiber data structure. We should pass the component instance to the reconciler, since the reconciler is the thing that is supposed to be instancemap aware. --- packages/react-dom/src/client/ReactDOM.js | 15 +-------------- .../src/NativeMethodsMixin.js | 8 +------- .../react-native-renderer/src/ReactFabric.js | 9 +-------- .../src/ReactNativeComponent.js | 8 +------- .../src/ReactNativeRenderer.js | 9 +-------- packages/react-noop-renderer/src/ReactNoop.js | 4 +--- .../src/ReactFiberReconciler.js | 17 +++++++++++++++-- 7 files changed, 21 insertions(+), 49 deletions(-) diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js index 8a03d03a8bea4..1bb0c86ed24b7 100644 --- a/packages/react-dom/src/client/ReactDOM.js +++ b/packages/react-dom/src/client/ReactDOM.js @@ -1147,20 +1147,7 @@ const ReactDOM: Object = { return (componentOrElement: any); } - const inst = ReactInstanceMap.get(componentOrElement); - if (inst) { - return DOMRenderer.findHostInstance(inst); - } - - if (typeof componentOrElement.render === 'function') { - invariant(false, 'Unable to find node on an unmounted component.'); - } else { - invariant( - false, - 'Element appears to be neither ReactComponent nor DOMNode. Keys: %s', - Object.keys(componentOrElement), - ); - } + return DOMRenderer.findHostInstance(componentOrElement); }, hydrate(element: React$Node, container: DOMContainer, callback: ?Function) { diff --git a/packages/react-native-renderer/src/NativeMethodsMixin.js b/packages/react-native-renderer/src/NativeMethodsMixin.js index d6b3cdfe7c5c6..c7caa562c51b2 100644 --- a/packages/react-native-renderer/src/NativeMethodsMixin.js +++ b/packages/react-native-renderer/src/NativeMethodsMixin.js @@ -26,7 +26,6 @@ import { throwOnStylesProp, warnForStyleProps, } from './NativeMethodsMixinUtils'; -import * as ReactInstanceMap from 'shared/ReactInstanceMap'; export default function( findNodeHandle: any => ?number, @@ -129,17 +128,12 @@ export default function( // We want the instance/wrapper (not the native tag). let maybeInstance; - const fiber = ReactInstanceMap.get(this); - if (!fiber) { - return; - } - // 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 = findHostInstance(fiber); + maybeInstance = findHostInstance(this); } catch (error) {} // If there is no host component beneath this we should fail silently. diff --git a/packages/react-native-renderer/src/ReactFabric.js b/packages/react-native-renderer/src/ReactFabric.js index 3bac58d7fac00..9e2c82548d2b5 100644 --- a/packages/react-native-renderer/src/ReactFabric.js +++ b/packages/react-native-renderer/src/ReactFabric.js @@ -23,10 +23,7 @@ import ReactFabricRenderer from './ReactFabricRenderer'; import {getInspectorDataForViewTag} from './ReactNativeFiberInspector'; import {ReactCurrentOwner} from 'shared/ReactGlobalSharedState'; -import type {Fiber} from 'react-reconciler/src/ReactFiber'; -import * as ReactInstanceMap from 'shared/ReactInstanceMap'; import getComponentName from 'shared/getComponentName'; -import invariant from 'fbjs/lib/invariant'; import warning from 'fbjs/lib/warning'; const findHostInstance = ReactFabricRenderer.findHostInstance; @@ -61,11 +58,7 @@ function findNodeHandle(componentOrHandle: any): ?number { if (componentOrHandle.canonical && componentOrHandle.canonical._nativeTag) { return componentOrHandle.canonical._nativeTag; } - const internalInstance: Fiber = ReactInstanceMap.get(componentOrHandle); - if (!internalInstance) { - return null; - } - const hostInstance = findHostInstance(internalInstance); + const hostInstance = findHostInstance(componentOrHandle); if (hostInstance == null) { return hostInstance; } diff --git a/packages/react-native-renderer/src/ReactNativeComponent.js b/packages/react-native-renderer/src/ReactNativeComponent.js index a8f225aaadd84..2b38cc89d6cd3 100644 --- a/packages/react-native-renderer/src/ReactNativeComponent.js +++ b/packages/react-native-renderer/src/ReactNativeComponent.js @@ -23,7 +23,6 @@ import UIManager from 'UIManager'; import * as ReactNativeAttributePayload from './ReactNativeAttributePayload'; import {mountSafeCallback} from './NativeMethodsMixinUtils'; -import * as ReactInstanceMap from 'shared/ReactInstanceMap'; export default function( findNodeHandle: any => ?number, @@ -140,17 +139,12 @@ export default function( // We want the instance/wrapper (not the native tag). let maybeInstance; - const fiber = ReactInstanceMap.get(this); - if (!fiber) { - return; - } - // 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 = findHostInstance(fiber); + maybeInstance = findHostInstance(this); } catch (error) {} // If there is no host component beneath this we should fail silently. diff --git a/packages/react-native-renderer/src/ReactNativeRenderer.js b/packages/react-native-renderer/src/ReactNativeRenderer.js index c988005bef74a..f8e984d0e63fd 100644 --- a/packages/react-native-renderer/src/ReactNativeRenderer.js +++ b/packages/react-native-renderer/src/ReactNativeRenderer.js @@ -27,10 +27,7 @@ import ReactNativeFiberRenderer from './ReactNativeFiberRenderer'; import {getInspectorDataForViewTag} from './ReactNativeFiberInspector'; import {ReactCurrentOwner} from 'shared/ReactGlobalSharedState'; -import type {Fiber} from 'react-reconciler/src/ReactFiber'; -import * as ReactInstanceMap from 'shared/ReactInstanceMap'; import getComponentName from 'shared/getComponentName'; -import invariant from 'fbjs/lib/invariant'; import warning from 'fbjs/lib/warning'; const findHostInstance = ReactNativeFiberRenderer.findHostInstance; @@ -65,11 +62,7 @@ function findNodeHandle(componentOrHandle: any): ?number { if (componentOrHandle.canonical && componentOrHandle.canonical._nativeTag) { return componentOrHandle.canonical._nativeTag; } - const internalInstance: Fiber = ReactInstanceMap.get(componentOrHandle); - if (!internalInstance) { - return null; - } - const hostInstance = findHostInstance(internalInstance); + const hostInstance = findHostInstance(componentOrHandle); if (hostInstance == null) { return hostInstance; } diff --git a/packages/react-noop-renderer/src/ReactNoop.js b/packages/react-noop-renderer/src/ReactNoop.js index 15eef0fb762ed..aa5e91bdc23f7 100644 --- a/packages/react-noop-renderer/src/ReactNoop.js +++ b/packages/react-noop-renderer/src/ReactNoop.js @@ -19,7 +19,6 @@ import type {UpdateQueue} from 'react-reconciler/src/ReactFiberUpdateQueue'; import type {ReactNodeList} from 'shared/ReactTypes'; import ReactFiberReconciler from 'react-reconciler'; import {enablePersistentReconciler} from 'shared/ReactFeatureFlags'; -import * as ReactInstanceMap from 'shared/ReactInstanceMap'; import * as ReactPortal from 'shared/ReactPortal'; import emptyObject from 'fbjs/lib/emptyObject'; import expect from 'expect'; @@ -401,8 +400,7 @@ const ReactNoop = { if (typeof component.id === 'number') { return component; } - const inst = ReactInstanceMap.get(component); - return inst ? NoopRenderer.findHostInstance(inst) : null; + return NoopRenderer.findHostInstance(component); }, flushDeferredPri(timeout: number = Infinity): Array { diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index 09c1b21d9599a..ca774c834adb5 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -20,6 +20,7 @@ import * as ReactInstanceMap from 'shared/ReactInstanceMap'; import {HostComponent} from 'shared/ReactTypeOfWork'; import emptyObject from 'fbjs/lib/emptyObject'; import getComponentName from 'shared/getComponentName'; +import invariant from 'fbjs/lib/invariant'; import warning from 'fbjs/lib/warning'; import {createFiberRoot} from './ReactFiberRoot'; @@ -264,7 +265,7 @@ export type Reconciler = { ): React$Component | TI | I | null, // Use for findDOMNode/findHostNode. Legacy API. - findHostInstance(component: Fiber): I | TI | null, + findHostInstance(component: Object): I | TI | null, // Used internally for filtering out portals. Legacy API. findHostInstanceWithNoPortals(component: Fiber): I | TI | null, @@ -402,7 +403,19 @@ export default function( ); } - function findHostInstance(fiber: Fiber): PI | null { + function findHostInstance(component: Object): PI | null { + const fiber = ReactInstanceMap.get(component); + if (fiber === undefined) { + if (typeof component.render === 'function') { + invariant(false, 'Unable to find node on an unmounted component.'); + } else { + invariant( + false, + 'Argument appears to not be a ReactComponent. Keys: %s', + Object.keys(component), + ); + } + } const hostFiber = findCurrentHostFiber(fiber); if (hostFiber === null) { return null; From cf454c58eaa3d628529a668ac7f63882f7245a18 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 9 Apr 2018 18:55:33 -0700 Subject: [PATCH 3/3] Fix devtools injection --- packages/react-reconciler/src/ReactFiberReconciler.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index ca774c834adb5..8118675232b37 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -521,7 +521,11 @@ export default function( return ReactFiberDevToolsHook.injectInternals({ ...devToolsConfig, findHostInstanceByFiber(fiber: Fiber): I | TI | null { - return findHostInstance(fiber); + const hostFiber = findCurrentHostFiber(fiber); + if (hostFiber === null) { + return null; + } + return hostFiber.stateNode; }, findFiberByHostInstance(instance: I | TI): Fiber | null { if (!findFiberByHostInstance) {