@samitouri / QOS-React-2 / commits / 0493b27222
Fork 0

The library for web and native user interfaces.

Don't update value or textContent unnecessarily

_updateDOMChildren was already updating textContent so we don't need to do it in _updateDOMProperties. Additionally, don't update .value if it'll be a noop because it has side-effects (like moving the cursor) in some browsers (like IE 9). Refactor tests to be a bit more robust and a bit cleaner too.

Ben Alpert committed Jun 13, 2013 at 20:27 UTC 0493b27222bd8da9b724b46200bf393c120b1ec9
3 files changed +78 -42
src/core/ReactDOMTextarea.js
+32 -15
@@ -30,8 +30,34 @@ var textarea = ReactDOM.textarea;
30 var CONTENT_TYPES = {'string': true, 'number': true};
31
32 var getTextContent = function(props) {
33 - return props.content != null ? '' + props.content :
34 - CONTENT_TYPES[typeof props.children] ? '' + props.children : '';
33 + if (!props) {
34 + return '';
35 + }
36 +
37 + invariant(
38 + props.dangerouslySetInnerHTML == null,
39 + '`dangerouslySetInnerHTML` does not make sense on textarea.'
40 + );
41 +
42 + var content;
43 + if (Array.isArray(props.children)) {
44 + invariant(
45 + props.children.length <= 1,
46 + 'textarea can have at most one child'
47 + );
48 + content = props.children[0];
49 + } else if (props.children != null) {
50 + content = props.children;
51 + } else {
52 + content = props.content;
53 + }
54 +
55 + invariant(
56 + content == null || CONTENT_TYPES[typeof content],
57 + 'textarea must contain a single string or number, not an array or ' +
58 + 'object.'
59 + );
60 + return content != null ? '' + content : '';
61 };
62
63 /**
@@ -40,33 +66,24 @@ var getTextContent = function(props) {
66 */
67 var ReactDOMTextarea = ReactCompositeComponent.createClass({
68 getInitialState: function() {
43 - // We keep the original value of content and children here so that
69 + // We keep the original value of content or children here so that
70 // ReactNativeComponent doesn't update textContent (unnecessary since we
71 // update value).
72 return {
47 - initialContent: this.props.content,
48 - initialChildren: this.props.children
73 + initialContent: getTextContent(this.props)
74 };
75 },
76
77 render: function() {
53 - invariant(
54 - this.props.dangerouslySetInnerHTML == null,
55 - '`dangerouslySetInnerHTML` does not make sense on textarea.'
56 - );
57 - invariant(
58 - this.props.children == null || CONTENT_TYPES[typeof this.props.children],
59 - 'When present, textarea children must be a single string or number.'
60 - );
78 return this.transferPropsTo(textarea({
79 content: this.state.initialContent
63 - }, this.state.initialChildren));
80 + }));
81 },
82
83 componentDidUpdate: function(prevProps, prevState, rootNode) {
84 var oldContent = getTextContent(prevProps);
85 var newContent = getTextContent(this.props);
69 - if (oldContent !== newContent) {
86 + if (oldContent !== newContent && rootNode.value !== newContent) {
87 rootNode.value = newContent;
88 }
89 }
src/core/ReactNativeComponent.js
+4 -6
@@ -219,12 +219,13 @@ ReactNativeComponent.Mixin = {
219 styleUpdates[styleName] = '';
220 }
221 }
222 - } else if (propKey === DANGEROUSLY_SET_INNER_HTML ||
223 - propKey === CONTENT) {
222 + } else if (propKey === DANGEROUSLY_SET_INNER_HTML) {
223 ReactComponent.DOMIDOperations.updateTextContentByID(
224 this._rootNodeID,
225 ''
226 );
227 + } else if (propKey === CONTENT) {
228 + // (Removal happens in _updateDOMChildren)
229 } else if (registrationNames[propKey]) {
230 deleteListener(this._rootNodeID, propKey);
231 } else {
@@ -275,10 +276,7 @@ ReactNativeComponent.Mixin = {
276 );
277 }
278 } else if (propKey === CONTENT) {
278 - ReactComponent.DOMIDOperations.updateTextContentByID(
279 - this._rootNodeID,
280 - '' + nextProp
281 - );
279 + // (Update happens in _updateDOMChildren)
280 } else if (registrationNames[propKey]) {
281 putListener(this._rootNodeID, propKey, nextProp);
282 } else {
src/core/__tests__/ReactDOMTextarea-test.js
+42 -21
@@ -19,45 +19,66 @@
19
20 "use strict";
21
22 +var mocks = require('mocks');
23 +
24 describe('ReactDOMTextarea', function() {
25 var React;
26 var ReactTestUtils;
25 - var transaction;
27 + var getTextContentAccessor;
28
29 beforeEach(function() {
30 React = require('React');
31 ReactTestUtils = require('ReactTestUtils');
32 + getTextContentAccessor = require('getTextContentAccessor');
33 });
34
32 - it("should remove value with removed children", function() {
35 + it("should update value and not update textContent", function() {
36 + var textContentAccessor = getTextContentAccessor();
37 var stub = ReactTestUtils.renderIntoDocument(<textarea>giraffe</textarea>);
38 + var node = stub.getDOMNode();
39
35 - expect(stub.getDOMNode().value).toEqual('giraffe');
36 - stub.replaceProps({ children: null });
37 - expect(stub.getDOMNode().value).toEqual('');
38 - });
40 + var nodeText = node[textContentAccessor];
41 + var nodeTextSetter = mocks.getMockFunction();
42 + Object.defineProperty(node, textContentAccessor, {
43 + get: function() {
44 + return nodeText;
45 + },
46 + set: nodeTextSetter.mockImplementation(function(newText) {
47 + nodeText = newText;
48 + })
49 + });
50
40 - it("should update value with a single string child", function() {
41 - var stub = ReactTestUtils.renderIntoDocument(<textarea>monkey</textarea>);
51 + expect(node.value).toEqual('giraffe');
52
43 - expect(stub.getDOMNode().value).toEqual('monkey');
53 stub.replaceProps({ children: 'gorilla' });
45 - expect(stub.getDOMNode().value).toEqual('gorilla');
46 - });
54 + expect(node.value).toEqual('gorilla');
55
48 - it("should update value with a single numerical child", function() {
49 - var stub = ReactTestUtils.renderIntoDocument(<textarea>17</textarea>);
56 + stub.replaceProps({ children: 17 });
57 + expect(node.value).toEqual('17');
58
51 - expect(stub.getDOMNode().value).toEqual('17');
52 - stub.replaceProps({ children: 289 });
53 - expect(stub.getDOMNode().value).toEqual('289');
54 - });
59 + stub.replaceProps({ children: [42] });
60 + expect(node.value).toEqual('42');
61
56 - it("should update value with the content property", function() {
57 - var stub = ReactTestUtils.renderIntoDocument(<textarea content="purple" />);
62 + stub.replaceProps({ children: null });
63 + expect(node.value).toEqual('');
64
59 - expect(stub.getDOMNode().value).toEqual('purple');
65 stub.replaceProps({ content: 'eggplant' });
61 - expect(stub.getDOMNode().value).toEqual('eggplant');
66 + expect(node.value).toEqual('eggplant');
67 +
68 + expect(nodeTextSetter.mock.calls.length).toBe(0);
69 + });
70 +
71 + it("should throw with multiple or invalid children", function() {
72 + expect(function() {
73 + var stub = ReactTestUtils.renderIntoDocument(
74 + <textarea>{'hello'}{'there'}</textarea>
75 + );
76 + }).toThrow();
77 +
78 + expect(function() {
79 + var stub = ReactTestUtils.renderIntoDocument(
80 + <textarea><strong /></textarea>
81 + );
82 + }).toThrow();
83 });
84 });