@samitouri / QOS-React / commits / 6ab2869a20

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.

Brian Vaughn committed Aug 22, 2017 at 13:31 UTC 6ab2869a20e5e9a6f1f0ac02da34692dac9c1c9d
5 files changed +121 -22
src/renderers/native/NativeMethodsMixin.js
+13 -6
@@ -183,11 +183,16 @@ function setNativePropsFiber(componentOrHandle: any, nativeProps: Object) {
183 viewConfig.validAttributes,
184 );
185
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 + }
196 }
197
198 // TODO (bvaughn) Remove this once ReactNativeStack is dropped.
@@ -238,7 +243,9 @@ function setNativePropsStack(componentOrHandle: any, nativeProps: Object) {
243 viewConfig.validAttributes,
244 );
245
241 - UIManager.updateView(tag, viewConfig.uiViewClassName, updatePayload);
246 + if (updatePayload) {
247 + UIManager.updateView(tag, viewConfig.uiViewClassName, updatePayload);
248 + }
249 }
250
251 // Switching based on fiber vs stack to avoid a lot of inline checks at runtime.
src/renderers/native/ReactNativeComponent.js
+13 -6
@@ -174,11 +174,16 @@ function setNativePropsFiber(componentOrHandle: any, nativeProps: Object) {
174 viewConfig.validAttributes,
175 );
176
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 + }
187 }
188
189 // TODO (bvaughn) Remove this once ReactNativeStack is dropped.
@@ -225,7 +230,9 @@ function setNativePropsStack(componentOrHandle: any, nativeProps: Object) {
230 viewConfig.validAttributes,
231 );
232
228 - UIManager.updateView(tag, viewConfig.uiViewClassName, updatePayload);
233 + if (updatePayload != null) {
234 + UIManager.updateView(tag, viewConfig.uiViewClassName, updatePayload);
235 + }
236 }
237
238 // Switching based on fiber vs stack to avoid a lot of inline checks at runtime.
src/renderers/native/ReactNativeFiberHostComponent.js
+10 -5
@@ -90,11 +90,16 @@ class ReactNativeFiberHostComponent {
90 this.viewConfig.validAttributes,
91 );
92
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 + }
103 }
104 }
105
src/renderers/native/ReactNativeFiberRenderer.js
+10 -5
@@ -143,11 +143,16 @@ const NativeRenderer = ReactFiberReconciler({
143 viewConfig.validAttributes,
144 );
145
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 + }
156 },
157
158 createInstance(
src/renderers/native/__tests__/ReactNativeMount-test.js
+75
@@ -11,6 +11,7 @@
11
12 'use strict';
13
14 +var PropTypes;
15 var React;
16 var ReactNative;
17 var createReactNativeComponentClass;
@@ -20,6 +21,7 @@ describe('ReactNative', () => {
21 beforeEach(() => {
22 jest.resetModules();
23
24 + PropTypes = require('prop-types');
25 React = require('react');
26 ReactNative = require('react-native');
27 UIManager = require('UIManager');
@@ -60,6 +62,79 @@ describe('ReactNative', () => {
62 expect(UIManager.updateView).toBeCalledWith(2, 'View', {foo: 'bar'});
63 });
64
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 +
138 it('returns the correct instance and calls it in the callback', () => {
139 var View = createReactNativeComponentClass({
140 validAttributes: {foo: true},