From 870214f37ad63333e750f31ef0cc0bde5793aee5 Mon Sep 17 00:00:00 2001 From: Eli White Date: Mon, 25 Feb 2019 15:00:39 -0800 Subject: [PATCH] Deprecate ref.setNativeProps in favor of ReactNative.setNativeProps (#14912) * Deprecate ref.setNativeProps in favor of ReactNative.setNativeProps * Using a feature flag for the setNativeProps warning * Removing extra line breaks * Set the FB native feature flag to true * Prettier --- .../src/NativeMethodsMixin.js | 14 ++++++++++ .../src/ReactFabricHostConfig.js | 11 ++++++++ .../src/ReactNativeComponent.js | 15 +++++++++++ .../src/ReactNativeFiberHostComponent.js | 12 +++++++++ .../__tests__/ReactFabric-test.internal.js | 22 ++++++++++++++-- .../ReactNativeMount-test.internal.js | 26 ++++++++++++++++--- packages/shared/ReactFeatureFlags.js | 4 +++ .../forks/ReactFeatureFlags.native-fb.js | 1 + .../forks/ReactFeatureFlags.native-oss.js | 1 + .../forks/ReactFeatureFlags.persistent.js | 1 + .../forks/ReactFeatureFlags.test-renderer.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 1 + .../shared/forks/ReactFeatureFlags.www.js | 1 + 13 files changed, 105 insertions(+), 5 deletions(-) diff --git a/packages/react-native-renderer/src/NativeMethodsMixin.js b/packages/react-native-renderer/src/NativeMethodsMixin.js index 62791c7e62441..2755c21ea2932 100644 --- a/packages/react-native-renderer/src/NativeMethodsMixin.js +++ b/packages/react-native-renderer/src/NativeMethodsMixin.js @@ -27,6 +27,9 @@ import { warnForStyleProps, } from './NativeMethodsMixinUtils'; +import warningWithoutStack from 'shared/warningWithoutStack'; +import {warnAboutDeprecatedSetNativeProps} from 'shared/ReactFeatureFlags'; + export default function( findNodeHandle: any => ?number, findHostInstance: any => any, @@ -121,6 +124,17 @@ 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. diff --git a/packages/react-native-renderer/src/ReactFabricHostConfig.js b/packages/react-native-renderer/src/ReactFabricHostConfig.js index 2f447cbe63f7d..a1e0b882c6ef6 100644 --- a/packages/react-native-renderer/src/ReactFabricHostConfig.js +++ b/packages/react-native-renderer/src/ReactFabricHostConfig.js @@ -30,6 +30,8 @@ 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'; @@ -140,6 +142,15 @@ 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); } diff --git a/packages/react-native-renderer/src/ReactNativeComponent.js b/packages/react-native-renderer/src/ReactNativeComponent.js index f03e8f7ddf7f7..efd2c51938005 100644 --- a/packages/react-native-renderer/src/ReactNativeComponent.js +++ b/packages/react-native-renderer/src/ReactNativeComponent.js @@ -24,6 +24,9 @@ import UIManager from 'UIManager'; import {create} from './ReactNativeAttributePayload'; import {mountSafeCallback_NOT_REALLY_SAFE} from './NativeMethodsMixinUtils'; +import warningWithoutStack from 'shared/warningWithoutStack'; +import {warnAboutDeprecatedSetNativeProps} from 'shared/ReactFeatureFlags'; + export default function( findNodeHandle: any => ?number, findHostInstance: any => any, @@ -132,6 +135,18 @@ 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. diff --git a/packages/react-native-renderer/src/ReactNativeFiberHostComponent.js b/packages/react-native-renderer/src/ReactNativeFiberHostComponent.js index 748f57c4ac460..af75b088d5af0 100644 --- a/packages/react-native-renderer/src/ReactNativeFiberHostComponent.js +++ b/packages/react-native-renderer/src/ReactNativeFiberHostComponent.js @@ -26,6 +26,9 @@ import { warnForStyleProps, } from './NativeMethodsMixinUtils'; +import warningWithoutStack from 'shared/warningWithoutStack'; +import {warnAboutDeprecatedSetNativeProps} from 'shared/ReactFeatureFlags'; + /** * This component defines the same methods as NativeMethodsMixin but without the * findNodeHandle wrapper. This wrapper is unnecessary for HostComponent views @@ -81,6 +84,15 @@ class ReactNativeFiberHostComponent { 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); } 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 2f19765c9e095..58c9eba11b3c5 100644 --- a/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js @@ -12,6 +12,7 @@ let React; let ReactFabric; +let ReactFeatureFlags; let createReactClass; let createReactNativeComponentClass; let UIManager; @@ -19,6 +20,12 @@ 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"; + jest.mock('shared/ReactFeatureFlags', () => require('shared/forks/ReactFeatureFlags.native-oss'), ); @@ -29,6 +36,8 @@ describe('ReactFabric', () => { React = require('react'); StrictMode = React.StrictMode; + ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactFeatureFlags.warnAboutDeprecatedSetNativeProps = true; ReactFabric = require('react-native-renderer/fabric'); FabricUIManager = require('FabricUIManager'); UIManager = require('UIManager'); @@ -201,10 +210,19 @@ describe('ReactFabric', () => { ); expect(UIManager.updateView).not.toBeCalled(); - viewRef.setNativeProps({}); + expect(() => { + viewRef.setNativeProps({}); + }).toWarnDev([SET_NATIVE_PROPS_DEPRECATION_MESSAGE], { + withoutStack: true, + }); + expect(UIManager.updateView).not.toBeCalled(); - viewRef.setNativeProps({foo: 'baz'}); + expect(() => { + viewRef.setNativeProps({foo: 'baz'}); + }).toWarnDev([SET_NATIVE_PROPS_DEPRECATION_MESSAGE], { + withoutStack: true, + }); expect(UIManager.updateView).toHaveBeenCalledTimes(1); expect(UIManager.updateView).toHaveBeenCalledWith( expect.any(Number), 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 d655f67870fd8..47065e794d320 100644 --- a/packages/react-native-renderer/src/__tests__/ReactNativeMount-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ReactNativeMount-test.internal.js @@ -11,6 +11,7 @@ 'use strict'; let React; +let ReactFeatureFlags; let StrictMode; let ReactNative; let createReactClass; @@ -18,12 +19,20 @@ let createReactNativeComponentClass; let UIManager; 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"; + describe('ReactNative', () => { beforeEach(() => { jest.resetModules(); React = require('react'); StrictMode = React.StrictMode; + ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactFeatureFlags.warnAboutDeprecatedSetNativeProps = true; ReactNative = require('react-native-renderer'); UIManager = require('UIManager'); createReactClass = require('create-react-class/factory')( @@ -98,7 +107,7 @@ describe('ReactNative', () => { expect(UIManager.updateView).toHaveBeenCalledTimes(4); }); - 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', @@ -132,10 +141,19 @@ describe('ReactNative', () => { ); expect(UIManager.updateView).not.toBeCalled(); - viewRef.setNativeProps({}); + expect(() => { + viewRef.setNativeProps({}); + }).toWarnDev([SET_NATIVE_PROPS_DEPRECATION_MESSAGE], { + withoutStack: true, + }); expect(UIManager.updateView).not.toBeCalled(); - viewRef.setNativeProps({foo: 'baz'}); + expect(() => { + viewRef.setNativeProps({foo: 'baz'}); + }).toWarnDev([SET_NATIVE_PROPS_DEPRECATION_MESSAGE], { + withoutStack: true, + }); + expect(UIManager.updateView).toHaveBeenCalledTimes(1); expect(UIManager.updateView).toHaveBeenCalledWith( expect.any(Number), @@ -164,7 +182,9 @@ describe('ReactNative', () => { 11, ); + ReactNative.setNativeProps(viewRef, {}); expect(UIManager.updateView).not.toBeCalled(); + ReactNative.setNativeProps(viewRef, {foo: 'baz'}); expect(UIManager.updateView).toHaveBeenCalledTimes(1); expect(UIManager.updateView).toHaveBeenCalledWith( diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index af5241d1f28b1..ac67f3d8ead1e 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -51,3 +51,7 @@ export const disableInputAttributeSyncing = false; export const enableStableConcurrentModeAPIs = false; export const warnAboutShorthandPropertyCollision = false; + +// See https://github.com/react-native-community/discussions-and-proposals/issues/72 for more information +// This is a flag so we can fix warnings in RN core before turning it on +export const warnAboutDeprecatedSetNativeProps = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 1a308519bad78..b5cef5b36353d 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -27,6 +27,7 @@ export const debugRenderPhaseSideEffectsForStrictMode = true; export const disableInputAttributeSyncing = false; export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__; export const warnAboutDeprecatedLifecycles = true; +export const warnAboutDeprecatedSetNativeProps = true; // Only used in www builds. export function addUserTimingListener() { diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 2e4918dc1e593..25000d24f2fc6 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -24,6 +24,7 @@ export const disableInputAttributeSyncing = false; export const enableStableConcurrentModeAPIs = false; export const warnAboutShorthandPropertyCollision = false; export const enableSchedulerDebugging = false; +export const warnAboutDeprecatedSetNativeProps = false; // Only used in www builds. export function addUserTimingListener() { diff --git a/packages/shared/forks/ReactFeatureFlags.persistent.js b/packages/shared/forks/ReactFeatureFlags.persistent.js index 34cce860a0d5a..6179f0da5634c 100644 --- a/packages/shared/forks/ReactFeatureFlags.persistent.js +++ b/packages/shared/forks/ReactFeatureFlags.persistent.js @@ -24,6 +24,7 @@ export const disableInputAttributeSyncing = false; export const enableStableConcurrentModeAPIs = false; export const warnAboutShorthandPropertyCollision = false; export const enableSchedulerDebugging = false; +export const warnAboutDeprecatedSetNativeProps = false; // Only used in www builds. export function addUserTimingListener() { diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index d33865e96468e..0798a7f7dff99 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -24,6 +24,7 @@ export const disableInputAttributeSyncing = false; export const enableStableConcurrentModeAPIs = false; export const warnAboutShorthandPropertyCollision = false; export const enableSchedulerDebugging = false; +export const warnAboutDeprecatedSetNativeProps = false; // Only used in www builds. export function addUserTimingListener() { diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 09c63908f825f..e51f5810e2e49 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -22,6 +22,7 @@ export const enableSchedulerTracing = false; export const enableSuspenseServerRenderer = false; export const enableStableConcurrentModeAPIs = false; export const enableSchedulerDebugging = false; +export const warnAboutDeprecatedSetNativeProps = false; // Only used in www builds. export function addUserTimingListener() { diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 9bfd3ad3a68be..3048979a9d32e 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -18,6 +18,7 @@ export const { warnAboutDeprecatedLifecycles, disableInputAttributeSyncing, warnAboutShorthandPropertyCollision, + warnAboutDeprecatedSetNativeProps, } = require('ReactFeatureFlags'); // In www, we have experimental support for gathering data