diff --git a/src/core/__tests__/ReactDOMComponent-test.js b/src/core/__tests__/ReactDOMComponent-test.js index 973f03cb04..46a9f2064d 100644 --- a/src/core/__tests__/ReactDOMComponent-test.js +++ b/src/core/__tests__/ReactDOMComponent-test.js @@ -187,7 +187,7 @@ describe('ReactDOMComponent', function() { var stub = ReactTestUtils.renderIntoDocument(
); var node = stub.getDOMNode(); - var nodeValue = node.value; + var nodeValue = ''; // node.value always returns undefined var nodeValueSetter = mocks.getMockFunction(); Object.defineProperty(node, 'value', { get: function() { diff --git a/src/dom/DOMProperty.js b/src/dom/DOMProperty.js index 67d0596afe..f6a3a20ebe 100644 --- a/src/dom/DOMProperty.js +++ b/src/dom/DOMProperty.js @@ -29,9 +29,10 @@ var DOMPropertyInjection = { * specifies how the associated DOM property should be accessed or rendered. */ MUST_USE_ATTRIBUTE: 0x1, - MUST_USE_PROPERTY: 0x2, - HAS_BOOLEAN_VALUE: 0x4, - HAS_SIDE_EFFECTS: 0x8, + MUST_USE_PROPERTY: 0x2, + HAS_SIDE_EFFECTS: 0x4, + HAS_BOOLEAN_VALUE: 0x8, + HAS_POSITIVE_NUMERIC_VALUE: 0x10, /** * Inject some specialized knowledge about the DOM. This takes a config object @@ -105,15 +106,17 @@ var DOMPropertyInjection = { propConfig & DOMPropertyInjection.MUST_USE_ATTRIBUTE; DOMProperty.mustUseProperty[propName] = propConfig & DOMPropertyInjection.MUST_USE_PROPERTY; - DOMProperty.hasBooleanValue[propName] = - propConfig & DOMPropertyInjection.HAS_BOOLEAN_VALUE; DOMProperty.hasSideEffects[propName] = propConfig & DOMPropertyInjection.HAS_SIDE_EFFECTS; - + DOMProperty.hasBooleanValue[propName] = + propConfig & DOMPropertyInjection.HAS_BOOLEAN_VALUE; + DOMProperty.hasPositiveNumericValue[propName] = + propConfig & DOMPropertyInjection.HAS_POSITIVE_NUMERIC_VALUE; + invariant( !DOMProperty.mustUseAttribute[propName] || !DOMProperty.mustUseProperty[propName], - 'DOMProperty: Cannot use require using both attribute and property: %s', + 'DOMProperty: Cannot require using both attribute and property: %s', propName ); invariant( @@ -122,6 +125,12 @@ var DOMPropertyInjection = { 'DOMProperty: Properties that have side effects must use property: %s', propName ); + invariant( + !DOMProperty.hasBooleanValue[propName] || + !DOMProperty.hasPositiveNumericValue[propName], + 'DOMProperty: Cannot have both boolean and positive numeric value: %s', + propName + ); } } }; @@ -189,12 +198,6 @@ var DOMProperty = { */ mustUseProperty: {}, - /** - * Whether the property should be removed when set to a falsey value. - * @type {Object} - */ - hasBooleanValue: {}, - /** * Whether or not setting a value causes side effects such as triggering * resources to be loaded or text selection changes. We must ensure that @@ -202,7 +205,20 @@ var DOMProperty = { * @type {Object} */ hasSideEffects: {}, + + /** + * Whether the property should be removed when set to a falsey value. + * @type {Object} + */ + hasBooleanValue: {}, + /** + * Whether the property must be positive numeric or parse as a positive + * numeric and should be removed when set to a falsey value. + * @type {Object} + */ + hasPositiveNumericValue: {}, + /** * All of the isCustomAttribute() functions that have been injected. */ diff --git a/src/dom/DOMPropertyOperations.js b/src/dom/DOMPropertyOperations.js index 2898b41144..fa796137f4 100644 --- a/src/dom/DOMPropertyOperations.js +++ b/src/dom/DOMPropertyOperations.js @@ -24,6 +24,12 @@ var DOMProperty = require('DOMProperty'); var escapeTextForBrowser = require('escapeTextForBrowser'); var memoizeStringOnly = require('memoizeStringOnly'); +function shouldIgnoreValue(name, value) { + return value == null || + DOMProperty.hasBooleanValue[name] && !value || + DOMProperty.hasPositiveNumericValue[name] && (isNaN(value) || value < 1); +}; + var processAttributeNameAndPrefix = memoizeStringOnly(function(name) { return escapeTextForBrowser(name) + '="'; }); @@ -74,7 +80,7 @@ var DOMPropertyOperations = { */ createMarkupForProperty: function(name, value) { if (DOMProperty.isStandardName[name]) { - if (value == null || DOMProperty.hasBooleanValue[name] && !value) { + if (shouldIgnoreValue(name, value)) { return ''; } var attributeName = DOMProperty.getAttributeName[name]; @@ -104,12 +110,10 @@ var DOMPropertyOperations = { var mutationMethod = DOMProperty.getMutationMethod[name]; if (mutationMethod) { mutationMethod(node, value); + } else if (shouldIgnoreValue(name, value)) { + this.deleteValueForProperty(node, name); } else if (DOMProperty.mustUseAttribute[name]) { - if (DOMProperty.hasBooleanValue[name] && !value) { - node.removeAttribute(DOMProperty.getAttributeName[name]); - } else { - node.setAttribute(DOMProperty.getAttributeName[name], '' + value); - } + node.setAttribute(DOMProperty.getAttributeName[name], '' + value); } else { var propName = DOMProperty.getPropertyName[name]; if (!DOMProperty.hasSideEffects[name] || node[propName] !== value) { @@ -117,7 +121,11 @@ var DOMPropertyOperations = { } } } else if (DOMProperty.isCustomAttribute(name)) { - node.setAttribute(name, '' + value); + if (value == null) { + node.removeAttribute(DOMProperty.getAttributeName[name]); + } else { + node.setAttribute(name, '' + value); + } } else if (__DEV__) { warnUnknownProperty(name); } @@ -138,10 +146,14 @@ var DOMPropertyOperations = { node.removeAttribute(DOMProperty.getAttributeName[name]); } else { var propName = DOMProperty.getPropertyName[name]; - node[propName] = DOMProperty.getDefaultValueForProperty( + var defaultValue = DOMProperty.getDefaultValueForProperty( node.nodeName, name ); + if (!DOMProperty.hasSideEffects[name] || + node[propName] !== defaultValue) { + node[propName] = defaultValue; + } } } else if (DOMProperty.isCustomAttribute(name)) { node.removeAttribute(name); diff --git a/src/dom/DefaultDOMPropertyConfig.js b/src/dom/DefaultDOMPropertyConfig.js index d78b46cdc2..270fda08a1 100644 --- a/src/dom/DefaultDOMPropertyConfig.js +++ b/src/dom/DefaultDOMPropertyConfig.js @@ -26,6 +26,7 @@ var MUST_USE_ATTRIBUTE = DOMProperty.injection.MUST_USE_ATTRIBUTE; var MUST_USE_PROPERTY = DOMProperty.injection.MUST_USE_PROPERTY; var HAS_BOOLEAN_VALUE = DOMProperty.injection.HAS_BOOLEAN_VALUE; var HAS_SIDE_EFFECTS = DOMProperty.injection.HAS_SIDE_EFFECTS; +var HAS_POSITIVE_NUMERIC_VALUE = DOMProperty.injection.HAS_POSITIVE_NUMERIC_VALUE; var DefaultDOMPropertyConfig = { isCustomAttribute: RegExp.prototype.test.bind( @@ -50,6 +51,7 @@ var DefaultDOMPropertyConfig = { charSet: MUST_USE_ATTRIBUTE, checked: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE, className: MUST_USE_PROPERTY, + cols: MUST_USE_ATTRIBUTE | HAS_POSITIVE_NUMERIC_VALUE, colSpan: null, content: null, contentEditable: null, @@ -89,11 +91,12 @@ var DefaultDOMPropertyConfig = { rel: null, required: HAS_BOOLEAN_VALUE, role: MUST_USE_ATTRIBUTE, + rows: MUST_USE_ATTRIBUTE | HAS_POSITIVE_NUMERIC_VALUE, rowSpan: null, scrollLeft: MUST_USE_PROPERTY, scrollTop: MUST_USE_PROPERTY, selected: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE, - size: null, + size: MUST_USE_ATTRIBUTE | HAS_POSITIVE_NUMERIC_VALUE, spellCheck: null, src: null, step: null,