From ef52e22e8906b3ec9c2c449a6a8220bc4eb4f88d Mon Sep 17 00:00:00 2001 From: Eli White Date: Wed, 20 Feb 2019 14:56:59 -0800 Subject: [PATCH 1/5] Adding ReactNative.setNativeProps that takes a ref --- .../react-native-renderer/src/ReactFabric.js | 3 + .../src/ReactNativeRenderer.js | 3 + .../src/ReactNativeSetNativeProps.js | 48 +++++++++++ .../__tests__/ReactFabric-test.internal.js | 86 ++++++++++++++++++- .../ReactNativeMount-test.internal.js | 84 ++++++++++++++++++ 5 files changed, 223 insertions(+), 1 deletion(-) create mode 100644 packages/react-native-renderer/src/ReactNativeSetNativeProps.js diff --git a/packages/react-native-renderer/src/ReactFabric.js b/packages/react-native-renderer/src/ReactFabric.js index 6f296545cba2a..8051fe368791c 100644 --- a/packages/react-native-renderer/src/ReactFabric.js +++ b/packages/react-native-renderer/src/ReactFabric.js @@ -32,6 +32,7 @@ import NativeMethodsMixin from './NativeMethodsMixin'; import ReactNativeComponent from './ReactNativeComponent'; import {getClosestInstanceFromNode} from './ReactFabricComponentTree'; import {getInspectorDataForViewTag} from './ReactNativeFiberInspector'; +import {setNativeProps} from './ReactNativeSetNativeProps'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import getComponentName from 'shared/getComponentName'; @@ -104,6 +105,8 @@ const ReactFabric: ReactFabricType = { findNodeHandle, + setNativeProps, + render(element: React$Element, containerTag: any, callback: ?Function) { let root = roots.get(containerTag); diff --git a/packages/react-native-renderer/src/ReactNativeRenderer.js b/packages/react-native-renderer/src/ReactNativeRenderer.js index e26d6c5ee8ecf..8113b7ce969a9 100644 --- a/packages/react-native-renderer/src/ReactNativeRenderer.js +++ b/packages/react-native-renderer/src/ReactNativeRenderer.js @@ -38,6 +38,7 @@ import NativeMethodsMixin from './NativeMethodsMixin'; import ReactNativeComponent from './ReactNativeComponent'; import {getClosestInstanceFromNode} from './ReactNativeComponentTree'; import {getInspectorDataForViewTag} from './ReactNativeFiberInspector'; +import {setNativeProps} from './ReactNativeSetNativeProps'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import getComponentName from 'shared/getComponentName'; @@ -116,6 +117,8 @@ const ReactNativeRenderer: ReactNativeType = { findNodeHandle, + setNativeProps, + render(element: React$Element, containerTag: any, callback: ?Function) { let root = roots.get(containerTag); diff --git a/packages/react-native-renderer/src/ReactNativeSetNativeProps.js b/packages/react-native-renderer/src/ReactNativeSetNativeProps.js new file mode 100644 index 0000000000000..66db01a52a31f --- /dev/null +++ b/packages/react-native-renderer/src/ReactNativeSetNativeProps.js @@ -0,0 +1,48 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import {create} from './ReactNativeAttributePayload'; +import {warnForStyleProps} from './NativeMethodsMixinUtils'; + +import ReactSharedInternals from 'shared/ReactSharedInternals'; +import getComponentName from 'shared/getComponentName'; +import warningWithoutStack from 'shared/warningWithoutStack'; + +// Module provided by RN: +import UIManager from 'UIManager'; + +const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner; + +export function setNativeProps(handle, nativeProps: Object) { + if (handle._nativeTag == null) { + warningWithoutStack( + handle._nativeTag != null, + "setNativeProps was called on a ref that isn't a " + + 'native component. Use React.forwardRef to get access to the underlying native component', + ); + return; + } + + if (__DEV__) { + warnForStyleProps(nativeProps, handle.viewConfig.validAttributes); + } + + const updatePayload = create(nativeProps, handle.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( + handle._nativeTag, + handle.viewConfig.uiViewClassName, + updatePayload, + ); + } +} 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 a045729e972d3..90ad6c3e80b4b 100644 --- a/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js @@ -167,7 +167,7 @@ describe('ReactFabric', () => { expect(FabricUIManager.__dumpHierarchyForJestTestsOnly()).toMatchSnapshot(); }); - it('should not call UIManager.updateView from setNativeProps for properties that have not changed', () => { + it('should not call UIManager.updateView from ref.setNativeProps for properties that have not changed', () => { const View = createReactNativeComponentClass('RCTView', () => ({ validAttributes: {foo: true}, uiViewClassName: 'RCTView', @@ -214,6 +214,90 @@ describe('ReactFabric', () => { }); }); + it('should be able to setNativeProps on native refs', () => { + const View = createReactNativeComponentClass('RCTView', () => ({ + validAttributes: {foo: true}, + uiViewClassName: 'RCTView', + })); + + UIManager.updateView.mockReset(); + + let viewRef; + ReactFabric.render( + { + viewRef = ref; + }} + />, + 11, + ); + + expect(UIManager.updateView).not.toBeCalled(); + ReactFabric.setNativeProps(viewRef, {foo: 'baz'}); + expect(UIManager.updateView).toHaveBeenCalledTimes(1); + expect(UIManager.updateView).toHaveBeenCalledWith( + expect.any(Number), + 'RCTView', + {foo: 'baz'}, + ); + }); + + it('should warn and no-op if calling setNativeProps on non native refs', () => { + const View = createReactNativeComponentClass('RCTView', () => ({ + validAttributes: {foo: true}, + uiViewClassName: 'RCTView', + })); + + class BasicClass extends React.Component { + render() { + return ; + } + } + + class Subclass extends ReactFabric.NativeComponent { + render() { + return ; + } + } + + const CreateClass = createReactClass({ + mixins: [NativeMethodsMixin], + render: () => { + return ; + }, + }); + + [BasicClass, Subclass, CreateClass].forEach(Component => { + UIManager.updateView.mockReset(); + + let viewRef; + ReactFabric.render( + { + viewRef = ref; + }} + />, + 11, + ); + + expect(UIManager.updateView).not.toBeCalled(); + expect(() => { + ReactFabric.setNativeProps(viewRef, {foo: 'baz'}); + }).toWarnDev( + [ + "Warning: setNativeProps was called on a ref that isn't a " + + 'native component. Use React.forwardRef to get access ' + + 'to the underlying native component', + ], + {withoutStack: true}, + ); + + expect(UIManager.updateView).not.toBeCalled(); + }); + }); + it('returns the correct instance and calls it in the callback', () => { const View = createReactNativeComponentClass('RCTView', () => ({ validAttributes: {foo: true}, diff --git a/packages/react-native-renderer/src/__tests__/ReactNativeMount-test.internal.js b/packages/react-native-renderer/src/__tests__/ReactNativeMount-test.internal.js index 8089088f8faea..225c941b3981f 100644 --- a/packages/react-native-renderer/src/__tests__/ReactNativeMount-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ReactNativeMount-test.internal.js @@ -145,6 +145,90 @@ describe('ReactNative', () => { }); }); + it('should be able to setNativeProps on native refs', () => { + const View = createReactNativeComponentClass('RCTView', () => ({ + validAttributes: {foo: true}, + uiViewClassName: 'RCTView', + })); + + UIManager.updateView.mockReset(); + + let viewRef; + ReactNative.render( + { + viewRef = ref; + }} + />, + 11, + ); + + expect(UIManager.updateView).not.toBeCalled(); + ReactNative.setNativeProps(viewRef, {foo: 'baz'}); + expect(UIManager.updateView).toHaveBeenCalledTimes(1); + expect(UIManager.updateView).toHaveBeenCalledWith( + expect.any(Number), + 'RCTView', + {foo: 'baz'}, + ); + }); + + it('should warn and no-op if calling setNativeProps on non native refs', () => { + const View = createReactNativeComponentClass('RCTView', () => ({ + validAttributes: {foo: true}, + uiViewClassName: 'RCTView', + })); + + class BasicClass extends React.Component { + render() { + return ; + } + } + + class Subclass extends ReactNative.NativeComponent { + render() { + return ; + } + } + + const CreateClass = createReactClass({ + mixins: [NativeMethodsMixin], + render: () => { + return ; + }, + }); + + [BasicClass, Subclass, CreateClass].forEach(Component => { + UIManager.updateView.mockReset(); + + let viewRef; + ReactNative.render( + { + viewRef = ref; + }} + />, + 11, + ); + + expect(UIManager.updateView).not.toBeCalled(); + expect(() => { + ReactNative.setNativeProps(viewRef, {foo: 'baz'}); + }).toWarnDev( + [ + "Warning: setNativeProps was called on a ref that isn't a " + + 'native component. Use React.forwardRef to get access ' + + 'to the underlying native component', + ], + {withoutStack: true}, + ); + + expect(UIManager.updateView).not.toBeCalled(); + }); + }); + it('returns the correct instance and calls it in the callback', () => { const View = createReactNativeComponentClass('RCTView', () => ({ validAttributes: {foo: true}, From 411ca62370b98bcda97a88be46ef04c2ed7593ee Mon Sep 17 00:00:00 2001 From: Eli White Date: Wed, 20 Feb 2019 15:55:00 -0800 Subject: [PATCH 2/5] Adding test for components rendered with Fabric with Paper's setNativeProps --- .../src/ReactNativeSetNativeProps.js | 7 +----- .../ReactFabricAndNative-test.internal.js | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/packages/react-native-renderer/src/ReactNativeSetNativeProps.js b/packages/react-native-renderer/src/ReactNativeSetNativeProps.js index 66db01a52a31f..0d512a1ea5a3b 100644 --- a/packages/react-native-renderer/src/ReactNativeSetNativeProps.js +++ b/packages/react-native-renderer/src/ReactNativeSetNativeProps.js @@ -10,16 +10,12 @@ import {create} from './ReactNativeAttributePayload'; import {warnForStyleProps} from './NativeMethodsMixinUtils'; -import ReactSharedInternals from 'shared/ReactSharedInternals'; -import getComponentName from 'shared/getComponentName'; import warningWithoutStack from 'shared/warningWithoutStack'; // Module provided by RN: import UIManager from 'UIManager'; -const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner; - -export function setNativeProps(handle, nativeProps: Object) { +export function setNativeProps(handle: any, nativeProps: Object) { if (handle._nativeTag == null) { warningWithoutStack( handle._nativeTag != null, @@ -34,7 +30,6 @@ export function setNativeProps(handle, nativeProps: Object) { } const updatePayload = create(nativeProps, handle.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. diff --git a/packages/react-native-renderer/src/__tests__/ReactFabricAndNative-test.internal.js b/packages/react-native-renderer/src/__tests__/ReactFabricAndNative-test.internal.js index 6381235257e2a..572fc84a6cedf 100644 --- a/packages/react-native-renderer/src/__tests__/ReactFabricAndNative-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ReactFabricAndNative-test.internal.js @@ -13,12 +13,14 @@ let React; let ReactFabric; let ReactNative; +let UIManager; let createReactNativeComponentClass; describe('ReactFabric', () => { beforeEach(() => { jest.resetModules(); ReactNative = require('react-native-renderer'); + UIManager = require('UIManager'); jest.resetModules(); jest.mock('shared/ReactFeatureFlags', () => require('shared/forks/ReactFeatureFlags.native-oss'), @@ -49,4 +51,25 @@ describe('ReactFabric', () => { let handle = ReactNative.findNodeHandle(ref.current); expect(handle).toBe(2); }); + + it('sets native props with setNativeProps on Fabric nodes with the RN renderer', () => { + UIManager.updateView.mockReset(); + const View = createReactNativeComponentClass('RCTView', () => ({ + validAttributes: {title: true}, + uiViewClassName: 'RCTView', + })); + + let ref = React.createRef(); + + ReactFabric.render(, 11); + expect(UIManager.updateView).not.toBeCalled(); + ReactNative.setNativeProps(ref.current, {title: 'baz'}); + expect(UIManager.updateView).toHaveBeenCalledTimes(1); + expect(UIManager.updateView).toHaveBeenCalledWith( + expect.any(Number), + 'RCTView', + {title: 'baz'}, + ); + }); + }); From 676a5bad6f58517b083a7e83edc00d685eb4352d Mon Sep 17 00:00:00 2001 From: Eli White Date: Wed, 20 Feb 2019 16:01:47 -0800 Subject: [PATCH 3/5] Fixing flow types --- packages/react-native-renderer/src/ReactNativeSetNativeProps.js | 2 +- packages/react-native-renderer/src/ReactNativeTypes.js | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/react-native-renderer/src/ReactNativeSetNativeProps.js b/packages/react-native-renderer/src/ReactNativeSetNativeProps.js index 0d512a1ea5a3b..ddfd296fd2082 100644 --- a/packages/react-native-renderer/src/ReactNativeSetNativeProps.js +++ b/packages/react-native-renderer/src/ReactNativeSetNativeProps.js @@ -15,7 +15,7 @@ import warningWithoutStack from 'shared/warningWithoutStack'; // Module provided by RN: import UIManager from 'UIManager'; -export function setNativeProps(handle: any, nativeProps: Object) { +export function setNativeProps(handle: any, nativeProps: Object): void { if (handle._nativeTag == null) { warningWithoutStack( handle._nativeTag != null, diff --git a/packages/react-native-renderer/src/ReactNativeTypes.js b/packages/react-native-renderer/src/ReactNativeTypes.js index f9058abc1e939..1e145ed5e1d5c 100644 --- a/packages/react-native-renderer/src/ReactNativeTypes.js +++ b/packages/react-native-renderer/src/ReactNativeTypes.js @@ -131,6 +131,7 @@ type SecretInternalsFabricType = { export type ReactNativeType = { NativeComponent: typeof ReactNativeComponent, findNodeHandle(componentOrHandle: any): ?number, + setNativeProps(handle: any, nativeProps: Object): void, render( element: React$Element, containerTag: any, @@ -146,6 +147,7 @@ export type ReactNativeType = { export type ReactFabricType = { NativeComponent: typeof ReactNativeComponent, findNodeHandle(componentOrHandle: any): ?number, + setNativeProps(handle: any, nativeProps: Object): void, render( element: React$Element, containerTag: any, From 6a46015d74d187d1f60760fefb57cf71e60c3ab6 Mon Sep 17 00:00:00 2001 From: Eli White Date: Wed, 20 Feb 2019 22:55:30 -0800 Subject: [PATCH 4/5] Fix prettier --- .../src/__tests__/ReactFabricAndNative-test.internal.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/react-native-renderer/src/__tests__/ReactFabricAndNative-test.internal.js b/packages/react-native-renderer/src/__tests__/ReactFabricAndNative-test.internal.js index 572fc84a6cedf..1fff126bb1bba 100644 --- a/packages/react-native-renderer/src/__tests__/ReactFabricAndNative-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ReactFabricAndNative-test.internal.js @@ -71,5 +71,4 @@ describe('ReactFabric', () => { {title: 'baz'}, ); }); - }); From ada38e2027d8338e8db42cd6436525f4499cef1c Mon Sep 17 00:00:00 2001 From: Eli White Date: Wed, 20 Feb 2019 23:02:06 -0800 Subject: [PATCH 5/5] Rename ReactNativeSetNativeProps.js to be more general --- packages/react-native-renderer/src/ReactFabric.js | 2 +- packages/react-native-renderer/src/ReactNativeRenderer.js | 2 +- ...iveSetNativeProps.js => ReactNativeRendererSharedExports.js} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename packages/react-native-renderer/src/{ReactNativeSetNativeProps.js => ReactNativeRendererSharedExports.js} (100%) diff --git a/packages/react-native-renderer/src/ReactFabric.js b/packages/react-native-renderer/src/ReactFabric.js index 8051fe368791c..b6fe0c65d7be0 100644 --- a/packages/react-native-renderer/src/ReactFabric.js +++ b/packages/react-native-renderer/src/ReactFabric.js @@ -32,7 +32,7 @@ import NativeMethodsMixin from './NativeMethodsMixin'; import ReactNativeComponent from './ReactNativeComponent'; import {getClosestInstanceFromNode} from './ReactFabricComponentTree'; import {getInspectorDataForViewTag} from './ReactNativeFiberInspector'; -import {setNativeProps} from './ReactNativeSetNativeProps'; +import {setNativeProps} from './ReactNativeRendererSharedExports'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import getComponentName from 'shared/getComponentName'; diff --git a/packages/react-native-renderer/src/ReactNativeRenderer.js b/packages/react-native-renderer/src/ReactNativeRenderer.js index 8113b7ce969a9..6cfe575dcb3a9 100644 --- a/packages/react-native-renderer/src/ReactNativeRenderer.js +++ b/packages/react-native-renderer/src/ReactNativeRenderer.js @@ -38,7 +38,7 @@ import NativeMethodsMixin from './NativeMethodsMixin'; import ReactNativeComponent from './ReactNativeComponent'; import {getClosestInstanceFromNode} from './ReactNativeComponentTree'; import {getInspectorDataForViewTag} from './ReactNativeFiberInspector'; -import {setNativeProps} from './ReactNativeSetNativeProps'; +import {setNativeProps} from './ReactNativeRendererSharedExports'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import getComponentName from 'shared/getComponentName'; diff --git a/packages/react-native-renderer/src/ReactNativeSetNativeProps.js b/packages/react-native-renderer/src/ReactNativeRendererSharedExports.js similarity index 100% rename from packages/react-native-renderer/src/ReactNativeSetNativeProps.js rename to packages/react-native-renderer/src/ReactNativeRendererSharedExports.js