From 68edaa3cfd8c4c460b7123ebdb935c841e2133a9 Mon Sep 17 00:00:00 2001 From: Andreas Svensson Date: Tue, 12 Nov 2013 11:03:44 +0100 Subject: [PATCH] Add HAS_POSITIVE_NUMERIC_VALUE to DOMProperty and normalize behavior of null values Uniformly remove null values, rather than sometimes set/remove, could potentially assign 'null' or 'undefined' --- src/dom/DOMProperty.js | 42 ++++++++++++++++++++--------- src/dom/DOMPropertyOperations.js | 45 ++++++++++++++++++++++++-------- 2 files changed, 63 insertions(+), 24 deletions(-) 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 6f629d4083..92094f9f77 100644 --- a/src/dom/DOMPropertyOperations.js +++ b/src/dom/DOMPropertyOperations.js @@ -75,7 +75,10 @@ var DOMPropertyOperations = { */ createMarkupForProperty: function(name, value) { if (DOMProperty.isStandardName[name]) { - if (value == null || DOMProperty.hasBooleanValue[name] && !value) { + if (value == null || + DOMProperty.hasBooleanValue[name] && !value || + DOMProperty.hasPositiveNumericValue[name] && + (isNaN(+value) || +value < 1)) { return ''; } var attributeName = DOMProperty.getAttributeName[name]; @@ -105,20 +108,40 @@ var DOMPropertyOperations = { var mutationMethod = DOMProperty.getMutationMethod[name]; if (mutationMethod) { mutationMethod(node, value); - } else if (DOMProperty.mustUseAttribute[name]) { - if (DOMProperty.hasBooleanValue[name] && !value) { - node.removeAttribute(DOMProperty.getAttributeName[name]); - } else { - node.setAttribute(DOMProperty.getAttributeName[name], '' + value); - } } else { - var propName = DOMProperty.getPropertyName[name]; - if (!DOMProperty.hasSideEffects[name] || node[propName] !== value) { - node[propName] = value; + if (value == null || + DOMProperty.hasBooleanValue[name] && !value || + DOMProperty.hasPositiveNumericValue[name] && + (isNaN(+value) || +value < 1)) { + if (DOMProperty.mustUseAttribute[name]) { + node.removeAttribute(DOMProperty.getAttributeName[name]); + } else { + var propName = DOMProperty.getPropertyName[name]; + value = DOMProperty.getDefaultValueForProperty( + node.nodeName, + name + ); + if (!DOMProperty.hasSideEffects[name] || + node[propName] !== value) { + node[propName] = value; + } + } + } else if (DOMProperty.mustUseAttribute[name]) { + node.setAttribute(DOMProperty.getAttributeName[name], '' + value); + } else { + var propName = DOMProperty.getPropertyName[name]; + if (!DOMProperty.hasSideEffects[name] || + node[propName] !== value) { + node[propName] = value; + } } } } 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); }