@samitouri / QOS-React-2 / commits / 01511ea557
Fork 0

The library for web and native user interfaces.

Remove Unnecessary DOM Mutations

This fixes an edge case that can cause unnecessary mutations in the DOM. Namely, if a prop is falsey, it will get touched on every update by reconciliation. See unit test.

CommitSyncScript committed Jun 13, 2013 at 16:33 UTC 01511ea5575d5fb8c2b0af9d4652f5b20a1d62de
2 files changed +31 -10
src/core/ReactNativeComponent.js
+8 -10
@@ -208,25 +208,23 @@ ReactNativeComponent.Mixin = {
208 _updateDOMProperties: function(nextProps) {
209 var lastProps = this.props;
210 var propKey;
211 - var nextProp;
212 - var lastProp;
211 var styleName;
212 var styleUpdates;
213 for (propKey in lastProps) {
216 - nextProp = nextProps[propKey];
217 - lastProp = lastProps[propKey];
218 - if (!lastProps.hasOwnProperty(propKey) || nextProp) {
214 + if (nextProps.hasOwnProperty(propKey) ||
215 + !lastProps.hasOwnProperty(propKey)) {
216 continue;
217 }
218 if (propKey === STYLE) {
222 - for (styleName in lastProp) {
223 - if (lastProp.hasOwnProperty(styleName)) {
219 + var lastStyle = lastProps[propKey];
220 + for (styleName in lastStyle) {
221 + if (lastStyle.hasOwnProperty(styleName)) {
222 styleUpdates = styleUpdates || {};
223 styleUpdates[styleName] = '';
224 }
225 }
226 } else if (propKey === DANGEROUSLY_SET_INNER_HTML ||
229 - propKey === CONTENT) {
227 + propKey === CONTENT) {
228 ReactComponent.DOMIDOperations.updateTextContentByID(
229 this._rootNodeID,
230 ''
@@ -241,8 +239,8 @@ ReactNativeComponent.Mixin = {
239 }
240 }
241 for (propKey in nextProps) {
244 - nextProp = nextProps[propKey];
245 - lastProp = lastProps[propKey];
242 + var nextProp = nextProps[propKey];
243 + var lastProp = lastProps[propKey];
244 if (!nextProps.hasOwnProperty(propKey) || nextProp === lastProp) {
245 continue;
246 }
src/core/__tests__/ReactNativeComponent-test.js
+23
@@ -21,6 +21,8 @@
21
22 "use strict";
23
24 +var mocks = require('mocks');
25 +
26 describe('ReactNativeComponent', function() {
27
28 describe('updateDOM', function() {
@@ -149,6 +151,27 @@ describe('ReactNativeComponent', function() {
151 expect(stub.getDOMNode().innerHTML).toEqual('');
152 });
153
154 + it("should not incur unnecessary DOM mutations", function() {
155 + var stub = ReactTestUtils.renderIntoDocument(<div value="" />);
156 +
157 + var node = stub.getDOMNode();
158 + var nodeValue = node.value;
159 + var nodeValueSetter = mocks.getMockFunction();
160 + Object.defineProperty(node, 'value', {
161 + get: function() {
162 + return nodeValue;
163 + },
164 + set: nodeValueSetter.mockImplementation(function(newValue) {
165 + nodeValue = newValue;
166 + })
167 + });
168 +
169 + stub.receiveProps({value: ''}, transaction);
170 + expect(nodeValueSetter.mock.calls.length).toBe(0);
171 +
172 + stub.receiveProps({}, transaction);
173 + expect(nodeValueSetter.mock.calls.length).toBe(1);
174 + });
175 });
176
177 describe('createOpenTagMarkup', function() {