From 1ac4f7ec66e932c863d0fa2243cda7fb7540b87d Mon Sep 17 00:00:00 2001 From: Pete Hunt Date: Tue, 2 Jul 2013 18:26:59 -0700 Subject: [PATCH] Injectable DOMProperty configs, and add back ID attribute https://github.com/facebook/react/pull/141 --- src/core/ReactDefaultInjection.js | 13 +- .../__tests__/ReactNativeComponent-test.js | 5 +- src/dom/DOMProperty.js | 273 +++++++----------- src/dom/DefaultDOMPropertyConfig.js | 139 +++++++++ .../__tests__/DOMPropertyOperations-test.js | 65 +++++ .../__tests__/AnalyticsEventPlugin-test.js | 25 +- 6 files changed, 345 insertions(+), 175 deletions(-) create mode 100644 src/dom/DefaultDOMPropertyConfig.js diff --git a/src/core/ReactDefaultInjection.js b/src/core/ReactDefaultInjection.js index 3357722..e03d107 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'); @@ -37,7 +40,7 @@ function inject() { EventPluginHub.injection.injectInstanceHandle(ReactInstanceHandles); /** - * Two important event plugins included by default (without having to require + * Some important event plugins included by default (without having to require * them). */ EventPluginHub.injection.injectEventPluginsByName({ @@ -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/core/__tests__/ReactNativeComponent-test.js b/src/core/__tests__/ReactNativeComponent-test.js index 9b6ab66..b746232 100644 --- a/src/core/__tests__/ReactNativeComponent-test.js +++ b/src/core/__tests__/ReactNativeComponent-test.js @@ -193,6 +193,9 @@ describe('ReactNativeComponent', function() { beforeEach(function() { require('mock-modules').dumpCache(); + var ReactDefaultInjection = require('ReactDefaultInjection'); + ReactDefaultInjection.inject(); + var mixInto = require('mixInto'); var ReactNativeComponent = require('ReactNativeComponent'); @@ -217,7 +220,7 @@ describe('ReactNativeComponent', function() { }); }); - it("should handle className", function() { + it("should generate the correct markup with className", function() { expect(genMarkup({ className: 'a' })).toHaveAttribute('class', 'a'); expect(genMarkup({ className: 'a b' })).toHaveAttribute('class', 'a b'); expect(genMarkup({ className: '' })).toHaveAttribute('class', ''); diff --git a/src/dom/DOMProperty.js b/src/dom/DOMProperty.js index 0d76303..1bded21 100644 --- a/src/dom/DOMProperty.js +++ b/src/dom/DOMProperty.js @@ -23,6 +23,101 @@ 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) { + 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] = + 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 +189,22 @@ 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) { + return DOMProperty._isCustomAttributeFunctions.some( + function(isCustomAttributeFn) { + return isCustomAttributeFn.call(null, attributeName); + } + ); + }, /** * Returns the default property value for a DOM property (i.e., not an @@ -121,168 +225,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 + injection: DOMPropertyInjection }; -/** - * 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 || ''; - } -}; - -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 0000000..f33c862 --- /dev/null +++ b/src/dom/DefaultDOMPropertyConfig.js @@ -0,0 +1,139 @@ +/** + * 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 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_.\-]*$/ + ), + Properties: { + /** + * Standard Properties + */ + accept: null, + action: null, + ajaxify: MUST_USE_ATTRIBUTE, + allowFullScreen: MUST_USE_ATTRIBUTE | HAS_BOOLEAN_VALUE, + alt: null, + autoComplete: null, + autoplay: HAS_BOOLEAN_VALUE, + cellPadding: null, + cellSpacing: null, + checked: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE, + className: MUST_USE_PROPERTY, + colSpan: null, + contentEditable: null, + controls: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE, + data: null, // For `` acts as `src`. + dir: null, + disabled: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE, + draggable: null, + enctype: null, + height: MUST_USE_ATTRIBUTE, + href: null, + htmlFor: null, + id: MUST_USE_PROPERTY, + max: null, + method: null, + min: null, + multiple: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE, + name: null, + poster: null, + preload: null, + placeholder: null, + rel: null, + 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, + style: null, + tabIndex: null, + target: null, + title: null, + type: null, + value: MUST_USE_PROPERTY | HAS_SIDE_EFFECTS, + width: MUST_USE_ATTRIBUTE, + wmode: MUST_USE_ATTRIBUTE, + /** + * SVG Properties + */ + 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', + 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; diff --git a/src/dom/__tests__/DOMPropertyOperations-test.js b/src/dom/__tests__/DOMPropertyOperations-test.js index 1091da0..acc10d6 100644 --- a/src/dom/__tests__/DOMPropertyOperations-test.js +++ b/src/dom/__tests__/DOMPropertyOperations-test.js @@ -21,10 +21,15 @@ describe('DOMPropertyOperations', function() { var DOMPropertyOperations; + var DOMProperty; beforeEach(function() { require('mock-modules').dumpCache(); + var ReactDefaultInjection = require('ReactDefaultInjection'); + ReactDefaultInjection.inject(); + DOMPropertyOperations = require('DOMPropertyOperations'); + DOMProperty = require('DOMProperty'); }); describe('createMarkupForProperty', function() { @@ -46,6 +51,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', @@ -126,4 +138,57 @@ 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"'); + + // It should complain about double injections. + expect(function() { + DOMProperty.injection.injectDOMPropertyConfig( + {Properties: {foobar: null}} + ); + }).toThrow(); + }); + }); }); diff --git a/src/eventPlugins/__tests__/AnalyticsEventPlugin-test.js b/src/eventPlugins/__tests__/AnalyticsEventPlugin-test.js index 3e54875..fb6dbf0 100644 --- a/src/eventPlugins/__tests__/AnalyticsEventPlugin-test.js +++ b/src/eventPlugins/__tests__/AnalyticsEventPlugin-test.js @@ -26,23 +26,42 @@ describe('AnalyticsEventPlugin', function() { var EventPluginHub; var EventPluginRegistry; var React; - var ReactDefaultInjection; var ReactEventEmitter; var ReactEventTopLevelCallback; var ReactTestUtils; + var DefaultEventPluginOrder; + var EnterLeaveEventPlugin; + var ChangeEventPlugin; + var ReactInstanceHandles; + var SimpleEventPlugin; + beforeEach(function() { AnalyticsEventPluginFactory = require('AnalyticsEventPluginFactory'); EventPluginHub = require('EventPluginHub'); EventPluginRegistry = require('EventPluginRegistry'); React = require('React'); - ReactDefaultInjection = require('ReactDefaultInjection'); ReactEventEmitter = require('ReactEventEmitter'); ReactEventTopLevelCallback = require('ReactEventTopLevelCallback'); ReactTestUtils = require('ReactTestUtils'); EventPluginRegistry._resetEventPlugins(); - ReactDefaultInjection.inject(); + + // Re-inject default events system after resetting. + DefaultEventPluginOrder = require('DefaultEventPluginOrder'); + EnterLeaveEventPlugin = require('EnterLeaveEventPlugin'); + ChangeEventPlugin = require('ChangeEventPlugin'); + ReactInstanceHandles = require('ReactInstanceHandles'); + SimpleEventPlugin = require('SimpleEventPlugin'); + + EventPluginHub.injection.injectEventPluginOrder(DefaultEventPluginOrder); + EventPluginHub.injection.injectInstanceHandle(ReactInstanceHandles); + + EventPluginHub.injection.injectEventPluginsByName({ + 'SimpleEventPlugin': SimpleEventPlugin, + 'EnterLeaveEventPlugin': EnterLeaveEventPlugin, + 'ChangeEventPlugin': ChangeEventPlugin + }); ReactEventEmitter.ensureListening(false, ReactEventTopLevelCallback); });