From 54f803b546afd5fd5863fc4275ea6c05be2307c0 Mon Sep 17 00:00:00 2001 From: Eli White Date: Tue, 12 Mar 2019 13:17:55 -0700 Subject: [PATCH 1/2] Make setNativeProps a no-op with Fabric renderer --- .../src/NativeMethodsMixin.js | 31 ++++++++----- .../react-native-renderer/src/ReactFabric.js | 12 ++++- .../src/ReactFabricHostConfig.js | 29 ++---------- .../src/ReactNativeComponent.js | 32 ++++++++----- .../__tests__/ReactFabric-test.internal.js | 46 +++++++------------ 5 files changed, 71 insertions(+), 79 deletions(-) diff --git a/packages/react-native-renderer/src/NativeMethodsMixin.js b/packages/react-native-renderer/src/NativeMethodsMixin.js index 2755c21ea29..5df51392a99 100644 --- a/packages/react-native-renderer/src/NativeMethodsMixin.js +++ b/packages/react-native-renderer/src/NativeMethodsMixin.js @@ -124,17 +124,6 @@ export default function( * Manipulation](docs/direct-manipulation.html)). */ setNativeProps: function(nativeProps: Object) { - if (__DEV__) { - if (warnAboutDeprecatedSetNativeProps) { - warningWithoutStack( - false, - 'Warning: Calling ref.setNativeProps(nativeProps) ' + - 'is deprecated and will be removed in a future release. ' + - 'Use the setNativeProps export from the react-native package instead.' + - "\n\timport {setNativeProps} from 'react-native';\n\tsetNativeProps(ref, nativeProps);\n", - ); - } - } // 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. @@ -156,6 +145,26 @@ export default function( return; } + if (maybeInstance.canonical) { + warningWithoutStack( + false, + 'Warning: setNativeProps is not currently supported in Fabric', + ); + return; + } + + if (__DEV__) { + if (warnAboutDeprecatedSetNativeProps) { + warningWithoutStack( + false, + 'Warning: Calling ref.setNativeProps(nativeProps) ' + + 'is deprecated and will be removed in a future release. ' + + 'Use the setNativeProps export from the react-native package instead.' + + "\n\timport {setNativeProps} from 'react-native';\n\tsetNativeProps(ref, nativeProps);\n", + ); + } + } + const nativeTag = maybeInstance._nativeTag || maybeInstance.canonical._nativeTag; const viewConfig: ReactNativeBaseComponentViewConfig<> = diff --git a/packages/react-native-renderer/src/ReactFabric.js b/packages/react-native-renderer/src/ReactFabric.js index b6fe0c65d7b..81a0765b617 100644 --- a/packages/react-native-renderer/src/ReactFabric.js +++ b/packages/react-native-renderer/src/ReactFabric.js @@ -32,7 +32,6 @@ import NativeMethodsMixin from './NativeMethodsMixin'; import ReactNativeComponent from './ReactNativeComponent'; import {getClosestInstanceFromNode} from './ReactFabricComponentTree'; import {getInspectorDataForViewTag} from './ReactNativeFiberInspector'; -import {setNativeProps} from './ReactNativeRendererSharedExports'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import getComponentName from 'shared/getComponentName'; @@ -105,7 +104,16 @@ const ReactFabric: ReactFabricType = { findNodeHandle, - setNativeProps, + setNativeProps(handle: any, nativeProps: Object) { + if (__DEV__) { + warningWithoutStack( + false, + 'Warning: setNativeProps is not currently supported in Fabric', + ); + } + + return; + }, render(element: React$Element, containerTag: any, callback: ?Function) { let root = roots.get(containerTag); diff --git a/packages/react-native-renderer/src/ReactFabricHostConfig.js b/packages/react-native-renderer/src/ReactFabricHostConfig.js index c1268eb663e..577aa4ef265 100644 --- a/packages/react-native-renderer/src/ReactFabricHostConfig.js +++ b/packages/react-native-renderer/src/ReactFabricHostConfig.js @@ -17,7 +17,6 @@ import type { import { mountSafeCallback_NOT_REALLY_SAFE, - warnForStyleProps, } from './NativeMethodsMixinUtils'; import {create, diff} from './ReactNativeAttributePayload'; import {get as getViewConfigForType} from 'ReactNativeViewConfigRegistry'; @@ -25,7 +24,6 @@ import {get as getViewConfigForType} from 'ReactNativeViewConfigRegistry'; import deepFreezeAndThrowOnMutationInDev from 'deepFreezeAndThrowOnMutationInDev'; import invariant from 'shared/invariant'; import warningWithoutStack from 'shared/warningWithoutStack'; -import {warnAboutDeprecatedSetNativeProps} from 'shared/ReactFeatureFlags'; import {dispatchEvent} from './ReactFabricEventEmitter'; @@ -136,30 +134,13 @@ class ReactFabricHostComponent { setNativeProps(nativeProps: Object) { if (__DEV__) { - if (warnAboutDeprecatedSetNativeProps) { - warningWithoutStack( - false, - 'Warning: Calling ref.setNativeProps(nativeProps) ' + - 'is deprecated and will be removed in a future release. ' + - 'Use the setNativeProps export from the react-native package instead.' + - "\n\timport {setNativeProps} from 'react-native';\n\tsetNativeProps(ref, nativeProps);\n", - ); - } - warnForStyleProps(nativeProps, this.viewConfig.validAttributes); - } - - const updatePayload = create(nativeProps, this.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( - this._nativeTag, - this.viewConfig.uiViewClassName, - updatePayload, + warningWithoutStack( + false, + 'Warning: setNativeProps is not currently supported in Fabric', ); } + + return; } } diff --git a/packages/react-native-renderer/src/ReactNativeComponent.js b/packages/react-native-renderer/src/ReactNativeComponent.js index efd2c519380..d16adf61831 100644 --- a/packages/react-native-renderer/src/ReactNativeComponent.js +++ b/packages/react-native-renderer/src/ReactNativeComponent.js @@ -135,18 +135,6 @@ export default function( * Manipulation](docs/direct-manipulation.html)). */ setNativeProps(nativeProps: Object): void { - if (__DEV__) { - if (warnAboutDeprecatedSetNativeProps) { - warningWithoutStack( - false, - 'Warning: Calling ref.setNativeProps(nativeProps) ' + - 'is deprecated and will be removed in a future release. ' + - 'Use the setNativeProps export from the react-native package instead.' + - "\n\timport {setNativeProps} from 'react-native';\n\tsetNativeProps(ref, nativeProps);\n", - ); - } - } - // 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. @@ -168,6 +156,26 @@ export default function( return; } + if (maybeInstance.canonical) { + warningWithoutStack( + false, + 'Warning: setNativeProps is not currently supported in Fabric', + ); + return; + } + + if (__DEV__) { + if (warnAboutDeprecatedSetNativeProps) { + warningWithoutStack( + false, + 'Warning: Calling ref.setNativeProps(nativeProps) ' + + 'is deprecated and will be removed in a future release. ' + + 'Use the setNativeProps export from the react-native package instead.' + + "\n\timport {setNativeProps} from 'react-native';\n\tsetNativeProps(ref, nativeProps);\n", + ); + } + } + const nativeTag = maybeInstance._nativeTag || maybeInstance.canonical._nativeTag; const viewConfig: ReactNativeBaseComponentViewConfig<> = 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 58c9eba11b3..4244b7c5198 100644 --- a/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js @@ -20,11 +20,8 @@ let FabricUIManager; let StrictMode; let NativeMethodsMixin; -const SET_NATIVE_PROPS_DEPRECATION_MESSAGE = - 'Warning: Calling ref.setNativeProps(nativeProps) ' + - 'is deprecated and will be removed in a future release. ' + - 'Use the setNativeProps export from the react-native package instead.' + - "\n\timport {setNativeProps} from 'react-native';\n\tsetNativeProps(ref, nativeProps);\n"; +const SET_NATIVE_PROPS_NOT_SUPPORTED_MESSAGE = + 'Warning: setNativeProps is not currently supported in Fabric'; jest.mock('shared/ReactFeatureFlags', () => require('shared/forks/ReactFeatureFlags.native-oss'), @@ -176,7 +173,7 @@ describe('ReactFabric', () => { expect(FabricUIManager.__dumpHierarchyForJestTestsOnly()).toMatchSnapshot(); }); - it('should not call UIManager.updateView from ref.setNativeProps for properties that have not changed', () => { + it('should not call UIManager.updateView from ref.setNativeProps', () => { const View = createReactNativeComponentClass('RCTView', () => ({ validAttributes: {foo: true}, uiViewClassName: 'RCTView', @@ -212,7 +209,7 @@ describe('ReactFabric', () => { expect(() => { viewRef.setNativeProps({}); - }).toWarnDev([SET_NATIVE_PROPS_DEPRECATION_MESSAGE], { + }).toWarnDev([SET_NATIVE_PROPS_NOT_SUPPORTED_MESSAGE], { withoutStack: true, }); @@ -220,19 +217,14 @@ describe('ReactFabric', () => { expect(() => { viewRef.setNativeProps({foo: 'baz'}); - }).toWarnDev([SET_NATIVE_PROPS_DEPRECATION_MESSAGE], { + }).toWarnDev([SET_NATIVE_PROPS_NOT_SUPPORTED_MESSAGE], { withoutStack: true, }); - expect(UIManager.updateView).toHaveBeenCalledTimes(1); - expect(UIManager.updateView).toHaveBeenCalledWith( - expect.any(Number), - 'RCTView', - {foo: 'baz'}, - ); + expect(UIManager.updateView).not.toBeCalled(); }); }); - it('should be able to setNativeProps on native refs', () => { + it('setNativeProps on native refs should no-op', () => { const View = createReactNativeComponentClass('RCTView', () => ({ validAttributes: {foo: true}, uiViewClassName: 'RCTView', @@ -252,13 +244,12 @@ describe('ReactFabric', () => { ); 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'}, - ); + expect(() => { + ReactFabric.setNativeProps(viewRef, {foo: 'baz'}); + }).toWarnDev([SET_NATIVE_PROPS_NOT_SUPPORTED_MESSAGE], { + withoutStack: true, + }); + expect(UIManager.updateView).not.toBeCalled(); }); it('should warn and no-op if calling setNativeProps on non native refs', () => { @@ -303,14 +294,9 @@ describe('ReactFabric', () => { expect(UIManager.updateView).not.toBeCalled(); expect(() => { ReactFabric.setNativeProps(viewRef, {foo: 'baz'}); - }).toWarnDev( - [ - "Warning: setNativeProps was called with a ref that isn't a " + - 'native component. Use React.forwardRef to get access ' + - 'to the underlying native component', - ], - {withoutStack: true}, - ); + }).toWarnDev([SET_NATIVE_PROPS_NOT_SUPPORTED_MESSAGE], { + withoutStack: true, + }); expect(UIManager.updateView).not.toBeCalled(); }); From 4b4694ad0ff59dd387e6d1f4fb058bc587577459 Mon Sep 17 00:00:00 2001 From: Eli White Date: Tue, 12 Mar 2019 13:21:35 -0700 Subject: [PATCH 2/2] Remove unnecessary __DEV__ check --- packages/react-native-renderer/src/ReactFabric.js | 10 ++++------ .../src/ReactFabricHostConfig.js | 14 +++++--------- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/packages/react-native-renderer/src/ReactFabric.js b/packages/react-native-renderer/src/ReactFabric.js index 81a0765b617..a22c78aa579 100644 --- a/packages/react-native-renderer/src/ReactFabric.js +++ b/packages/react-native-renderer/src/ReactFabric.js @@ -105,12 +105,10 @@ const ReactFabric: ReactFabricType = { findNodeHandle, setNativeProps(handle: any, nativeProps: Object) { - if (__DEV__) { - warningWithoutStack( - false, - 'Warning: setNativeProps is not currently supported in Fabric', - ); - } + warningWithoutStack( + false, + 'Warning: setNativeProps is not currently supported in Fabric', + ); return; }, diff --git a/packages/react-native-renderer/src/ReactFabricHostConfig.js b/packages/react-native-renderer/src/ReactFabricHostConfig.js index 577aa4ef265..e2aea8daa21 100644 --- a/packages/react-native-renderer/src/ReactFabricHostConfig.js +++ b/packages/react-native-renderer/src/ReactFabricHostConfig.js @@ -15,9 +15,7 @@ import type { ReactNativeBaseComponentViewConfig, } from './ReactNativeTypes'; -import { - mountSafeCallback_NOT_REALLY_SAFE, -} from './NativeMethodsMixinUtils'; +import {mountSafeCallback_NOT_REALLY_SAFE} from './NativeMethodsMixinUtils'; import {create, diff} from './ReactNativeAttributePayload'; import {get as getViewConfigForType} from 'ReactNativeViewConfigRegistry'; @@ -133,12 +131,10 @@ class ReactFabricHostComponent { } setNativeProps(nativeProps: Object) { - if (__DEV__) { - warningWithoutStack( - false, - 'Warning: setNativeProps is not currently supported in Fabric', - ); - } + warningWithoutStack( + false, + 'Warning: setNativeProps is not currently supported in Fabric', + ); return; }