Skip to content

Commit 6ab2869

Browse files
bvaughnbvaughn
authored
RN (fiber) avoids the overhead of bridge calls if there's no update. (#10505)
This is an expensive no-op for Android, and causes an unnecessary view invalidation for certain components (eg RCTTextInput) on iOS.
1 parent 640eb70 commit 6ab2869

File tree

5 files changed

+121
-22
lines changed

5 files changed

+121
-22
lines changed

‎src/renderers/native/NativeMethodsMixin.js‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -183,11 +183,16 @@ function setNativePropsFiber(componentOrHandle: any, nativeProps: Object) {
183183
viewConfig.validAttributes,
184184
);
185185

186-
UIManager.updateView(
187-
maybeInstance._nativeTag,
188-
viewConfig.uiViewClassName,
189-
updatePayload,
190-
);
186+
// Avoid the overhead of bridge calls if there's no update.
187+
// This is an expensive no-op for Android, and causes an unnecessary
188+
// view invalidation for certain components (eg RCTTextInput) on iOS.
189+
if (updatePayload != null) {
190+
UIManager.updateView(
191+
maybeInstance._nativeTag,
192+
viewConfig.uiViewClassName,
193+
updatePayload,
194+
);
195+
}
191196
}
192197

193198
// TODO (bvaughn) Remove this once ReactNativeStack is dropped.
@@ -238,7 +243,9 @@ function setNativePropsStack(componentOrHandle: any, nativeProps: Object) {
238243
viewConfig.validAttributes,
239244
);
240245

241-
UIManager.updateView(tag, viewConfig.uiViewClassName, updatePayload);
246+
if (updatePayload) {
247+
UIManager.updateView(tag, viewConfig.uiViewClassName, updatePayload);
248+
}
242249
}
243250

244251
// Switching based on fiber vs stack to avoid a lot of inline checks at runtime.

‎src/renderers/native/ReactNativeComponent.js‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -174,11 +174,16 @@ function setNativePropsFiber(componentOrHandle: any, nativeProps: Object) {
174174
viewConfig.validAttributes,
175175
);
176176

177-
UIManager.updateView(
178-
maybeInstance._nativeTag,
179-
viewConfig.uiViewClassName,
180-
updatePayload,
181-
);
177+
// Avoid the overhead of bridge calls if there's no update.
178+
// This is an expensive no-op for Android, and causes an unnecessary
179+
// view invalidation for certain components (eg RCTTextInput) on iOS.
180+
if (updatePayload != null) {
181+
UIManager.updateView(
182+
maybeInstance._nativeTag,
183+
viewConfig.uiViewClassName,
184+
updatePayload,
185+
);
186+
}
182187
}
183188

184189
// TODO (bvaughn) Remove this once ReactNativeStack is dropped.
@@ -225,7 +230,9 @@ function setNativePropsStack(componentOrHandle: any, nativeProps: Object) {
225230
viewConfig.validAttributes,
226231
);
227232

228-
UIManager.updateView(tag, viewConfig.uiViewClassName, updatePayload);
233+
if (updatePayload != null) {
234+
UIManager.updateView(tag, viewConfig.uiViewClassName, updatePayload);
235+
}
229236
}
230237

231238
// Switching based on fiber vs stack to avoid a lot of inline checks at runtime.

‎src/renderers/native/ReactNativeFiberHostComponent.js‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,16 @@ class ReactNativeFiberHostComponent {
9090
this.viewConfig.validAttributes,
9191
);
9292

93-
UIManager.updateView(
94-
this._nativeTag,
95-
this.viewConfig.uiViewClassName,
96-
updatePayload,
97-
);
93+
// Avoid the overhead of bridge calls if there's no update.
94+
// This is an expensive no-op for Android, and causes an unnecessary
95+
// view invalidation for certain components (eg RCTTextInput) on iOS.
96+
if (updatePayload != null) {
97+
UIManager.updateView(
98+
this._nativeTag,
99+
this.viewConfig.uiViewClassName,
100+
updatePayload,
101+
);
102+
}
98103
}
99104
}
100105

