From 6c331fba07d49f05594d6014899e2c95d159b5f2 Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Wed, 26 Feb 2014 21:13:35 -0800 Subject: [PATCH] Add hasOwnProperty checks where appropriate For boolean-like objects, I've added hasOwnProperty checks in addition to the existing truthiness check even though for most of these dicts, we leave out false keys instead of setting them explicitly to false. In DOMPropertyOperations, we don't need to check hasOwnProperty if the property is in isStandardName because (with the exception of getPossibleStandardName) the dicts on DOMProperty will now be consistently populated with every valid attribute. --- .../ReactTransitionChildMapping.js | 4 ++-- src/addons/update.js | 2 +- src/browser/ReactEventEmitter.js | 9 +++++--- .../AnalyticsEventPluginFactory.js | 13 +++++------ src/browser/ui/ReactDOMComponent.js | 6 ++--- src/browser/ui/dom/DOMProperty.js | 22 +++++++++++-------- src/browser/ui/dom/DOMPropertyOperations.js | 21 +++++++++++++----- src/browser/ui/dom/dangerousStyleValue.js | 13 ++++++----- src/core/ReactCompositeComponent.js | 7 ++++-- src/event/EventPluginRegistry.js | 5 +++-- 10 files changed, 62 insertions(+), 40 deletions(-) diff --git a/src/addons/transitions/ReactTransitionChildMapping.js b/src/addons/transitions/ReactTransitionChildMapping.js index 8b69a570ae..11b6a4447c 100644 --- a/src/addons/transitions/ReactTransitionChildMapping.js +++ b/src/addons/transitions/ReactTransitionChildMapping.js @@ -70,7 +70,7 @@ var ReactTransitionChildMapping = { var pendingKeys = []; for (var prevKey in prev) { - if (next[prevKey]) { + if (next.hasOwnProperty(prevKey)) { if (pendingKeys.length) { nextKeysPending[prevKey] = pendingKeys; pendingKeys = []; @@ -83,7 +83,7 @@ var ReactTransitionChildMapping = { var i; var childMapping = {}; for (var nextKey in next) { - if (nextKeysPending[nextKey]) { + if (nextKeysPending.hasOwnProperty(nextKey)) { for (i = 0; i < nextKeysPending[nextKey].length; i++) { var pendingNextKey = nextKeysPending[nextKey][i]; childMapping[nextKeysPending[nextKey][i]] = getValueForKey( diff --git a/src/addons/update.js b/src/addons/update.js index 959d2107b9..2eb79eca52 100644 --- a/src/addons/update.js +++ b/src/addons/update.js @@ -148,7 +148,7 @@ function update(value, spec) { } for (var k in spec) { - if (!ALL_COMMANDS_SET[k]) { + if (!(ALL_COMMANDS_SET.hasOwnProperty(k) && ALL_COMMANDS_SET[k])) { nextValue[k] = update(value[k], spec[k]); } } diff --git a/src/browser/ReactEventEmitter.js b/src/browser/ReactEventEmitter.js index c09ed460de..5481c2e9b8 100644 --- a/src/browser/ReactEventEmitter.js +++ b/src/browser/ReactEventEmitter.js @@ -129,7 +129,7 @@ var topEventMapping = { var topListenersIDKey = "_reactListenersID" + String(Math.random()).slice(2); function getListeningForDocument(mountAt) { - if (mountAt[topListenersIDKey] == null) { + if (!mountAt.hasOwnProperty(topListenersIDKey)) { mountAt[topListenersIDKey] = reactTopListenersCounter++; alreadyListeningTo[mountAt[topListenersIDKey]] = {}; } @@ -254,7 +254,10 @@ var ReactEventEmitter = merge(ReactEventEmitterMixin, { var topLevelTypes = EventConstants.topLevelTypes; for (var i = 0, l = dependencies.length; i < l; i++) { var dependency = dependencies[i]; - if (!isListening[dependency]) { + if (!( + isListening.hasOwnProperty(dependency) && + isListening[dependency] + )) { var topLevelType = topLevelTypes[dependency]; if (topLevelType === topLevelTypes.topWheel) { @@ -293,7 +296,7 @@ var ReactEventEmitter = merge(ReactEventEmitterMixin, { // to make sure blur and focus event listeners are only attached once isListening[topLevelTypes.topBlur] = true; isListening[topLevelTypes.topFocus] = true; - } else if (topEventMapping[dependency]) { + } else if (topEventMapping.hasOwnProperty(dependency)) { trapBubbledEvent(topLevelType, topEventMapping[dependency], mountAt); } diff --git a/src/browser/eventPlugins/AnalyticsEventPluginFactory.js b/src/browser/eventPlugins/AnalyticsEventPluginFactory.js index 571fb21aad..3bd5b7a3b8 100644 --- a/src/browser/eventPlugins/AnalyticsEventPluginFactory.js +++ b/src/browser/eventPlugins/AnalyticsEventPluginFactory.js @@ -149,19 +149,18 @@ function extractEvents( topLevelTargetID, nativeEvent) { var currentEvent = topLevelTypesToAnalyticsEvent[topLevelType]; - if (!currentEvent || !topLevelTarget || !topLevelTarget.attributes) { + if (!currentEvent || !topLevelTarget) { return null; } - var analyticsIDAttribute = topLevelTarget.attributes[ANALYTICS_ID]; - var analyticsEventsAttribute = topLevelTarget.attributes[ANALYTICS_EVENTS]; - if (!analyticsIDAttribute || !analyticsEventsAttribute) { + var analyticsID = topLevelTarget.getAttribute(ANALYTICS_ID); + var analyticsEventsStr = topLevelTarget.getAttribute(ANALYTICS_EVENTS); + if (!analyticsID || !analyticsEventsStr) { return null; } - var analyticsEventsArr = analyticsEventsAttribute.value.split(","); - var analyticsID = analyticsIDAttribute.value; - if (!analyticsData[analyticsID]) { + var analyticsEventsArr = analyticsEventsStr.split(","); + if (!analyticsData.hasOwnProperty(analyticsID)) { initAnalyticsDataForID(analyticsID, analyticsEventsArr); } diff --git a/src/browser/ui/ReactDOMComponent.js b/src/browser/ui/ReactDOMComponent.js index 6b65664e86..e8ecbbf230 100644 --- a/src/browser/ui/ReactDOMComponent.js +++ b/src/browser/ui/ReactDOMComponent.js @@ -147,7 +147,7 @@ ReactDOMComponent.Mixin = { if (propValue == null) { continue; } - if (registrationNameModules[propKey]) { + if (registrationNameModules.hasOwnProperty(propKey)) { putListener(this._rootNodeID, propKey, propValue, transaction); } else { if (propKey === STYLE) { @@ -282,7 +282,7 @@ ReactDOMComponent.Mixin = { styleUpdates[styleName] = ''; } } - } else if (registrationNameModules[propKey]) { + } else if (registrationNameModules.hasOwnProperty(propKey)) { deleteListener(this._rootNodeID, propKey); } else if ( DOMProperty.isStandardName[propKey] || @@ -324,7 +324,7 @@ ReactDOMComponent.Mixin = { // Relies on `updateStylesByID` not mutating `styleUpdates`. styleUpdates = nextProp; } - } else if (registrationNameModules[propKey]) { + } else if (registrationNameModules.hasOwnProperty(propKey)) { putListener(this._rootNodeID, propKey, nextProp, transaction); } else if ( DOMProperty.isStandardName[propKey] || diff --git a/src/browser/ui/dom/DOMProperty.js b/src/browser/ui/dom/DOMProperty.js index 8d3d6626d0..46d757c597 100644 --- a/src/browser/ui/dom/DOMProperty.js +++ b/src/browser/ui/dom/DOMProperty.js @@ -75,7 +75,7 @@ var DOMPropertyInjection = { for (var propName in Properties) { invariant( - !DOMProperty.isStandardName[propName], + !DOMProperty.isStandardName.hasOwnProperty(propName), 'injectDOMPropertyConfig(...): You\'re trying to inject DOM property ' + '\'%s\' which has already been injected. You may be accidentally ' + 'injecting the same DOM property config twice, or you may be ' + @@ -88,19 +88,23 @@ var DOMPropertyInjection = { var lowerCased = propName.toLowerCase(); DOMProperty.getPossibleStandardName[lowerCased] = propName; - var attributeName = DOMAttributeNames[propName]; - if (attributeName) { + if (DOMAttributeNames.hasOwnProperty(propName)) { + var attributeName = DOMAttributeNames[propName]; DOMProperty.getPossibleStandardName[attributeName] = propName; + DOMProperty.getAttributeName[propName] = attributeName; + } else { + DOMProperty.getAttributeName[propName] = lowerCased; } - DOMProperty.getAttributeName[propName] = attributeName || lowerCased; - DOMProperty.getPropertyName[propName] = - DOMPropertyNames[propName] || propName; + DOMPropertyNames.hasOwnProperty(propName) ? + DOMPropertyNames[propName] : + propName; - var mutationMethod = DOMMutationMethods[propName]; - if (mutationMethod) { - DOMProperty.getMutationMethod[propName] = mutationMethod; + if (DOMMutationMethods.hasOwnProperty(propName)) { + DOMProperty.getMutationMethod[propName] = DOMMutationMethods[propName]; + } else { + DOMProperty.getMutationMethod[propName] = null; } var propConfig = Properties[propName]; diff --git a/src/browser/ui/dom/DOMPropertyOperations.js b/src/browser/ui/dom/DOMPropertyOperations.js index 12a3502bb9..828c8325ce 100644 --- a/src/browser/ui/dom/DOMPropertyOperations.js +++ b/src/browser/ui/dom/DOMPropertyOperations.js @@ -47,7 +47,8 @@ if (__DEV__) { var warnedProperties = {}; var warnUnknownProperty = function(name) { - if (reactProps[name] || warnedProperties[name]) { + if (reactProps.hasOwnProperty(name) && reactProps[name] || + warnedProperties.hasOwnProperty(name) && warnedProperties[name]) { return; } @@ -55,8 +56,13 @@ if (__DEV__) { var lowerCasedName = name.toLowerCase(); // data-* attributes should be lowercase; suggest the lowercase version - var standardName = DOMProperty.isCustomAttribute(lowerCasedName) ? - lowerCasedName : DOMProperty.getPossibleStandardName[lowerCasedName]; + var standardName = ( + DOMProperty.isCustomAttribute(lowerCasedName) ? + lowerCasedName : + DOMProperty.getPossibleStandardName.hasOwnProperty(lowerCasedName) ? + DOMProperty.getPossibleStandardName[lowerCasedName] : + null + ); // For now, only warn when we have a suggested correction. This prevents // logging too much when using transferPropsTo. @@ -92,7 +98,8 @@ var DOMPropertyOperations = { * @return {?string} Markup string, or null if the property was invalid. */ createMarkupForProperty: function(name, value) { - if (DOMProperty.isStandardName[name]) { + if (DOMProperty.isStandardName.hasOwnProperty(name) && + DOMProperty.isStandardName[name]) { if (shouldIgnoreValue(name, value)) { return ''; } @@ -123,7 +130,8 @@ var DOMPropertyOperations = { * @param {*} value */ setValueForProperty: function(node, name, value) { - if (DOMProperty.isStandardName[name]) { + if (DOMProperty.isStandardName.hasOwnProperty(name) && + DOMProperty.isStandardName[name]) { var mutationMethod = DOMProperty.getMutationMethod[name]; if (mutationMethod) { mutationMethod(node, value); @@ -155,7 +163,8 @@ var DOMPropertyOperations = { * @param {string} name */ deleteValueForProperty: function(node, name) { - if (DOMProperty.isStandardName[name]) { + if (DOMProperty.isStandardName.hasOwnProperty(name) && + DOMProperty.isStandardName[name]) { var mutationMethod = DOMProperty.getMutationMethod[name]; if (mutationMethod) { mutationMethod(node, undefined); diff --git a/src/browser/ui/dom/dangerousStyleValue.js b/src/browser/ui/dom/dangerousStyleValue.js index 2118f2b2a6..44cbe8166d 100644 --- a/src/browser/ui/dom/dangerousStyleValue.js +++ b/src/browser/ui/dom/dangerousStyleValue.js @@ -21,16 +21,18 @@ var CSSProperty = require('CSSProperty'); +var isUnitlessNumber = CSSProperty.isUnitlessNumber; + /** - * Convert a value into the proper css writable value. The `styleName` name - * name should be logical (no hyphens), as specified + * Convert a value into the proper css writable value. The style name `name` + * should be logical (no hyphens), as specified * in `CSSProperty.isUnitlessNumber`. * - * @param {string} styleName CSS property name such as `topMargin`. + * @param {string} name CSS property name such as `topMargin`. * @param {*} value CSS property value such as `10px`. * @return {string} Normalized style value with dimensions applied. */ -function dangerousStyleValue(styleName, value) { +function dangerousStyleValue(name, value) { // Note that we've removed escapeTextForBrowser() calls here since the // whole string will be escaped when the attribute is injected into // the markup. If you provide unsafe user data here they can inject @@ -47,7 +49,8 @@ function dangerousStyleValue(styleName, value) { } var isNonNumeric = isNaN(value); - if (isNonNumeric || value === 0 || CSSProperty.isUnitlessNumber[styleName]) { + if (isNonNumeric || value === 0 || + isUnitlessNumber.hasOwnProperty(name) && isUnitlessNumber[name]) { return '' + value; // cast to string } diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index 668074bdae..daf90b59ed 100644 --- a/src/core/ReactCompositeComponent.js +++ b/src/core/ReactCompositeComponent.js @@ -389,7 +389,9 @@ function validateTypeDef(Constructor, typeDef, location) { } function validateMethodOverride(proto, name) { - var specPolicy = ReactCompositeComponentInterface[name]; + var specPolicy = ReactCompositeComponentInterface.hasOwnProperty(name) ? + ReactCompositeComponentInterface[name] : + null; // Disallow overriding of base class methods unless explicitly allowed. if (ReactCompositeComponentMixin.hasOwnProperty(name)) { @@ -923,7 +925,8 @@ var ReactCompositeComponentMixin = { var props = merge(newProps); var defaultProps = this._defaultProps; for (var propName in defaultProps) { - if (typeof props[propName] === 'undefined') { + if (!props.hasOwnProperty(propName) || + typeof props[propName] === 'undefined') { props[propName] = defaultProps[propName]; } } diff --git a/src/event/EventPluginRegistry.js b/src/event/EventPluginRegistry.js index 2dbb3fb2c0..2441bdace1 100644 --- a/src/event/EventPluginRegistry.js +++ b/src/event/EventPluginRegistry.js @@ -86,7 +86,7 @@ function recomputePluginOrdering() { */ function publishEventForPlugin(dispatchConfig, PluginModule, eventName) { invariant( - !EventPluginRegistry.eventNameDispatchConfigs[eventName], + !EventPluginRegistry.eventNameDispatchConfigs.hasOwnProperty(eventName), 'EventPluginHub: More than one plugin attempted to publish the same ' + 'event name, `%s`.', eventName @@ -201,7 +201,8 @@ var EventPluginRegistry = { continue; } var PluginModule = injectedNamesToPlugins[pluginName]; - if (namesToPlugins[pluginName] !== PluginModule) { + if (!namesToPlugins.hasOwnProperty(pluginName) || + namesToPlugins[pluginName] !== PluginModule) { invariant( !namesToPlugins[pluginName], 'EventPluginRegistry: Cannot inject two different event plugins ' +