From 232a47ad0493fa2664f3205986dbc73ac5061bff Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 23 Dec 2015 17:34:27 +0000 Subject: [PATCH 1/4] Pass SVG attributes through All attributes defined on SVG elements will now be passed directly regardless of the whitelist. The casing specified by user will be preserved, and setAttribute() will be used. In the future we will remove support for the camel case aliases to the hyphenated attributes. For example, we currently map `strokeWidth` to `stroke-width` but this is now deprecated behind a warning. When we remove support for this we can remove some of the code paths introduced in this commit. The purpose of this change is to stop maintaining a separate SVG property config. The config still exists for two purposes: * Allow a migration path for deprecated camelcased versions of hyphenated SVG attributes * Track special namespaced attributes (they still require a whitelist) However it is no longer a blocker for using new non-namespaced SVG attributes, and users don't have to ask us to add them to the whitelist. Fixes #1657 --- .../dom/client/__tests__/ReactDOMSVG-test.js | 39 +++++ .../dom/shared/DOMPropertyOperations.js | 77 +++++++++ src/renderers/dom/shared/ReactDOMComponent.js | 13 ++ .../dom/shared/SVGDOMPropertyConfig.js | 37 ---- .../__tests__/ReactDOMComponent-test.js | 162 ++++++++++++++++++ 5 files changed, 291 insertions(+), 37 deletions(-) diff --git a/src/renderers/dom/client/__tests__/ReactDOMSVG-test.js b/src/renderers/dom/client/__tests__/ReactDOMSVG-test.js index 2898d2c2e47e3..b932b136721f2 100644 --- a/src/renderers/dom/client/__tests__/ReactDOMSVG-test.js +++ b/src/renderers/dom/client/__tests__/ReactDOMSVG-test.js @@ -21,6 +21,45 @@ describe('ReactDOMSVG', function() { ReactDOMServer = require('ReactDOMServer'); }); + it('creates initial markup for known hyphenated attributes', function() { + var markup = ReactDOMServer.renderToString( + + ); + expect(markup).toContain('clip-path="url(#starlet)"'); + }); + + it('creates initial markup for camel case attributes', function() { + var markup = ReactDOMServer.renderToString( + + ); + expect(markup).toContain('viewBox="0 0 100 100"'); + }); + + it('deprecates camel casing of hyphenated attributes', function() { + spyOn(console, 'error'); + var markup = ReactDOMServer.renderToString( + + ); + expect(markup).toContain('clip-path="url(#starlet)"'); + expect(console.error.argsForCall.length).toBe(1); + expect(console.error.argsForCall[0][0]).toContain('clipPath'); + expect(console.error.argsForCall[0][0]).toContain('clip-path'); + }); + + it('creates initial markup for unknown hyphenated attributes', function() { + var markup = ReactDOMServer.renderToString( + + ); + expect(markup).toContain('the-word="the-bird"'); + }); + + it('creates initial markup for unknown camel case attributes', function() { + var markup = ReactDOMServer.renderToString( + + ); + expect(markup).toContain('theWord="theBird"'); + }); + it('creates initial namespaced markup', function() { var markup = ReactDOMServer.renderToString( diff --git a/src/renderers/dom/shared/DOMPropertyOperations.js b/src/renderers/dom/shared/DOMPropertyOperations.js index 5375edc23a488..0fdb7e0480774 100644 --- a/src/renderers/dom/shared/DOMPropertyOperations.js +++ b/src/renderers/dom/shared/DOMPropertyOperations.js @@ -50,6 +50,32 @@ function shouldIgnoreValue(propertyInfo, value) { (propertyInfo.hasOverloadedBooleanValue && value === false); } +if (__DEV__) { + var reactProps = { + children: true, + dangerouslySetInnerHTML: true, + key: true, + ref: true, + }; + var warnedSVGProperties = {}; + + var warnDeprecatedSVGProperty = function(name, attributeName) { + if (reactProps.hasOwnProperty(name) && reactProps[name] || + warnedSVGProperties.hasOwnProperty(name) && warnedSVGProperties[name]) { + return; + } + + warnedSVGProperties[name] = true; + warning( + false, + 'SVG property %s is deprecated. Use the original attribute name ' + + '%s for SVG tags instead.', + name, + attributeName + ); + }; +} + /** * Operations for dealing with DOM properties. */ @@ -124,6 +150,31 @@ var DOMPropertyOperations = { return name + '=' + quoteAttributeValueForBrowser(value); }, + /** + * Creates markup for an SVG property. + * + * @param {string} name + * @param {*} value + * @return {string} Markup string, or empty string if the property was invalid. + */ + createMarkupForSVGAttribute: function(name, value) { + if (!isAttributeNameSafe(name) || value == null) { + return ''; + } + var propertyInfo = DOMProperty.properties.hasOwnProperty(name) ? + DOMProperty.properties[name] : null; + if (propertyInfo) { + var { attributeName, attributeNamespace } = propertyInfo; + if (__DEV__) { + if (!attributeNamespace && name !== attributeName) { + warnDeprecatedSVGProperty(name, attributeName); + } + } + return attributeName + '=' + quoteAttributeValueForBrowser(value); + } + return name + '=' + quoteAttributeValueForBrowser(value); + }, + /** * Sets the value for a property on a node. * @@ -183,6 +234,22 @@ var DOMPropertyOperations = { } }, + setValueForSVGAttribute: function(node, name, value) { + var propertyInfo = DOMProperty.properties.hasOwnProperty(name) ? + DOMProperty.properties[name] : null; + if (propertyInfo) { + if (__DEV__) { + var { attributeName, attributeNamespace } = propertyInfo; + if (!attributeNamespace && name !== attributeName) { + warnDeprecatedSVGProperty(name, attributeName); + } + } + DOMPropertyOperations.setValueForProperty(node, name, value); + } else { + DOMPropertyOperations.setValueForAttribute(node, name, value); + } + }, + /** * Deletes the value for a property on a node. * @@ -217,6 +284,16 @@ var DOMPropertyOperations = { } }, + deleteValueForSVGAttribute: function(node, name) { + var propertyInfo = DOMProperty.properties.hasOwnProperty(name) ? + DOMProperty.properties[name] : null; + if (propertyInfo) { + DOMPropertyOperations.deleteValueForProperty(node, name); + } else { + node.removeAttribute(name); + } + }, + }; ReactPerf.measureMethods(DOMPropertyOperations, 'DOMPropertyOperations', { diff --git a/src/renderers/dom/shared/ReactDOMComponent.js b/src/renderers/dom/shared/ReactDOMComponent.js index 65b76b892b7cf..c3202db6ac0ee 100644 --- a/src/renderers/dom/shared/ReactDOMComponent.js +++ b/src/renderers/dom/shared/ReactDOMComponent.js @@ -647,6 +647,8 @@ ReactDOMComponent.Mixin = { if (propKey !== CHILDREN) { markup = DOMPropertyOperations.createMarkupForCustomAttribute(propKey, propValue); } + } else if (this._namespaceURI === DOMNamespaces.svg) { + markup = DOMPropertyOperations.createMarkupForSVGAttribute(propKey, propValue); } else { markup = DOMPropertyOperations.createMarkupForProperty(propKey, propValue); } @@ -862,6 +864,11 @@ ReactDOMComponent.Mixin = { DOMProperty.properties[propKey] || DOMProperty.isCustomAttribute(propKey)) { DOMPropertyOperations.deleteValueForProperty(getNode(this), propKey); + } else if (this._namespaceURI === DOMNamespaces.svg) { + DOMPropertyOperations.deleteValueForSVGAttribute( + getNode(this), + propKey + ); } } for (propKey in nextProps) { @@ -924,6 +931,12 @@ ReactDOMComponent.Mixin = { propKey, nextProp ); + } else if (this._namespaceURI === DOMNamespaces.svg) { + DOMPropertyOperations.setValueForSVGAttribute( + getNode(this), + propKey, + nextProp + ); } else if ( DOMProperty.properties[propKey] || DOMProperty.isCustomAttribute(propKey)) { diff --git a/src/renderers/dom/shared/SVGDOMPropertyConfig.js b/src/renderers/dom/shared/SVGDOMPropertyConfig.js index 8dad8b1e60790..d1299791a2281 100644 --- a/src/renderers/dom/shared/SVGDOMPropertyConfig.js +++ b/src/renderers/dom/shared/SVGDOMPropertyConfig.js @@ -23,46 +23,19 @@ var NS = { var SVGDOMPropertyConfig = { Properties: { clipPath: MUST_USE_ATTRIBUTE, - cx: MUST_USE_ATTRIBUTE, - cy: MUST_USE_ATTRIBUTE, - d: MUST_USE_ATTRIBUTE, - dx: MUST_USE_ATTRIBUTE, - dy: MUST_USE_ATTRIBUTE, - fill: MUST_USE_ATTRIBUTE, fillOpacity: MUST_USE_ATTRIBUTE, fontFamily: MUST_USE_ATTRIBUTE, fontSize: MUST_USE_ATTRIBUTE, - fx: MUST_USE_ATTRIBUTE, - fy: MUST_USE_ATTRIBUTE, - gradientTransform: MUST_USE_ATTRIBUTE, - gradientUnits: MUST_USE_ATTRIBUTE, markerEnd: MUST_USE_ATTRIBUTE, markerMid: MUST_USE_ATTRIBUTE, markerStart: MUST_USE_ATTRIBUTE, - offset: MUST_USE_ATTRIBUTE, - opacity: MUST_USE_ATTRIBUTE, - patternContentUnits: MUST_USE_ATTRIBUTE, - patternUnits: MUST_USE_ATTRIBUTE, - points: MUST_USE_ATTRIBUTE, - preserveAspectRatio: MUST_USE_ATTRIBUTE, - r: MUST_USE_ATTRIBUTE, - rx: MUST_USE_ATTRIBUTE, - ry: MUST_USE_ATTRIBUTE, - spreadMethod: MUST_USE_ATTRIBUTE, stopColor: MUST_USE_ATTRIBUTE, stopOpacity: MUST_USE_ATTRIBUTE, - stroke: MUST_USE_ATTRIBUTE, strokeDasharray: MUST_USE_ATTRIBUTE, strokeLinecap: MUST_USE_ATTRIBUTE, strokeOpacity: MUST_USE_ATTRIBUTE, strokeWidth: MUST_USE_ATTRIBUTE, textAnchor: MUST_USE_ATTRIBUTE, - transform: MUST_USE_ATTRIBUTE, - version: MUST_USE_ATTRIBUTE, - viewBox: MUST_USE_ATTRIBUTE, - x1: MUST_USE_ATTRIBUTE, - x2: MUST_USE_ATTRIBUTE, - x: MUST_USE_ATTRIBUTE, xlinkActuate: MUST_USE_ATTRIBUTE, xlinkArcrole: MUST_USE_ATTRIBUTE, xlinkHref: MUST_USE_ATTRIBUTE, @@ -73,9 +46,6 @@ var SVGDOMPropertyConfig = { xmlBase: MUST_USE_ATTRIBUTE, xmlLang: MUST_USE_ATTRIBUTE, xmlSpace: MUST_USE_ATTRIBUTE, - y1: MUST_USE_ATTRIBUTE, - y2: MUST_USE_ATTRIBUTE, - y: MUST_USE_ATTRIBUTE, }, DOMAttributeNamespaces: { xlinkActuate: NS.xlink, @@ -94,15 +64,9 @@ var SVGDOMPropertyConfig = { fillOpacity: 'fill-opacity', fontFamily: 'font-family', fontSize: 'font-size', - gradientTransform: 'gradientTransform', - gradientUnits: 'gradientUnits', markerEnd: 'marker-end', markerMid: 'marker-mid', markerStart: 'marker-start', - patternContentUnits: 'patternContentUnits', - patternUnits: 'patternUnits', - preserveAspectRatio: 'preserveAspectRatio', - spreadMethod: 'spreadMethod', stopColor: 'stop-color', stopOpacity: 'stop-opacity', strokeDasharray: 'stroke-dasharray', @@ -110,7 +74,6 @@ var SVGDOMPropertyConfig = { strokeOpacity: 'stroke-opacity', strokeWidth: 'stroke-width', textAnchor: 'text-anchor', - viewBox: 'viewBox', xlinkActuate: 'xlink:actuate', xlinkArcrole: 'xlink:arcrole', xlinkHref: 'xlink:href', diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js index 1e920255932b0..368c2ba38cbb0 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js @@ -260,6 +260,75 @@ describe('ReactDOMComponent', function() { expect(container.firstChild.hasAttribute('height')).toBe(false); }); + it('should remove known SVG camel case attributes', function() { + var container = document.createElement('div'); + ReactDOM.render(, container); + + expect(container.firstChild.hasAttribute('viewBox')).toBe(true); + ReactDOM.render(, container); + expect(container.firstChild.hasAttribute('viewBox')).toBe(false); + }); + + it('should remove known SVG hyphenated attributes', function() { + var container = document.createElement('div'); + ReactDOM.render(, container); + + expect(container.firstChild.hasAttribute('clip-path')).toBe(true); + ReactDOM.render(, container); + expect(container.firstChild.hasAttribute('clip-path')).toBe(false); + }); + + it('should remove arbitrary SVG hyphenated attributes', function() { + var container = document.createElement('div'); + ReactDOM.render(, container); + + expect(container.firstChild.hasAttribute('the-word')).toBe(true); + ReactDOM.render(, container); + expect(container.firstChild.hasAttribute('the-word')).toBe(false); + }); + + it('should remove arbitrary SVG camel case attributes', function() { + var container = document.createElement('div'); + ReactDOM.render(, container); + + expect(container.firstChild.hasAttribute('theWord')).toBe(true); + ReactDOM.render(, container); + expect(container.firstChild.hasAttribute('theWord')).toBe(false); + }); + + it('should remove SVG attributes that should have been hyphenated', function() { + spyOn(console, 'error'); + var container = document.createElement('div'); + ReactDOM.render(, container); + expect(console.error.argsForCall.length).toBe(1); + expect(console.error.argsForCall[0][0]).toContain('clipPath'); + expect(console.error.argsForCall[0][0]).toContain('clip-path'); + + expect(container.firstChild.hasAttribute('clip-path')).toBe(true); + ReactDOM.render(, container); + expect(container.firstChild.hasAttribute('clip-path')).toBe(false); + }); + + it('should remove namespaced SVG attributes', function() { + var container = document.createElement('div'); + ReactDOM.render( + + + , + container + ); + + expect(container.firstChild.firstChild.hasAttributeNS( + 'http://www.w3.org/1999/xlink', + 'href' + )).toBe(true); + ReactDOM.render(, container); + expect(container.firstChild.firstChild.hasAttributeNS( + 'http://www.w3.org/1999/xlink', + 'href' + )).toBe(false); + }); + it('should remove properties', function() { var container = document.createElement('div'); ReactDOM.render(
, container); @@ -331,6 +400,99 @@ describe('ReactDOMComponent', function() { expect(container.childNodes[0].getAttribute('myattr')).toBe('myval'); }); + it('should update known hyphenated attributes for SVG tags', function() { + var container = document.createElement('div'); + + var beforeUpdate = React.createElement('svg', {}, null); + ReactDOM.render(beforeUpdate, container); + + var afterUpdate = ; + ReactDOM.render(afterUpdate, container); + + expect(container.childNodes[0].getAttribute('clip-path')).toBe( + 'url(#starlet)' + ); + }); + + it('should update camel case attributes for SVG tags', function() { + var container = document.createElement('div'); + + var beforeUpdate = React.createElement('svg', {}, null); + ReactDOM.render(beforeUpdate, container); + + var afterUpdate = ; + ReactDOM.render(afterUpdate, container); + + expect(container.childNodes[0].getAttribute('viewBox')).toBe( + '0 0 100 100' + ); + }); + + it('should warn camel casing hyphenated attributes for SVG tags', function() { + spyOn(console, 'error'); + var container = document.createElement('div'); + + var beforeUpdate = React.createElement('svg', {}, null); + ReactDOM.render(beforeUpdate, container); + + var afterUpdate = ; + ReactDOM.render(afterUpdate, container); + + expect(container.childNodes[0].getAttribute('clip-path')).toBe( + 'url(#starlet)' + ); + expect(console.error.argsForCall.length).toBe(1); + expect(console.error.argsForCall[0][0]).toContain('clipPath'); + expect(console.error.argsForCall[0][0]).toContain('clip-path'); + }); + + it('should update arbitrary hyphenated attributes for SVG tags', function() { + var container = document.createElement('div'); + + var beforeUpdate = React.createElement('svg', {}, null); + ReactDOM.render(beforeUpdate, container); + + var afterUpdate = ; + ReactDOM.render(afterUpdate, container); + + expect(container.childNodes[0].getAttribute('the-word')).toBe('the-bird'); + }); + + it('should update arbitrary camel case attributes for SVG tags', function() { + var container = document.createElement('div'); + + var beforeUpdate = React.createElement('svg', {}, null); + ReactDOM.render(beforeUpdate, container); + + var afterUpdate = ; + ReactDOM.render(afterUpdate, container); + + expect(container.childNodes[0].getAttribute('theWord')).toBe('theBird'); + }); + + it('should update namespaced SVG attributes', function() { + var container = document.createElement('div'); + + var beforeUpdate = ( + + + + ); + ReactDOM.render(beforeUpdate, container); + + var afterUpdate = ( + + + + ); + ReactDOM.render(afterUpdate, container); + + expect(container.firstChild.firstChild.getAttributeNS( + 'http://www.w3.org/1999/xlink', + 'href' + )).toBe('http://i.imgur.com/JvqCM2p.png'); + }); + it('should clear all the styles when removing `style`', function() { var styles = {display: 'none', color: 'red'}; var container = document.createElement('div'); From 251d6c30b51c6aa9a406f9076c0f895570a710b3 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 24 Dec 2015 17:50:43 +0000 Subject: [PATCH 2/4] Move SVG attribute deprecation warnings into a devtool In #5590 a new system was introduced for tracking dev-time warnings. This commit uses it for reporting SVG attribute deprecation warnings. --- .../dom/shared/DOMPropertyOperations.js | 54 ++++------------ src/renderers/dom/shared/ReactDOMDebugTool.js | 9 +++ .../ReactDOMSVGDeprecatedAttributeDevtool.js | 61 +++++++++++++++++++ .../ReactDOMUnknownPropertyDevtool.js | 2 +- 4 files changed, 83 insertions(+), 43 deletions(-) create mode 100644 src/renderers/dom/shared/devtools/ReactDOMSVGDeprecatedAttributeDevtool.js diff --git a/src/renderers/dom/shared/DOMPropertyOperations.js b/src/renderers/dom/shared/DOMPropertyOperations.js index 0fdb7e0480774..8322c3681373b 100644 --- a/src/renderers/dom/shared/DOMPropertyOperations.js +++ b/src/renderers/dom/shared/DOMPropertyOperations.js @@ -50,32 +50,6 @@ function shouldIgnoreValue(propertyInfo, value) { (propertyInfo.hasOverloadedBooleanValue && value === false); } -if (__DEV__) { - var reactProps = { - children: true, - dangerouslySetInnerHTML: true, - key: true, - ref: true, - }; - var warnedSVGProperties = {}; - - var warnDeprecatedSVGProperty = function(name, attributeName) { - if (reactProps.hasOwnProperty(name) && reactProps[name] || - warnedSVGProperties.hasOwnProperty(name) && warnedSVGProperties[name]) { - return; - } - - warnedSVGProperties[name] = true; - warning( - false, - 'SVG property %s is deprecated. Use the original attribute name ' + - '%s for SVG tags instead.', - name, - attributeName - ); - }; -} - /** * Operations for dealing with DOM properties. */ @@ -158,21 +132,21 @@ var DOMPropertyOperations = { * @return {string} Markup string, or empty string if the property was invalid. */ createMarkupForSVGAttribute: function(name, value) { + if (__DEV__) { + ReactDOMInstrumentation.debugTool.onCreateMarkupForSVGAttribute(name, value); + } if (!isAttributeNameSafe(name) || value == null) { return ''; } var propertyInfo = DOMProperty.properties.hasOwnProperty(name) ? DOMProperty.properties[name] : null; if (propertyInfo) { - var { attributeName, attributeNamespace } = propertyInfo; - if (__DEV__) { - if (!attributeNamespace && name !== attributeName) { - warnDeprecatedSVGProperty(name, attributeName); - } - } + // Migration path for deprecated camelCase aliases for SVG attributes + var { attributeName } = propertyInfo; return attributeName + '=' + quoteAttributeValueForBrowser(value); + } else { + return name + '=' + quoteAttributeValueForBrowser(value); } - return name + '=' + quoteAttributeValueForBrowser(value); }, /** @@ -235,15 +209,11 @@ var DOMPropertyOperations = { }, setValueForSVGAttribute: function(node, name, value) { - var propertyInfo = DOMProperty.properties.hasOwnProperty(name) ? - DOMProperty.properties[name] : null; - if (propertyInfo) { - if (__DEV__) { - var { attributeName, attributeNamespace } = propertyInfo; - if (!attributeNamespace && name !== attributeName) { - warnDeprecatedSVGProperty(name, attributeName); - } - } + if (__DEV__) { + ReactDOMInstrumentation.debugTool.onSetValueForSVGAttribute(node, name, value); + } + if (DOMProperty.properties.hasOwnProperty(name)) { + // Migration path for deprecated camelCase aliases for SVG attributes DOMPropertyOperations.setValueForProperty(node, name, value); } else { DOMPropertyOperations.setValueForAttribute(node, name, value); diff --git a/src/renderers/dom/shared/ReactDOMDebugTool.js b/src/renderers/dom/shared/ReactDOMDebugTool.js index 5dbd0c0ea4b5f..b0477610f5d15 100644 --- a/src/renderers/dom/shared/ReactDOMDebugTool.js +++ b/src/renderers/dom/shared/ReactDOMDebugTool.js @@ -12,6 +12,8 @@ 'use strict'; var ReactDOMUnknownPropertyDevtool = require('ReactDOMUnknownPropertyDevtool'); +var ReactDOMSVGDeprecatedAttributeDevtool = + require('ReactDOMSVGDeprecatedAttributeDevtool'); var warning = require('warning'); @@ -53,14 +55,21 @@ var ReactDOMDebugTool = { onCreateMarkupForProperty(name, value) { emitEvent('onCreateMarkupForProperty', name, value); }, + onCreateMarkupForSVGAttribute(name, value) { + emitEvent('onCreateMarkupForSVGAttribute', name, value); + }, onSetValueForProperty(node, name, value) { emitEvent('onSetValueForProperty', node, name, value); }, + onSetValueForSVGAttribute(node, name, value) { + emitEvent('onSetValueForSVGAttribute', node, name, value); + }, onDeleteValueForProperty(node, name) { emitEvent('onDeleteValueForProperty', node, name); }, }; ReactDOMDebugTool.addDevtool(ReactDOMUnknownPropertyDevtool); +ReactDOMDebugTool.addDevtool(ReactDOMSVGDeprecatedAttributeDevtool); module.exports = ReactDOMDebugTool; diff --git a/src/renderers/dom/shared/devtools/ReactDOMSVGDeprecatedAttributeDevtool.js b/src/renderers/dom/shared/devtools/ReactDOMSVGDeprecatedAttributeDevtool.js new file mode 100644 index 0000000000000..574c5f309182a --- /dev/null +++ b/src/renderers/dom/shared/devtools/ReactDOMSVGDeprecatedAttributeDevtool.js @@ -0,0 +1,61 @@ +/** + * Copyright 2013-2015, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + * @providesModule ReactDOMSVGDeprecatedAttributeDevtool + */ + +'use strict'; + +var DOMProperty = require('DOMProperty'); + +var warning = require('warning'); + +if (__DEV__) { + var reactProps = { + children: true, + dangerouslySetInnerHTML: true, + key: true, + ref: true, + }; + var warnedSVGAttributes = {}; + + var warnDeprecatedSVGAttribute = function(name) { + if (!DOMProperty.properties.hasOwnProperty(name)) { + return; + } + + if (reactProps.hasOwnProperty(name) && reactProps[name] || + warnedSVGAttributes.hasOwnProperty(name) && warnedSVGAttributes[name]) { + return; + } + + var { attributeName, attributeNamespace } = DOMProperty.properties[name]; + if (attributeNamespace || name === attributeName) { + return; + } + + warning( + false, + 'SVG property %s is deprecated. Use the original attribute name ' + + '%s for SVG tags instead.', + name, + attributeName + ); + }; +} + +var ReactDOMSVGDeprecatedAttributeDevtool = { + onCreateMarkupForSVGAttribute(name, value) { + warnDeprecatedSVGAttribute(name); + }, + onSetValueForSVGAttribute(node, name, value) { + warnDeprecatedSVGAttribute(name); + }, +}; + +module.exports = ReactDOMSVGDeprecatedAttributeDevtool; diff --git a/src/renderers/dom/shared/devtools/ReactDOMUnknownPropertyDevtool.js b/src/renderers/dom/shared/devtools/ReactDOMUnknownPropertyDevtool.js index 64c1c19582c40..7b53d03b42cf7 100644 --- a/src/renderers/dom/shared/devtools/ReactDOMUnknownPropertyDevtool.js +++ b/src/renderers/dom/shared/devtools/ReactDOMUnknownPropertyDevtool.js @@ -82,6 +82,6 @@ var ReactDOMUnknownPropertyDevtool = { onDeleteValueForProperty(node, name) { warnUnknownProperty(name); }, -} +}; module.exports = ReactDOMUnknownPropertyDevtool; From f27e3aa7505e7c33be81d669beec31dcec029c06 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 25 Dec 2015 13:48:13 +0000 Subject: [PATCH 3/4] Move the specific else if clause up --- src/renderers/dom/shared/ReactDOMComponent.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/renderers/dom/shared/ReactDOMComponent.js b/src/renderers/dom/shared/ReactDOMComponent.js index c3202db6ac0ee..1347e0f36c834 100644 --- a/src/renderers/dom/shared/ReactDOMComponent.js +++ b/src/renderers/dom/shared/ReactDOMComponent.js @@ -860,15 +860,15 @@ ReactDOMComponent.Mixin = { // listener (e.g., onClick={null}) deleteListener(this, propKey); } - } else if ( - DOMProperty.properties[propKey] || - DOMProperty.isCustomAttribute(propKey)) { - DOMPropertyOperations.deleteValueForProperty(getNode(this), propKey); } else if (this._namespaceURI === DOMNamespaces.svg) { DOMPropertyOperations.deleteValueForSVGAttribute( getNode(this), propKey ); + } else if ( + DOMProperty.properties[propKey] || + DOMProperty.isCustomAttribute(propKey)) { + DOMPropertyOperations.deleteValueForProperty(getNode(this), propKey); } } for (propKey in nextProps) { From 98a7100930d4a877cc1d88b9b1ea410271ee9ea9 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 25 Dec 2015 13:50:10 +0000 Subject: [PATCH 4/4] Use JSX in the new tests --- .../dom/shared/__tests__/ReactDOMComponent-test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js index 368c2ba38cbb0..52ca67a37c1cc 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js @@ -403,7 +403,7 @@ describe('ReactDOMComponent', function() { it('should update known hyphenated attributes for SVG tags', function() { var container = document.createElement('div'); - var beforeUpdate = React.createElement('svg', {}, null); + var beforeUpdate = ; ReactDOM.render(beforeUpdate, container); var afterUpdate = ; @@ -417,7 +417,7 @@ describe('ReactDOMComponent', function() { it('should update camel case attributes for SVG tags', function() { var container = document.createElement('div'); - var beforeUpdate = React.createElement('svg', {}, null); + var beforeUpdate = ; ReactDOM.render(beforeUpdate, container); var afterUpdate = ; @@ -432,7 +432,7 @@ describe('ReactDOMComponent', function() { spyOn(console, 'error'); var container = document.createElement('div'); - var beforeUpdate = React.createElement('svg', {}, null); + var beforeUpdate = ; ReactDOM.render(beforeUpdate, container); var afterUpdate = ; @@ -449,7 +449,7 @@ describe('ReactDOMComponent', function() { it('should update arbitrary hyphenated attributes for SVG tags', function() { var container = document.createElement('div'); - var beforeUpdate = React.createElement('svg', {}, null); + var beforeUpdate = ; ReactDOM.render(beforeUpdate, container); var afterUpdate = ; @@ -461,7 +461,7 @@ describe('ReactDOMComponent', function() { it('should update arbitrary camel case attributes for SVG tags', function() { var container = document.createElement('div'); - var beforeUpdate = React.createElement('svg', {}, null); + var beforeUpdate = ; ReactDOM.render(beforeUpdate, container); var afterUpdate = ;