Skip to content

Commit

Permalink
Make setNativeProps a no-op with Fabric renderer (#15094)
Browse files Browse the repository at this point in the history 
* Make setNativeProps a no-op with Fabric renderer

* Remove unnecessary __DEV__ check
  • Loading branch information
@elicwhite @bvaughn
elicwhite authored and bvaughn committed Mar 29, 2019
1 parent 08055a6 commit 1b94fd2
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 84 deletions.
31 changes: 20 additions & 11 deletions packages/react-native-renderer/src/NativeMethodsMixin.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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<> =
Expand Down
10 changes: 8 additions & 2 deletions packages/react-native-renderer/src/ReactFabric.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down
35 changes: 6 additions & 29 deletions packages/react-native-renderer/src/ReactFabricHostConfig.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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;
}
}

Expand Down
32 changes: 20 additions & 12 deletions packages/react-native-renderer/src/ReactNativeComponent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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<> =
Expand Down
46 changes: 16 additions & 30 deletions packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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();
});
Expand Down

0 comments on commit 1b94fd2

Please sign in to comment.