Make setNativeProps a no-op with Fabric renderer (#15094)

elicwhite · bvaughn · commit 1b94fd215dfd · 2019-03-29T15:44:15.000-07:00
* Make setNativeProps a no-op with Fabric renderer

* Remove unnecessary __DEV__ check
diff --git 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
@@ -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,14 @@ const ReactFabric: ReactFabricType = {
 
   findNodeHandle,
 
-  setNativeProps,
+  setNativeProps(handle: any, nativeProps: Object) {
+    warningWithoutStack(
+      false,
+      'Warning: setNativeProps is not currently supported in Fabric',
+    );
+
+    return;
+  },
 
   render(element: React$Element<any>, 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
@@ -16,17 +16,13 @@ import type {
 } from './ReactNativeTypes';
 import type {ReactEventResponder} from 'shared/ReactTypes';
 
-import {
-  mountSafeCallback_NOT_REALLY_SAFE,
-  warnForStyleProps,
-} from './NativeMethodsMixinUtils';
+import {mountSafeCallback_NOT_REALLY_SAFE} from './NativeMethodsMixinUtils';
 import {create, diff} from './ReactNativeAttributePayload';
 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,31 +132,12 @@ 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);
-    }
+    warningWithoutStack(
+      false,
+      'Warning: setNativeProps is not currently supported in Fabric',
+    );
 
-    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,
-      );
-    }
+    return;
   }
 }
 
diff --git 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
@@ -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,27 +209,22 @@ describe('ReactFabric', () => {
 
       expect(() => {
         viewRef.setNativeProps({});
-      }).toWarnDev([SET_NATIVE_PROPS_DEPRECATION_MESSAGE], {
+      }).toWarnDev([SET_NATIVE_PROPS_NOT_SUPPORTED_MESSAGE], {
         withoutStack: true,
       });
 
       expect(UIManager.updateView).not.toBeCalled();
 
       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();
     });
