diff --git a/packages/react-native-renderer/src/NativeMethodsMixin.js b/packages/react-native-renderer/src/NativeMethodsMixin.js index 5df51392a993d..97d659aa2b251 100644 --- a/packages/react-native-renderer/src/NativeMethodsMixin.js +++ b/packages/react-native-renderer/src/NativeMethodsMixin.js @@ -18,6 +18,7 @@ import type { import invariant from 'shared/invariant'; // Modules provided by RN: import TextInputState from 'TextInputState'; +import * as FabricUIManager from 'FabricUIManager'; import UIManager from 'UIManager'; import {create} from './ReactNativeAttributePayload'; @@ -68,10 +69,33 @@ export default function( * prop](docs/view.html#onlayout) instead. */ measure: function(callback: MeasureOnSuccessCallback) { - UIManager.measure( - findNodeHandle(this), - mountSafeCallback_NOT_REALLY_SAFE(this, callback), - ); + 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 = findHostInstance(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; + } + + if (maybeInstance.canonical) { + FabricUIManager.measure( + maybeInstance.node, + mountSafeCallback_NOT_REALLY_SAFE(this, callback), + ); + } else { + UIManager.measure( + findNodeHandle(this), + mountSafeCallback_NOT_REALLY_SAFE(this, callback), + ); + } }, /** @@ -90,10 +114,33 @@ export default function( * has been completed in native. */ measureInWindow: function(callback: MeasureInWindowOnSuccessCallback) { - UIManager.measureInWindow( - findNodeHandle(this), - mountSafeCallback_NOT_REALLY_SAFE(this, callback), - ); + 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 = findHostInstance(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; + } + + if (maybeInstance.canonical) { + FabricUIManager.measureInWindow( + maybeInstance.node, + mountSafeCallback_NOT_REALLY_SAFE(this, callback), + ); + } else { + UIManager.measureInWindow( + findNodeHandle(this), + mountSafeCallback_NOT_REALLY_SAFE(this, callback), + ); + } }, /** @@ -105,16 +152,60 @@ export default function( * `findNodeHandle(component)`. */ measureLayout: function( - relativeToNativeNode: number, + relativeToNativeNode: number | Object, onSuccess: MeasureLayoutOnSuccessCallback, onFail: () => void /* currently unused */, ) { - UIManager.measureLayout( - findNodeHandle(this), - relativeToNativeNode, - mountSafeCallback_NOT_REALLY_SAFE(this, onFail), - mountSafeCallback_NOT_REALLY_SAFE(this, onSuccess), - ); + 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 = findHostInstance(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; + } + + if (maybeInstance.canonical) { + warningWithoutStack( + false, + 'Warning: measureLayout on components using NativeMethodsMixin ' + + 'or ReactNative.NativeComponent is not currently supported in Fabric. ' + + 'measureLayout must be called on a native ref. Consider using forwardRef.', + ); + return; + } else { + let relativeNode; + + if (typeof relativeToNativeNode === 'number') { + // Already a node handle + relativeNode = relativeToNativeNode; + } else if (relativeToNativeNode._nativeTag) { + relativeNode = relativeToNativeNode._nativeTag; + } + + if (relativeNode == null) { + warningWithoutStack( + false, + 'Warning: ref.measureLayout must be called with a node handle or a ref to a native component.', + ); + + return; + } + + UIManager.measureLayout( + findNodeHandle(this), + relativeNode, + mountSafeCallback_NOT_REALLY_SAFE(this, onFail), + mountSafeCallback_NOT_REALLY_SAFE(this, onSuccess), + ); + } }, /** diff --git a/packages/react-native-renderer/src/ReactFabricHostConfig.js b/packages/react-native-renderer/src/ReactFabricHostConfig.js index 22553f5aabc0b..69f0f57040c8f 100644 --- a/packages/react-native-renderer/src/ReactFabricHostConfig.js +++ b/packages/react-native-renderer/src/ReactFabricHostConfig.js @@ -39,8 +39,10 @@ import { appendChildToSet as appendChildNodeToSet, completeRoot, registerEventHandler, + measure as fabricMeasure, + measureInWindow as fabricMeasureInWindow, + measureLayout as fabricMeasureLayout, } from 'FabricUIManager'; -import UIManager from 'UIManager'; // Counter for uniquely identifying views. // % 10 === 1 means it is a rootTag. @@ -85,15 +87,18 @@ class ReactFabricHostComponent { _nativeTag: number; viewConfig: ReactNativeBaseComponentViewConfig<>; currentProps: Props; + _internalInstanceHandle: Object; constructor( tag: number, viewConfig: ReactNativeBaseComponentViewConfig<>, props: Props, + internalInstanceHandle: Object, ) { this._nativeTag = tag; this.viewConfig = viewConfig; this.currentProps = props; + this._internalInstanceHandle = internalInstanceHandle; } blur() { @@ -105,15 +110,15 @@ class ReactFabricHostComponent { } measure(callback: MeasureOnSuccessCallback) { - UIManager.measure( - this._nativeTag, + fabricMeasure( + this._internalInstanceHandle.stateNode.node, mountSafeCallback_NOT_REALLY_SAFE(this, callback), ); } measureInWindow(callback: MeasureInWindowOnSuccessCallback) { - UIManager.measureInWindow( - this._nativeTag, + fabricMeasureInWindow( + this._internalInstanceHandle.stateNode.node, mountSafeCallback_NOT_REALLY_SAFE(this, callback), ); } @@ -123,32 +128,21 @@ class ReactFabricHostComponent { onSuccess: MeasureLayoutOnSuccessCallback, onFail: () => void /* currently unused */, ) { - let relativeNode; - - if (typeof relativeToNativeNode === 'number') { - // Already a node handle - relativeNode = relativeToNativeNode; - } else if (relativeToNativeNode._nativeTag) { - relativeNode = relativeToNativeNode._nativeTag; - } else if ( - relativeToNativeNode.canonical && - relativeToNativeNode.canonical._nativeTag + if ( + typeof relativeToNativeNode === 'number' || + !(relativeToNativeNode instanceof ReactFabricHostComponent) ) { - relativeNode = relativeToNativeNode.canonical._nativeTag; - } - - if (relativeNode == null) { warningWithoutStack( false, - 'Warning: ref.measureLayout must be called with a node handle or a ref to a native component.', + 'Warning: ref.measureLayout must be called with a ref to a native component.', ); return; } - UIManager.measureLayout( - this._nativeTag, - relativeNode, + fabricMeasureLayout( + this._internalInstanceHandle.stateNode.node, + relativeToNativeNode._internalInstanceHandle.stateNode.node, mountSafeCallback_NOT_REALLY_SAFE(this, onFail), mountSafeCallback_NOT_REALLY_SAFE(this, onSuccess), ); @@ -212,7 +206,12 @@ export function createInstance( internalInstanceHandle, // internalInstanceHandle ); - const component = new ReactFabricHostComponent(tag, viewConfig, props); + const component = new ReactFabricHostComponent( + tag, + viewConfig, + props, + internalInstanceHandle, + ); return { node: node, diff --git a/packages/react-native-renderer/src/ReactNativeComponent.js b/packages/react-native-renderer/src/ReactNativeComponent.js index d16adf61831a3..df19c665403ef 100644 --- a/packages/react-native-renderer/src/ReactNativeComponent.js +++ b/packages/react-native-renderer/src/ReactNativeComponent.js @@ -19,6 +19,7 @@ import type { import React from 'react'; // Modules provided by RN: import TextInputState from 'TextInputState'; +import * as FabricUIManager from 'FabricUIManager'; import UIManager from 'UIManager'; import {create} from './ReactNativeAttributePayload'; @@ -83,10 +84,33 @@ export default function( * [`onLayout` prop](docs/view.html#onlayout) instead. */ measure(callback: MeasureOnSuccessCallback): void { - UIManager.measure( - findNodeHandle(this), - mountSafeCallback_NOT_REALLY_SAFE(this, callback), - ); + 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 = findHostInstance(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; + } + + if (maybeInstance.canonical) { + FabricUIManager.measure( + maybeInstance.node, + mountSafeCallback_NOT_REALLY_SAFE(this, callback), + ); + } else { + UIManager.measure( + findNodeHandle(this), + mountSafeCallback_NOT_REALLY_SAFE(this, callback), + ); + } } /** @@ -103,10 +127,33 @@ export default function( * These values are not available until after natives rendering completes. */ measureInWindow(callback: MeasureInWindowOnSuccessCallback): void { - UIManager.measureInWindow( - findNodeHandle(this), - mountSafeCallback_NOT_REALLY_SAFE(this, callback), - ); + 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 = findHostInstance(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; + } + + if (maybeInstance.canonical) { + FabricUIManager.measureInWindow( + maybeInstance.node, + mountSafeCallback_NOT_REALLY_SAFE(this, callback), + ); + } else { + UIManager.measureInWindow( + findNodeHandle(this), + mountSafeCallback_NOT_REALLY_SAFE(this, callback), + ); + } } /** @@ -116,16 +163,60 @@ export default function( * Obtain a native node handle with `ReactNative.findNodeHandle(component)`. */ measureLayout( - relativeToNativeNode: number, + relativeToNativeNode: number | Object, onSuccess: MeasureLayoutOnSuccessCallback, onFail: () => void /* currently unused */, ): void { - UIManager.measureLayout( - findNodeHandle(this), - relativeToNativeNode, - mountSafeCallback_NOT_REALLY_SAFE(this, onFail), - mountSafeCallback_NOT_REALLY_SAFE(this, onSuccess), - ); + 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 = findHostInstance(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; + } + + if (maybeInstance.canonical) { + warningWithoutStack( + false, + 'Warning: measureLayout on components using NativeMethodsMixin ' + + 'or ReactNative.NativeComponent is not currently supported in Fabric. ' + + 'measureLayout must be called on a native ref. Consider using forwardRef.', + ); + return; + } else { + let relativeNode; + + if (typeof relativeToNativeNode === 'number') { + // Already a node handle + relativeNode = relativeToNativeNode; + } else if (relativeToNativeNode._nativeTag) { + relativeNode = relativeToNativeNode._nativeTag; + } + + if (relativeNode == null) { + warningWithoutStack( + false, + 'Warning: ref.measureLayout must be called with a node handle or a ref to a native component.', + ); + + return; + } + + UIManager.measureLayout( + findNodeHandle(this), + relativeNode, + mountSafeCallback_NOT_REALLY_SAFE(this, onFail), + mountSafeCallback_NOT_REALLY_SAFE(this, onSuccess), + ); + } } /** diff --git a/packages/react-native-renderer/src/ReactNativeTypes.js b/packages/react-native-renderer/src/ReactNativeTypes.js index 1e145ed5e1d5c..3e777114db2ae 100644 --- a/packages/react-native-renderer/src/ReactNativeTypes.js +++ b/packages/react-native-renderer/src/ReactNativeTypes.js @@ -89,7 +89,7 @@ class ReactNativeComponent extends React.Component { measure(callback: MeasureOnSuccessCallback): void {} measureInWindow(callback: MeasureInWindowOnSuccessCallback): void {} measureLayout( - relativeToNativeNode: number, + relativeToNativeNode: number | Object, onSuccess: MeasureLayoutOnSuccessCallback, onFail?: () => void, ): void {} @@ -106,7 +106,7 @@ export type NativeMethodsMixinType = { measure(callback: MeasureOnSuccessCallback): void, measureInWindow(callback: MeasureInWindowOnSuccessCallback): void, measureLayout( - relativeToNativeNode: number, + relativeToNativeNode: number | Object, onSuccess: MeasureLayoutOnSuccessCallback, onFail: () => void, ): void, diff --git a/packages/react-native-renderer/src/__mocks__/FabricUIManager.js b/packages/react-native-renderer/src/__mocks__/FabricUIManager.js index a3f80b04493cc..c44031b194353 100644 --- a/packages/react-native-renderer/src/__mocks__/FabricUIManager.js +++ b/packages/react-native-renderer/src/__mocks__/FabricUIManager.js @@ -119,6 +119,57 @@ const RCTFabricUIManager = { }), registerEventHandler: jest.fn(function registerEventHandler(callback) {}), + + measure: jest.fn(function measure(node, callback) { + invariant( + typeof node === 'object', + 'Expected node to be an object, was passed "%s"', + typeof node, + ); + invariant( + typeof node.viewName === 'string', + 'Expected node to be a host node.', + ); + callback(10, 10, 100, 100, 0, 0); + }), + measureInWindow: jest.fn(function measureInWindow(node, callback) { + invariant( + typeof node === 'object', + 'Expected node to be an object, was passed "%s"', + typeof node, + ); + invariant( + typeof node.viewName === 'string', + 'Expected node to be a host node.', + ); + callback(10, 10, 100, 100); + }), + measureLayout: jest.fn(function measureLayout( + node, + relativeNode, + fail, + success, + ) { + invariant( + typeof node === 'object', + 'Expected node to be an object, was passed "%s"', + typeof node, + ); + invariant( + typeof node.viewName === 'string', + 'Expected node to be a host node.', + ); + invariant( + typeof relativeNode === 'object', + 'Expected relative node to be an object, was passed "%s"', + typeof relativeNode, + ); + invariant( + typeof relativeNode.viewName === 'string', + 'Expected relative node to be a host node.', + ); + success(1, 1, 100, 100); + }), }; module.exports = RCTFabricUIManager; diff --git a/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js b/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js index 9ec320ad74e6b..5681aa7661795 100644 --- a/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js @@ -302,6 +302,88 @@ describe('ReactFabric', () => { }); }); + it('should call FabricUIManager.measure on ref.measure', () => { + const View = createReactNativeComponentClass('RCTView', () => ({ + validAttributes: {foo: true}, + uiViewClassName: 'RCTView', + })); + + class Subclass extends ReactFabric.NativeComponent { + render() { + return {this.props.children}; + } + } + + const CreateClass = createReactClass({ + mixins: [NativeMethodsMixin], + render() { + return {this.props.children}; + }, + }); + + [View, Subclass, CreateClass].forEach(Component => { + FabricUIManager.measure.mockClear(); + + let viewRef; + ReactFabric.render( + { + viewRef = ref; + }} + />, + 11, + ); + + expect(FabricUIManager.measure).not.toBeCalled(); + const successCallback = jest.fn(); + viewRef.measure(successCallback); + expect(FabricUIManager.measure).toHaveBeenCalledTimes(1); + expect(successCallback).toHaveBeenCalledTimes(1); + expect(successCallback).toHaveBeenCalledWith(10, 10, 100, 100, 0, 0); + }); + }); + + it('should call FabricUIManager.measureInWindow on ref.measureInWindow', () => { + const View = createReactNativeComponentClass('RCTView', () => ({ + validAttributes: {foo: true}, + uiViewClassName: 'RCTView', + })); + + class Subclass extends ReactFabric.NativeComponent { + render() { + return {this.props.children}; + } + } + + const CreateClass = createReactClass({ + mixins: [NativeMethodsMixin], + render() { + return {this.props.children}; + }, + }); + + [View, Subclass, CreateClass].forEach(Component => { + FabricUIManager.measureInWindow.mockClear(); + + let viewRef; + ReactFabric.render( + { + viewRef = ref; + }} + />, + 11, + ); + + expect(FabricUIManager.measureInWindow).not.toBeCalled(); + const successCallback = jest.fn(); + viewRef.measureInWindow(successCallback); + expect(FabricUIManager.measureInWindow).toHaveBeenCalledTimes(1); + expect(successCallback).toHaveBeenCalledTimes(1); + expect(successCallback).toHaveBeenCalledWith(10, 10, 100, 100); + }); + }); + it('should support ref in ref.measureLayout', () => { const View = createReactNativeComponentClass('RCTView', () => ({ validAttributes: {foo: true}, @@ -309,7 +391,7 @@ describe('ReactFabric', () => { })); [View].forEach(Component => { - UIManager.measureLayout.mockReset(); + FabricUIManager.measureLayout.mockClear(); let viewRef; let otherRef; @@ -330,30 +412,75 @@ describe('ReactFabric', () => { 11, ); - expect(UIManager.measureLayout).not.toBeCalled(); - + expect(FabricUIManager.measureLayout).not.toBeCalled(); const successCallback = jest.fn(); const failureCallback = jest.fn(); viewRef.measureLayout(otherRef, successCallback, failureCallback); + expect(FabricUIManager.measureLayout).toHaveBeenCalledTimes(1); + expect(successCallback).toHaveBeenCalledTimes(1); + expect(successCallback).toHaveBeenCalledWith(1, 1, 100, 100); + }); + }); + + it('should warn when calling measureLayout on Subclass and NativeMethodsMixin', () => { + const View = createReactNativeComponentClass('RCTView', () => ({ + validAttributes: {foo: true}, + uiViewClassName: 'RCTView', + })); + + class Subclass extends ReactFabric.NativeComponent { + render() { + return {this.props.children}; + } + } + + const CreateClass = createReactClass({ + mixins: [NativeMethodsMixin], + render() { + return {this.props.children}; + }, + }); - expect(UIManager.measureLayout).toHaveBeenCalledTimes(1); - expect(UIManager.measureLayout).toHaveBeenCalledWith( - expect.any(Number), - expect.any(Number), - expect.any(Function), - expect.any(Function), + [Subclass, CreateClass].forEach(Component => { + FabricUIManager.measureLayout.mockReset(); + + let viewRef; + let otherRef; + ReactFabric.render( + + { + viewRef = ref; + }} + /> + { + otherRef = ref; + }} + /> + , + 11, ); - const args = UIManager.measureLayout.mock.calls[0]; - expect(args[0]).not.toEqual(args[1]); - expect(successCallback).not.toBeCalled(); - expect(failureCallback).not.toBeCalled(); - args[2]('fail'); - expect(failureCallback).toBeCalledWith('fail'); + const successCallback = jest.fn(); + const failureCallback = jest.fn(); + + expect(() => { + viewRef.measureLayout(otherRef, successCallback, failureCallback); + }).toWarnDev( + [ + 'Warning: measureLayout on components using NativeMethodsMixin ' + + 'or ReactNative.NativeComponent is not currently supported in Fabric. ' + + 'measureLayout must be called on a native ref. Consider using forwardRef.', + ], + { + withoutStack: true, + }, + ); - expect(successCallback).not.toBeCalled(); - args[3]('success'); - expect(successCallback).toBeCalledWith('success'); + expect(FabricUIManager.measureLayout).not.toBeCalled(); + expect(UIManager.measureLayout).not.toBeCalled(); }); }); diff --git a/scripts/flow/react-native-host-hooks.js b/scripts/flow/react-native-host-hooks.js index 91a17955104ea..414775f48f4c5 100644 --- a/scripts/flow/react-native-host-hooks.js +++ b/scripts/flow/react-native-host-hooks.js @@ -10,6 +10,9 @@ /* eslint-disable */ import type { + MeasureOnSuccessCallback, + MeasureInWindowOnSuccessCallback, + MeasureLayoutOnSuccessCallback, ReactNativeBaseComponentViewConfig, ViewConfigGetter, } from 'react-native-renderer/src/ReactNativeTypes'; @@ -124,6 +127,21 @@ declare module 'FabricUIManager' { payload: Object, ) => void, ): void; + + declare function measure( + node: Node, + callback: MeasureOnSuccessCallback, + ): void; + declare function measureInWindow( + node: Node, + callback: MeasureInWindowOnSuccessCallback, + ): void; + declare function measureLayout( + node: Node, + relativeNode: Node, + onFail: () => void, + onSuccess: MeasureLayoutOnSuccessCallback, + ): void; } declare module 'View' { diff --git a/scripts/rollup/bundles.js b/scripts/rollup/bundles.js index e2e59d2c4092e..2d6babd5760fd 100644 --- a/scripts/rollup/bundles.js +++ b/scripts/rollup/bundles.js @@ -224,6 +224,7 @@ const bundles = [ 'RCTEventEmitter', 'TextInputState', 'UIManager', + 'FabricUIManager', 'deepDiffer', 'deepFreezeAndThrowOnMutationInDev', 'flattenStyle', @@ -243,6 +244,7 @@ const bundles = [ 'RCTEventEmitter', 'TextInputState', 'UIManager', + 'FabricUIManager', 'deepDiffer', 'deepFreezeAndThrowOnMutationInDev', 'flattenStyle',