‎src/renderers/native/ReactNativeFiberRenderer.js‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -143,11 +143,16 @@ const NativeRenderer = ReactFiberReconciler({
143143
viewConfig.validAttributes,
144144
);
145145

146-
UIManager.updateView(
147-
instance._nativeTag, // reactTag
148-
viewConfig.uiViewClassName, // viewName
149-
updatePayload, // props
150-
);
146+
// Avoid the overhead of bridge calls if there's no update.
147+
// This is an expensive no-op for Android, and causes an unnecessary
148+
// view invalidation for certain components (eg RCTTextInput) on iOS.
149+
if (updatePayload != null) {
150+
UIManager.updateView(
151+
instance._nativeTag, // reactTag
152+
viewConfig.uiViewClassName, // viewName
153+
updatePayload, // props
154+
);
155+
}
151156
},
152157

153158
createInstance(

‎src/renderers/native/__tests__/ReactNativeMount-test.js‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
'use strict';
1313

14+
var PropTypes;
1415
var React;
1516
var ReactNative;
1617
var createReactNativeComponentClass;
@@ -20,6 +21,7 @@ describe('ReactNative', () => {
2021
beforeEach(() => {
2122
jest.resetModules();
2223

24+
PropTypes = require('prop-types');
2325
React = require('react');
2426
ReactNative = require('react-native');
2527
UIManager = require('UIManager');
@@ -60,6 +62,79 @@ describe('ReactNative', () => {
6062
expect(UIManager.updateView).toBeCalledWith(2, 'View', {foo: 'bar'});
6163
});
6264

65+
it('should not call UIManager.updateView after render for properties that have not changed', () => {
66+
const Text = createReactNativeComponentClass({
67+
validAttributes: {foo: true},
68+
uiViewClassName: 'Text',
69+
});
70+
71+
// Context hack is required for RN text rendering in stack.
72+
// TODO Remove this from the test when RN stack has been deleted.
73+
class Hack extends React.Component {
74+
static childContextTypes = {isInAParentText: PropTypes.bool};
75+
getChildContext() {
76+
return {isInAParentText: true};
77+
}
78+
render() {
79+
return this.props.children;
80+
}
81+
}
82+
83+
ReactNative.render(<Hack><Text foo="a">1</Text></Hack>, 11);
84+
expect(UIManager.updateView).not.toBeCalled();
85+
86+
// If no properties have changed, we shouldn't call updateView.
87+
ReactNative.render(<Hack><Text foo="a">1</Text></Hack>, 11);
88+
expect(UIManager.updateView).not.toBeCalled();
89+
90+
// Only call updateView for the changed property (and not for text).
91+
ReactNative.render(<Hack><Text foo="b">1</Text></Hack>, 11);
92+
expect(UIManager.updateView.mock.calls.length).toBe(1);
93+
94+
// Only call updateView for the changed text (and no other properties).
95+
ReactNative.render(<Hack><Text foo="b">2</Text></Hack>, 11);
96+
expect(UIManager.updateView.mock.calls.length).toBe(2);
97+
98+
// Call updateView for both changed text and properties.
99+
ReactNative.render(<Hack><Text foo="c">3</Text></Hack>, 11);
100+
expect(UIManager.updateView.mock.calls.length).toBe(4);
101+
});
102+
103+
it('should not call UIManager.updateView from setNativeProps for properties that have not changed', () => {
104+
const View = createReactNativeComponentClass({
105+
validAttributes: {foo: true},
106+
uiViewClassName: 'View',
107+
});
108+
109+
class Subclass extends ReactNative.NativeComponent {
110+
render() {
111+
return <View />;
112+
}
113+
}
114+
115+
[View, Subclass].forEach(Component => {
116+
UIManager.updateView.mockReset();
117+
118+
let viewRef;
119+
ReactNative.render(
120+
<Component
121+
foo="bar"
122+
ref={ref => {
123+
viewRef = ref;
124+
}}
125+
/>,
126+
11,
127+
);
128+
expect(UIManager.updateView).not.toBeCalled();
129+
130+
viewRef.setNativeProps({});
131+
expect(UIManager.updateView).not.toBeCalled();
132+
133+
viewRef.setNativeProps({foo: 'baz'});
134+
expect(UIManager.updateView.mock.calls.length).toBe(1);
135+
});
136+
});
137+
63138
it('returns the correct instance and calls it in the callback', () => {
64139
var View = createReactNativeComponentClass({
65140
validAttributes: {foo: true},

0 commit comments

Comments
 (0)