From 8e267062b3e4d82d0a79be984741af6736820e4a Mon Sep 17 00:00:00 2001 From: petehunt Date: Sun, 30 Jun 2013 12:57:07 -0700 Subject: [PATCH 1/6] Injectable DOM properties --- src/core/ReactDefaultInjection.js | 11 +- src/dom/DOMProperty.js | 260 ++++++++++------------------ src/dom/DefaultDOMPropertyConfig.js | 140 +++++++++++++++ 3 files changed, 241 insertions(+), 170 deletions(-) create mode 100644 src/dom/DefaultDOMPropertyConfig.js diff --git a/src/core/ReactDefaultInjection.js b/src/core/ReactDefaultInjection.js index 33577220f244c..499f1cdef3ae0 100644 --- a/src/core/ReactDefaultInjection.js +++ b/src/core/ReactDefaultInjection.js @@ -22,6 +22,9 @@ var ReactDOM = require('ReactDOM'); var ReactDOMForm = require('ReactDOMForm'); var ReactDOMTextarea = require('ReactDOMTextarea'); +var DefaultDOMPropertyConfig = require('DefaultDOMPropertyConfig'); +var DOMProperty = require('DOMProperty'); + var DefaultEventPluginOrder = require('DefaultEventPluginOrder'); var EnterLeaveEventPlugin = require('EnterLeaveEventPlugin'); var ChangeEventPlugin = require('ChangeEventPlugin'); @@ -46,17 +49,13 @@ function inject() { 'ChangeEventPlugin': ChangeEventPlugin }); - /** - * This is a bit of a hack. We need to override the
element to be a - * composite component because IE8 does not bubble or capture submit to the - * top level. In order to make this work with our dependency graph we need to - * inject it here. - */ ReactDOM.injection.injectComponentClasses({ form: ReactDOMForm, // TODO: Inject `ReactDOMInput`. textarea: ReactDOMTextarea }); + + DOMProperty.injection.injectDOMPropertyConfig(DefaultDOMPropertyConfig); } module.exports = { diff --git a/src/dom/DOMProperty.js b/src/dom/DOMProperty.js index 0d763036a7fa6..d5f78e13929dd 100644 --- a/src/dom/DOMProperty.js +++ b/src/dom/DOMProperty.js @@ -23,6 +23,87 @@ var invariant = require('invariant'); +var DOMPropertyInjection = { + /** + * Mapping from normalized, camelcased property names to a configuration that + * 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, + + /** + * Inject some specialized knowledge about the DOM. This takes a config object with the + * following properties: + * + * isCustomAttribute: function that given an attribute name will return true if it can + * be inserted into the DOM verbatim. Useful for data-* or aria-* attributes where it's + * impossible to enumerate all of the possible attribute names, + * + * Properties: object mapping DOM property name to one of the DOMPropertyInjection + * constants or null. If your attribute isn't in here, it won't get written to the DOM. + * + * DOMAttributeNames: object mapping React attribute name to the DOM attribute name. + * Attribute names not specified use the **lowercase** normalized name. + * + * DOMPropertyNames: similar to DOMAttributeNames but for DOM properties. Property + * names not specified use the normalized name. + * + * DOMMutationMethods: Properties that require special mutation methods. If `value` is + * undefined, the mutation method should unset the property. + * + * @param {object} domPropertyConfig the config as described above. + */ + injectDOMPropertyConfig: function(domPropertyConfig) { + var Properties = domPropertyConfig.Properties; + var DOMAttributeNames = domPropertyConfig.DOMAttributeNames; + var DOMPropertyNames = domPropertyConfig.DOMPropertyNames; + var DOMMutationMethods = domPropertyConfig.DOMMutationMethods; + + if (domPropertyConfig.isCustomAttribute) { + DOMProperty._isCustomAttributeFunctions.push(domPropertyConfig.isCustomAttribute); + } + + for (var propName in Properties) { + DOMProperty.isStandardName[propName] = true; + + DOMProperty.getAttributeName[propName] = + DOMAttributeNames[propName] || propName.toLowerCase(); + + DOMProperty.getPropertyName[propName] = + DOMPropertyNames[propName] || propName; + + var mutationMethod = DOMMutationMethods[propName]; + if (mutationMethod) { + DOMProperty.getMutationMethod[propName] = mutationMethod; + } + + var propConfig = Properties[propName]; + DOMProperty.mustUseAttribute[propName] = + 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; + + invariant( + !DOMProperty.mustUseAttribute[propName] || + !DOMProperty.mustUseProperty[propName], + 'DOMProperty: Cannot use require using both attribute and property: %s', + propName + ); + invariant( + DOMProperty.mustUseProperty[propName] || + !DOMProperty.hasSideEffects[propName], + 'DOMProperty: Properties that have side effects must use property: %s', + propName + ); + } + } +}; var defaultValueCache = {}; /** @@ -94,13 +175,23 @@ var DOMProperty = { */ hasSideEffects: {}, + /** + * All of the isCustomAttribute() functions that have been injected. + */ + _isCustomAttributeFunctions: [], + /** * Checks whether a property name is a custom attribute. * @method */ - isCustomAttribute: RegExp.prototype.test.bind( - /^(data|aria)-[a-z_][a-z\d_.\-]*$/ - ), + isCustomAttribute: function(attributeName) { + for (var i = 0; i < DOMProperty._isCustomAttributeFunctions.length; i++) { + if (DOMProperty._isCustomAttributeFunctions[i].call(null, attributeName)) { + return true; + } + } + return false; + }, /** * Returns the default property value for a DOM property (i.e., not an @@ -121,168 +212,9 @@ var DOMProperty = { nodeDefaults[prop] = testElement[prop]; } return nodeDefaults[prop]; - } -}; - -/** - * Mapping from normalized, camelcased property names to a configuration that - * specifies how the associated DOM property should be accessed or rendered. - */ -var MustUseAttribute = 0x1; -var MustUseProperty = 0x2; -var HasBooleanValue = 0x4; -var HasSideEffects = 0x8; - -var Properties = { - /** - * Standard Properties - */ - accept: null, - action: null, - ajaxify: MustUseAttribute, - allowFullScreen: MustUseAttribute | HasBooleanValue, - alt: null, - autoComplete: null, - autoplay: HasBooleanValue, - cellPadding: null, - cellSpacing: null, - checked: MustUseProperty | HasBooleanValue, - className: MustUseProperty, - colSpan: null, - contentEditable: null, - controls: MustUseProperty | HasBooleanValue, - data: null, // For `` acts as `src`. - dir: null, - disabled: MustUseProperty | HasBooleanValue, - draggable: null, - enctype: null, - height: MustUseAttribute, - href: null, - htmlFor: null, - max: null, - method: null, - min: null, - multiple: MustUseProperty | HasBooleanValue, - name: null, - poster: null, - preload: null, - placeholder: null, - rel: null, - required: HasBooleanValue, - role: MustUseAttribute, - scrollLeft: MustUseProperty, - scrollTop: MustUseProperty, - selected: MustUseProperty | HasBooleanValue, - spellCheck: null, - src: null, - step: null, - style: null, - tabIndex: null, - target: null, - title: null, - type: null, - value: MustUseProperty | HasSideEffects, - width: MustUseAttribute, - wmode: MustUseAttribute, - /** - * SVG Properties - */ - cx: MustUseProperty, - cy: MustUseProperty, - d: MustUseProperty, - fill: MustUseProperty, - fx: MustUseProperty, - fy: MustUseProperty, - points: MustUseProperty, - r: MustUseProperty, - stroke: MustUseProperty, - strokeLinecap: MustUseProperty, - strokeWidth: MustUseProperty, - transform: MustUseProperty, - x: MustUseProperty, - x1: MustUseProperty, - x2: MustUseProperty, - version: MustUseProperty, - viewBox: MustUseProperty, - y: MustUseProperty, - y1: MustUseProperty, - y2: MustUseProperty, - spreadMethod: MustUseProperty, - offset: MustUseProperty, - stopColor: MustUseProperty, - stopOpacity: MustUseProperty, - gradientUnits: MustUseProperty, - gradientTransform: MustUseProperty -}; - -/** - * Attribute names not specified use the **lowercase** normalized name. - */ -var DOMAttributeNames = { - className: 'class', - htmlFor: 'for', - strokeLinecap: 'stroke-linecap', - strokeWidth: 'stroke-width', - stopColor: 'stop-color', - stopOpacity: 'stop-opacity' -}; - -/** - * Property names not specified use the normalized name. - */ -var DOMPropertyNames = { - autoComplete: 'autocomplete', - spellCheck: 'spellcheck' -}; + }, -/** - * Properties that require special mutation methods. If `value` is undefined, - * the mutation method should unset the property. - */ -var DOMMutationMethods = { - /** - * Setting `className` to null may cause it to be set to the string "null". - * - * @param {DOMElement} node - * @param {*} value - */ - className: function(node, value) { - node.className = value || ''; - } + injection: DOMPropertyInjection }; -for (var propName in Properties) { - DOMProperty.isStandardName[propName] = true; - - DOMProperty.getAttributeName[propName] = - DOMAttributeNames[propName] || propName.toLowerCase(); - - DOMProperty.getPropertyName[propName] = - DOMPropertyNames[propName] || propName; - - var mutationMethod = DOMMutationMethods[propName]; - if (mutationMethod) { - DOMProperty.getMutationMethod[propName] = mutationMethod; - } - - var propConfig = Properties[propName]; - DOMProperty.mustUseAttribute[propName] = propConfig & MustUseAttribute; - DOMProperty.mustUseProperty[propName] = propConfig & MustUseProperty; - DOMProperty.hasBooleanValue[propName] = propConfig & HasBooleanValue; - DOMProperty.hasSideEffects[propName] = propConfig & HasSideEffects; - - invariant( - !DOMProperty.mustUseAttribute[propName] || - !DOMProperty.mustUseProperty[propName], - 'DOMProperty: Cannot use require using both attribute and property: %s', - propName - ); - invariant( - DOMProperty.mustUseProperty[propName] || - !DOMProperty.hasSideEffects[propName], - 'DOMProperty: Properties that have side effects must use property: %s', - propName - ); -} - module.exports = DOMProperty; diff --git a/src/dom/DefaultDOMPropertyConfig.js b/src/dom/DefaultDOMPropertyConfig.js new file mode 100644 index 0000000000000..e427efabd6de9 --- /dev/null +++ b/src/dom/DefaultDOMPropertyConfig.js @@ -0,0 +1,140 @@ +/** + * Copyright 2013 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * @providesModule DefaultDOMPropertyConfig + */ + +"use strict"; + +var DOMProperty = require('DOMProperty'); + +var DefaultDOMPropertyConfig = { + isCustomAttribute: RegExp.prototype.test.bind( + /^(data|aria)-[a-z_][a-z\d_.\-]*$/ + ), + Properties: { + /** + * Standard Properties + */ + accept: null, + action: null, + ajaxify: DOMProperty.injection.MUST_USE_ATTRIBUTE, + allowFullScreen: DOMProperty.injection.MUST_USE_ATTRIBUTE | + DOMProperty.injection.HAS_BOOLEAN_VALUE, + alt: null, + autoComplete: null, + autoplay: DOMProperty.injection.HAS_BOOLEAN_VALUE, + cellPadding: null, + cellSpacing: null, + checked: DOMProperty.injection.MUST_USE_PROPERTY | + DOMProperty.injection.HAS_BOOLEAN_VALUE, + className: DOMProperty.injection.MUST_USE_PROPERTY, + colSpan: null, + contentEditable: null, + controls: DOMProperty.injection.MUST_USE_PROPERTY | + DOMProperty.injection.HAS_BOOLEAN_VALUE, + data: null, // For `` acts as `src`. + dir: null, + disabled: DOMProperty.injection.MUST_USE_PROPERTY | + DOMProperty.injection.HAS_BOOLEAN_VALUE, + draggable: null, + enctype: null, + height: DOMProperty.injection.MUST_USE_ATTRIBUTE, + href: null, + htmlFor: null, + max: null, + method: null, + min: null, + multiple: DOMProperty.injection.MUST_USE_PROPERTY | + DOMProperty.injection.HAS_BOOLEAN_VALUE, + name: null, + poster: null, + preload: null, + placeholder: null, + rel: null, + required: DOMProperty.injection.HAS_BOOLEAN_VALUE, + role: DOMProperty.injection.MUST_USE_ATTRIBUTE, + scrollLeft: DOMProperty.injection.MUST_USE_PROPERTY, + scrollTop: DOMProperty.injection.MUST_USE_PROPERTY, + selected: DOMProperty.injection.MUST_USE_PROPERTY | + DOMProperty.injection.HAS_BOOLEAN_VALUE, + spellCheck: null, + src: null, + step: null, + style: null, + tabIndex: null, + target: null, + title: null, + type: null, + value: DOMProperty.injection.MUST_USE_PROPERTY | + DOMProperty.injection.HAS_SIDE_EFFECTS, + width: DOMProperty.injection.MUST_USE_ATTRIBUTE, + wmode: DOMProperty.injection.MUST_USE_ATTRIBUTE, + /** + * SVG Properties + */ + cx: DOMProperty.injection.MUST_USE_PROPERTY, + cy: DOMProperty.injection.MUST_USE_PROPERTY, + d: DOMProperty.injection.MUST_USE_PROPERTY, + fill: DOMProperty.injection.MUST_USE_PROPERTY, + fx: DOMProperty.injection.MUST_USE_PROPERTY, + fy: DOMProperty.injection.MUST_USE_PROPERTY, + points: DOMProperty.injection.MUST_USE_PROPERTY, + r: DOMProperty.injection.MUST_USE_PROPERTY, + stroke: DOMProperty.injection.MUST_USE_PROPERTY, + strokeLinecap: DOMProperty.injection.MUST_USE_PROPERTY, + strokeWidth: DOMProperty.injection.MUST_USE_PROPERTY, + transform: DOMProperty.injection.MUST_USE_PROPERTY, + x: DOMProperty.injection.MUST_USE_PROPERTY, + x1: DOMProperty.injection.MUST_USE_PROPERTY, + x2: DOMProperty.injection.MUST_USE_PROPERTY, + version: DOMProperty.injection.MUST_USE_PROPERTY, + viewBox: DOMProperty.injection.MUST_USE_PROPERTY, + y: DOMProperty.injection.MUST_USE_PROPERTY, + y1: DOMProperty.injection.MUST_USE_PROPERTY, + y2: DOMProperty.injection.MUST_USE_PROPERTY, + spreadMethod: DOMProperty.injection.MUST_USE_PROPERTY, + offset: DOMProperty.injection.MUST_USE_PROPERTY, + stopColor: DOMProperty.injection.MUST_USE_PROPERTY, + stopOpacity: DOMProperty.injection.MUST_USE_PROPERTY, + gradientUnits: DOMProperty.injection.MUST_USE_PROPERTY, + gradientTransform: DOMProperty.injection.MUST_USE_PROPERTY + }, + DOMAttributeNames: { + className: 'class', + htmlFor: 'for', + strokeLinecap: 'stroke-linecap', + strokeWidth: 'stroke-width', + stopColor: 'stop-color', + stopOpacity: 'stop-opacity' + }, + DOMPropertyNames: { + autoComplete: 'autocomplete', + spellCheck: 'spellcheck' + }, + DOMMutationMethods: { + /** + * Setting `className` to null may cause it to be set to the string "null". + * + * @param {DOMElement} node + * @param {*} value + */ + className: function(node, value) { + node.className = value || ''; + } + } +}; + +module.exports = DefaultDOMPropertyConfig; \ No newline at end of file From 42b5df98d7f1dd8bfc44c7b860e1433d450301b0 Mon Sep 17 00:00:00 2001 From: petehunt Date: Sun, 30 Jun 2013 12:59:51 -0700 Subject: [PATCH 2/6] Expose DOMProperty on React --- src/core/React.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/core/React.js b/src/core/React.js index 7bdb3b39bdf34..6685b0591d443 100644 --- a/src/core/React.js +++ b/src/core/React.js @@ -24,6 +24,7 @@ var ReactDOM = require('ReactDOM'); var ReactMount = require('ReactMount'); var ReactProps = require('ReactProps'); var ReactServerRendering = require('ReactServerRendering'); +var DOMProperty = require('DOMProperty'); var ReactDefaultInjection = require('ReactDefaultInjection'); @@ -31,6 +32,7 @@ ReactDefaultInjection.inject(); var React = { DOM: ReactDOM, + DOMProperty: DOMProperty, Props: ReactProps, initializeTouchEvents: function(shouldUseTouch) { ReactMount.useTouchEvents = shouldUseTouch; From ef96e77e539a4f08373a2c15c8390d7de47a10f5 Mon Sep 17 00:00:00 2001 From: petehunt Date: Sun, 30 Jun 2013 13:10:10 -0700 Subject: [PATCH 3/6] Add test case --- src/dom/DOMProperty.js | 6 +-- .../__tests__/DOMPropertyOperations-test.js | 48 +++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/src/dom/DOMProperty.js b/src/dom/DOMProperty.js index d5f78e13929dd..7c1016c721de3 100644 --- a/src/dom/DOMProperty.js +++ b/src/dom/DOMProperty.js @@ -57,9 +57,9 @@ var DOMPropertyInjection = { */ injectDOMPropertyConfig: function(domPropertyConfig) { var Properties = domPropertyConfig.Properties; - var DOMAttributeNames = domPropertyConfig.DOMAttributeNames; - var DOMPropertyNames = domPropertyConfig.DOMPropertyNames; - var DOMMutationMethods = domPropertyConfig.DOMMutationMethods; + var DOMAttributeNames = domPropertyConfig.DOMAttributeNames || {}; + var DOMPropertyNames = domPropertyConfig.DOMPropertyNames || {}; + var DOMMutationMethods = domPropertyConfig.DOMMutationMethods || {}; if (domPropertyConfig.isCustomAttribute) { DOMProperty._isCustomAttributeFunctions.push(domPropertyConfig.isCustomAttribute); diff --git a/src/dom/__tests__/DOMPropertyOperations-test.js b/src/dom/__tests__/DOMPropertyOperations-test.js index 1091da09c069d..5768fe29a0cf6 100644 --- a/src/dom/__tests__/DOMPropertyOperations-test.js +++ b/src/dom/__tests__/DOMPropertyOperations-test.js @@ -21,10 +21,12 @@ describe('DOMPropertyOperations', function() { var DOMPropertyOperations; + var DOMProperty; beforeEach(function() { require('mock-modules').dumpCache(); DOMPropertyOperations = require('DOMPropertyOperations'); + DOMProperty = require('DOMProperty'); }); describe('createMarkupForProperty', function() { @@ -126,4 +128,50 @@ describe('DOMPropertyOperations', function() { }); }); + + describe('injectDOMPropertyConfig', function() { + it('should support custom attributes', function() { + // foobar does not exist yet + expect(DOMPropertyOperations.createMarkupForProperty( + 'foobar', + 'simple' + )).toBe(null); + + // foo-* does not exist yet + expect(DOMPropertyOperations.createMarkupForProperty( + 'foo-xyz', + 'simple' + )).toBe(null); + + // inject foobar DOM property + DOMProperty.injection.injectDOMPropertyConfig({ + isCustomAttribute: function(name) { + return name.indexOf('foo-') === 0; + }, + Properties: {foobar: null} + }); + + // Ensure old attributes still work + expect(DOMPropertyOperations.createMarkupForProperty( + 'name', + 'simple' + )).toBe('name="simple"'); + expect(DOMPropertyOperations.createMarkupForProperty( + 'data-name', + 'simple' + )).toBe('data-name="simple"'); + + // foobar should work + expect(DOMPropertyOperations.createMarkupForProperty( + 'foobar', + 'simple' + )).toBe('foobar="simple"'); + + // foo-* should work + expect(DOMPropertyOperations.createMarkupForProperty( + 'foo-xyz', + 'simple' + )).toBe('foo-xyz="simple"'); + }); + }); }); From eeecd5424bbc92d93646775b42b32ba3ad83474e Mon Sep 17 00:00:00 2001 From: petehunt Date: Sun, 30 Jun 2013 13:20:01 -0700 Subject: [PATCH 4/6] now we can use the id attribute --- src/dom/DOMProperty.js | 2 +- src/dom/DefaultDOMPropertyConfig.js | 1 + src/dom/__tests__/DOMPropertyOperations-test.js | 7 +++++++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/dom/DOMProperty.js b/src/dom/DOMProperty.js index 7c1016c721de3..992a86118c910 100644 --- a/src/dom/DOMProperty.js +++ b/src/dom/DOMProperty.js @@ -56,7 +56,7 @@ var DOMPropertyInjection = { * @param {object} domPropertyConfig the config as described above. */ injectDOMPropertyConfig: function(domPropertyConfig) { - var Properties = domPropertyConfig.Properties; + var Properties = domPropertyConfig.Properties || {}; var DOMAttributeNames = domPropertyConfig.DOMAttributeNames || {}; var DOMPropertyNames = domPropertyConfig.DOMPropertyNames || {}; var DOMMutationMethods = domPropertyConfig.DOMMutationMethods || {}; diff --git a/src/dom/DefaultDOMPropertyConfig.js b/src/dom/DefaultDOMPropertyConfig.js index e427efabd6de9..079e2c8903419 100644 --- a/src/dom/DefaultDOMPropertyConfig.js +++ b/src/dom/DefaultDOMPropertyConfig.js @@ -54,6 +54,7 @@ var DefaultDOMPropertyConfig = { height: DOMProperty.injection.MUST_USE_ATTRIBUTE, href: null, htmlFor: null, + id: DOMProperty.injection.MUST_USE_PROPERTY, max: null, method: null, min: null, diff --git a/src/dom/__tests__/DOMPropertyOperations-test.js b/src/dom/__tests__/DOMPropertyOperations-test.js index 5768fe29a0cf6..86d0411a47ac3 100644 --- a/src/dom/__tests__/DOMPropertyOperations-test.js +++ b/src/dom/__tests__/DOMPropertyOperations-test.js @@ -48,6 +48,13 @@ describe('DOMPropertyOperations', function() { )).toBe(''); }); + it('should work with the id attribute', function() { + expect(DOMPropertyOperations.createMarkupForProperty( + 'id', + 'simple' + )).toBe('id="simple"'); + }); + it('should create markup for boolean properties', function() { expect(DOMPropertyOperations.createMarkupForProperty( 'checked', From 7d465156113a88d9e0b8f069fdcb889be57b2c5c Mon Sep 17 00:00:00 2001 From: petehunt Date: Mon, 1 Jul 2013 10:21:12 -0700 Subject: [PATCH 5/6] use lcocals in DefaultDOMPropertyConfig --- src/dom/DefaultDOMPropertyConfig.js | 100 ++++++++++++++-------------- 1 file changed, 49 insertions(+), 51 deletions(-) diff --git a/src/dom/DefaultDOMPropertyConfig.js b/src/dom/DefaultDOMPropertyConfig.js index 079e2c8903419..b53ff8fceea90 100644 --- a/src/dom/DefaultDOMPropertyConfig.js +++ b/src/dom/DefaultDOMPropertyConfig.js @@ -20,6 +20,11 @@ var DOMProperty = require('DOMProperty'); +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 DefaultDOMPropertyConfig = { isCustomAttribute: RegExp.prototype.test.bind( /^(data|aria)-[a-z_][a-z\d_.\-]*$/ @@ -30,47 +35,41 @@ var DefaultDOMPropertyConfig = { */ accept: null, action: null, - ajaxify: DOMProperty.injection.MUST_USE_ATTRIBUTE, - allowFullScreen: DOMProperty.injection.MUST_USE_ATTRIBUTE | - DOMProperty.injection.HAS_BOOLEAN_VALUE, + ajaxify: MUST_USE_ATTRIBUTE, + allowFullScreen: MUST_USE_ATTRIBUTE | HAS_BOOLEAN_VALUE, alt: null, autoComplete: null, - autoplay: DOMProperty.injection.HAS_BOOLEAN_VALUE, + autoplay: HAS_BOOLEAN_VALUE, cellPadding: null, cellSpacing: null, - checked: DOMProperty.injection.MUST_USE_PROPERTY | - DOMProperty.injection.HAS_BOOLEAN_VALUE, - className: DOMProperty.injection.MUST_USE_PROPERTY, + checked: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE, + className: MUST_USE_PROPERTY, colSpan: null, contentEditable: null, - controls: DOMProperty.injection.MUST_USE_PROPERTY | - DOMProperty.injection.HAS_BOOLEAN_VALUE, + controls: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE, data: null, // For `` acts as `src`. dir: null, - disabled: DOMProperty.injection.MUST_USE_PROPERTY | - DOMProperty.injection.HAS_BOOLEAN_VALUE, + disabled: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE, draggable: null, enctype: null, - height: DOMProperty.injection.MUST_USE_ATTRIBUTE, + height: MUST_USE_ATTRIBUTE, href: null, htmlFor: null, - id: DOMProperty.injection.MUST_USE_PROPERTY, + id: MUST_USE_PROPERTY, max: null, method: null, min: null, - multiple: DOMProperty.injection.MUST_USE_PROPERTY | - DOMProperty.injection.HAS_BOOLEAN_VALUE, + multiple: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE, name: null, poster: null, preload: null, placeholder: null, rel: null, - required: DOMProperty.injection.HAS_BOOLEAN_VALUE, - role: DOMProperty.injection.MUST_USE_ATTRIBUTE, - scrollLeft: DOMProperty.injection.MUST_USE_PROPERTY, - scrollTop: DOMProperty.injection.MUST_USE_PROPERTY, - selected: DOMProperty.injection.MUST_USE_PROPERTY | - DOMProperty.injection.HAS_BOOLEAN_VALUE, + required: HAS_BOOLEAN_VALUE, + role: MUST_USE_ATTRIBUTE, + scrollLeft: MUST_USE_PROPERTY, + scrollTop: MUST_USE_PROPERTY, + selected: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE, spellCheck: null, src: null, step: null, @@ -79,39 +78,38 @@ var DefaultDOMPropertyConfig = { target: null, title: null, type: null, - value: DOMProperty.injection.MUST_USE_PROPERTY | - DOMProperty.injection.HAS_SIDE_EFFECTS, - width: DOMProperty.injection.MUST_USE_ATTRIBUTE, - wmode: DOMProperty.injection.MUST_USE_ATTRIBUTE, + value: MUST_USE_PROPERTY | HAS_SIDE_EFFECTS, + width: MUST_USE_ATTRIBUTE, + wmode: MUST_USE_ATTRIBUTE, /** * SVG Properties */ - cx: DOMProperty.injection.MUST_USE_PROPERTY, - cy: DOMProperty.injection.MUST_USE_PROPERTY, - d: DOMProperty.injection.MUST_USE_PROPERTY, - fill: DOMProperty.injection.MUST_USE_PROPERTY, - fx: DOMProperty.injection.MUST_USE_PROPERTY, - fy: DOMProperty.injection.MUST_USE_PROPERTY, - points: DOMProperty.injection.MUST_USE_PROPERTY, - r: DOMProperty.injection.MUST_USE_PROPERTY, - stroke: DOMProperty.injection.MUST_USE_PROPERTY, - strokeLinecap: DOMProperty.injection.MUST_USE_PROPERTY, - strokeWidth: DOMProperty.injection.MUST_USE_PROPERTY, - transform: DOMProperty.injection.MUST_USE_PROPERTY, - x: DOMProperty.injection.MUST_USE_PROPERTY, - x1: DOMProperty.injection.MUST_USE_PROPERTY, - x2: DOMProperty.injection.MUST_USE_PROPERTY, - version: DOMProperty.injection.MUST_USE_PROPERTY, - viewBox: DOMProperty.injection.MUST_USE_PROPERTY, - y: DOMProperty.injection.MUST_USE_PROPERTY, - y1: DOMProperty.injection.MUST_USE_PROPERTY, - y2: DOMProperty.injection.MUST_USE_PROPERTY, - spreadMethod: DOMProperty.injection.MUST_USE_PROPERTY, - offset: DOMProperty.injection.MUST_USE_PROPERTY, - stopColor: DOMProperty.injection.MUST_USE_PROPERTY, - stopOpacity: DOMProperty.injection.MUST_USE_PROPERTY, - gradientUnits: DOMProperty.injection.MUST_USE_PROPERTY, - gradientTransform: DOMProperty.injection.MUST_USE_PROPERTY + cx: MUST_USE_PROPERTY, + cy: MUST_USE_PROPERTY, + d: MUST_USE_PROPERTY, + fill: MUST_USE_PROPERTY, + fx: MUST_USE_PROPERTY, + fy: MUST_USE_PROPERTY, + points: MUST_USE_PROPERTY, + r: MUST_USE_PROPERTY, + stroke: MUST_USE_PROPERTY, + strokeLinecap: MUST_USE_PROPERTY, + strokeWidth: MUST_USE_PROPERTY, + transform: MUST_USE_PROPERTY, + x: MUST_USE_PROPERTY, + x1: MUST_USE_PROPERTY, + x2: MUST_USE_PROPERTY, + version: MUST_USE_PROPERTY, + viewBox: MUST_USE_PROPERTY, + y: MUST_USE_PROPERTY, + y1: MUST_USE_PROPERTY, + y2: MUST_USE_PROPERTY, + spreadMethod: MUST_USE_PROPERTY, + offset: MUST_USE_PROPERTY, + stopColor: MUST_USE_PROPERTY, + stopOpacity: MUST_USE_PROPERTY, + gradientUnits: MUST_USE_PROPERTY, + gradientTransform: MUST_USE_PROPERTY }, DOMAttributeNames: { className: 'class', From ab6e94216bd4dfe5dbcb16a5063de97faa9c301d Mon Sep 17 00:00:00 2001 From: petehunt Date: Mon, 1 Jul 2013 14:40:33 -0700 Subject: [PATCH 6/6] invariant() on double injection --- src/dom/DOMProperty.js | 9 +++++++++ src/dom/__tests__/DOMPropertyOperations-test.js | 5 +++++ 2 files changed, 14 insertions(+) diff --git a/src/dom/DOMProperty.js b/src/dom/DOMProperty.js index 992a86118c910..551e5534a69b1 100644 --- a/src/dom/DOMProperty.js +++ b/src/dom/DOMProperty.js @@ -66,6 +66,15 @@ var DOMPropertyInjection = { } for (var propName in Properties) { + invariant( + !DOMProperty.isStandardName[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 injecting two configs that have conflicting ' + + 'property names.', + propName + ); + DOMProperty.isStandardName[propName] = true; DOMProperty.getAttributeName[propName] = diff --git a/src/dom/__tests__/DOMPropertyOperations-test.js b/src/dom/__tests__/DOMPropertyOperations-test.js index 86d0411a47ac3..1588a3d827548 100644 --- a/src/dom/__tests__/DOMPropertyOperations-test.js +++ b/src/dom/__tests__/DOMPropertyOperations-test.js @@ -179,6 +179,11 @@ describe('DOMPropertyOperations', function() { 'foo-xyz', 'simple' )).toBe('foo-xyz="simple"'); + + // It should complain about double injections. + expect(function() { + DOMProperty.injection.injectDOMPropertyConfig({Properties: {foobar: null}}); + }).toThrow(); }); }); });