@samitouri / QOS-React-2 / commits / 232a47ad04

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

Dan Abramov committed Dec 23, 2015 at 17:34 UTC 232a47ad0493fa2664f3205986dbc73ac5061bff
5 files changed +291 -37
src/renderers/dom/client/__tests__/ReactDOMSVG-test.js
+39
@@ -21,6 +21,45 @@ describe('ReactDOMSVG', function() {
21 ReactDOMServer = require('ReactDOMServer');
22 });
23
24 + it('creates initial markup for known hyphenated attributes', function() {
25 + var markup = ReactDOMServer.renderToString(
26 + <svg clip-path="url(#starlet)" />
27 + );
28 + expect(markup).toContain('clip-path="url(#starlet)"');
29 + });
30 +
31 + it('creates initial markup for camel case attributes', function() {
32 + var markup = ReactDOMServer.renderToString(
33 + <svg viewBox="0 0 100 100" />
34 + );
35 + expect(markup).toContain('viewBox="0 0 100 100"');
36 + });
37 +
38 + it('deprecates camel casing of hyphenated attributes', function() {
39 + spyOn(console, 'error');
40 + var markup = ReactDOMServer.renderToString(
41 + <svg clipPath="url(#starlet)" />
42 + );
43 + expect(markup).toContain('clip-path="url(#starlet)"');
44 + expect(console.error.argsForCall.length).toBe(1);
45 + expect(console.error.argsForCall[0][0]).toContain('clipPath');
46 + expect(console.error.argsForCall[0][0]).toContain('clip-path');
47 + });
48 +
49 + it('creates initial markup for unknown hyphenated attributes', function() {
50 + var markup = ReactDOMServer.renderToString(
51 + <svg the-word="the-bird" />
52 + );
53 + expect(markup).toContain('the-word="the-bird"');
54 + });
55 +
56 + it('creates initial markup for unknown camel case attributes', function() {
57 + var markup = ReactDOMServer.renderToString(
58 + <svg theWord="theBird" />
59 + );
60 + expect(markup).toContain('theWord="theBird"');
61 + });
62 +
63 it('creates initial namespaced markup', function() {
64 var markup = ReactDOMServer.renderToString(
65 <svg>
src/renderers/dom/shared/DOMPropertyOperations.js
+77
@@ -50,6 +50,32 @@ function shouldIgnoreValue(propertyInfo, value) {
50 (propertyInfo.hasOverloadedBooleanValue && value === false);
51 }
52
53 +if (__DEV__) {
54 + var reactProps = {
55 + children: true,
56 + dangerouslySetInnerHTML: true,
57 + key: true,
58 + ref: true,
59 + };
60 + var warnedSVGProperties = {};
61 +
62 + var warnDeprecatedSVGProperty = function(name, attributeName) {
63 + if (reactProps.hasOwnProperty(name) && reactProps[name] ||
64 + warnedSVGProperties.hasOwnProperty(name) && warnedSVGProperties[name]) {
65 + return;
66 + }
67 +
68 + warnedSVGProperties[name] = true;
69 + warning(
70 + false,
71 + 'SVG property %s is deprecated. Use the original attribute name ' +
72 + '%s for SVG tags instead.',
73 + name,
74 + attributeName
75 + );
76 + };
77 +}
78 +
79 /**
80 * Operations for dealing with DOM properties.
81 */
@@ -124,6 +150,31 @@ var DOMPropertyOperations = {
150 return name + '=' + quoteAttributeValueForBrowser(value);
151 },
152
153 + /**
154 + * Creates markup for an SVG property.
155 + *
156 + * @param {string} name
157 + * @param {*} value
158 + * @return {string} Markup string, or empty string if the property was invalid.
159 + */
160 + createMarkupForSVGAttribute: function(name, value) {
161 + if (!isAttributeNameSafe(name) || value == null) {
162 + return '';
163 + }
164 + var propertyInfo = DOMProperty.properties.hasOwnProperty(name) ?
165 + DOMProperty.properties[name] : null;
166 + if (propertyInfo) {
167 + var { attributeName, attributeNamespace } = propertyInfo;
168 + if (__DEV__) {
169 + if (!attributeNamespace && name !== attributeName) {
170 + warnDeprecatedSVGProperty(name, attributeName);
171 + }
172 + }
173 + return attributeName + '=' + quoteAttributeValueForBrowser(value);
174 + }
175 + return name + '=' + quoteAttributeValueForBrowser(value);
176 + },
177 +
178 /**
179 * Sets the value for a property on a node.
180 *
@@ -183,6 +234,22 @@ var DOMPropertyOperations = {
234 }
235 },
236
237 + setValueForSVGAttribute: function(node, name, value) {
238 + var propertyInfo = DOMProperty.properties.hasOwnProperty(name) ?
239 + DOMProperty.properties[name] : null;
240 + if (propertyInfo) {
241 + if (__DEV__) {
242 + var { attributeName, attributeNamespace } = propertyInfo;
243 + if (!attributeNamespace && name !== attributeName) {
244 + warnDeprecatedSVGProperty(name, attributeName);
245 + }
246 + }
247 + DOMPropertyOperations.setValueForProperty(node, name, value);
248 + } else {
249 + DOMPropertyOperations.setValueForAttribute(node, name, value);
250 + }
251 + },
252 +
253 /**
254 * Deletes the value for a property on a node.
255 *
@@ -217,6 +284,16 @@ var DOMPropertyOperations = {
284 }
285 },
286
287 + deleteValueForSVGAttribute: function(node, name) {
288 + var propertyInfo = DOMProperty.properties.hasOwnProperty(name) ?
289 + DOMProperty.properties[name] : null;
290 + if (propertyInfo) {
291 + DOMPropertyOperations.deleteValueForProperty(node, name);
292 + } else {
293 + node.removeAttribute(name);
294 + }
295 + },
296 +
297 };
298
299 ReactPerf.measureMethods(DOMPropertyOperations, 'DOMPropertyOperations', {
src/renderers/dom/shared/ReactDOMComponent.js
+13
@@ -647,6 +647,8 @@ ReactDOMComponent.Mixin = {
647 if (propKey !== CHILDREN) {
648 markup = DOMPropertyOperations.createMarkupForCustomAttribute(propKey, propValue);
649 }
650 + } else if (this._namespaceURI === DOMNamespaces.svg) {
651 + markup = DOMPropertyOperations.createMarkupForSVGAttribute(propKey, propValue);
652 } else {
653 markup = DOMPropertyOperations.createMarkupForProperty(propKey, propValue);
654 }
@@ -862,6 +864,11 @@ ReactDOMComponent.Mixin = {
864 DOMProperty.properties[propKey] ||
865 DOMProperty.isCustomAttribute(propKey)) {
866 DOMPropertyOperations.deleteValueForProperty(getNode(this), propKey);
867 + } else if (this._namespaceURI === DOMNamespaces.svg) {
868 + DOMPropertyOperations.deleteValueForSVGAttribute(
869 + getNode(this),
870 + propKey
871 + );
872 }
873 }
874 for (propKey in nextProps) {
@@ -924,6 +931,12 @@ ReactDOMComponent.Mixin = {
931 propKey,
932 nextProp
933 );
934 + } else if (this._namespaceURI === DOMNamespaces.svg) {
935 + DOMPropertyOperations.setValueForSVGAttribute(
936 + getNode(this),
937 + propKey,
938 + nextProp
939 + );
940 } else if (
941 DOMProperty.properties[propKey] ||
942 DOMProperty.isCustomAttribute(propKey)) {
src/renderers/dom/shared/SVGDOMPropertyConfig.js
-37
@@ -23,46 +23,19 @@ var NS = {
23 var SVGDOMPropertyConfig = {
24 Properties: {
25 clipPath: MUST_USE_ATTRIBUTE,
26 - cx: MUST_USE_ATTRIBUTE,
27 - cy: MUST_USE_ATTRIBUTE,
28 - d: MUST_USE_ATTRIBUTE,
29 - dx: MUST_USE_ATTRIBUTE,
30 - dy: MUST_USE_ATTRIBUTE,
31 - fill: MUST_USE_ATTRIBUTE,
26 fillOpacity: MUST_USE_ATTRIBUTE,
27 fontFamily: MUST_USE_ATTRIBUTE,
28 fontSize: MUST_USE_ATTRIBUTE,
35 - fx: MUST_USE_ATTRIBUTE,
36 - fy: MUST_USE_ATTRIBUTE,
37 - gradientTransform: MUST_USE_ATTRIBUTE,
38 - gradientUnits: MUST_USE_ATTRIBUTE,
29 markerEnd: MUST_USE_ATTRIBUTE,
30 markerMid: MUST_USE_ATTRIBUTE,
31 markerStart: MUST_USE_ATTRIBUTE,
42 - offset: MUST_USE_ATTRIBUTE,
43 - opacity: MUST_USE_ATTRIBUTE,
44 - patternContentUnits: MUST_USE_ATTRIBUTE,
45 - patternUnits: MUST_USE_ATTRIBUTE,
46 - points: MUST_USE_ATTRIBUTE,
47 - preserveAspectRatio: MUST_USE_ATTRIBUTE,
48 - r: MUST_USE_ATTRIBUTE,
49 - rx: MUST_USE_ATTRIBUTE,
50 - ry: MUST_USE_ATTRIBUTE,
51 - spreadMethod: MUST_USE_ATTRIBUTE,
32 stopColor: MUST_USE_ATTRIBUTE,
33 stopOpacity: MUST_USE_ATTRIBUTE,
54 - stroke: MUST_USE_ATTRIBUTE,
34 strokeDasharray: MUST_USE_ATTRIBUTE,
35 strokeLinecap: MUST_USE_ATTRIBUTE,
36 strokeOpacity: MUST_USE_ATTRIBUTE,
37 strokeWidth: MUST_USE_ATTRIBUTE,
38 textAnchor: MUST_USE_ATTRIBUTE,
60 - transform: MUST_USE_ATTRIBUTE,
61 - version: MUST_USE_ATTRIBUTE,
62 - viewBox: MUST_USE_ATTRIBUTE,
63 - x1: MUST_USE_ATTRIBUTE,
64 - x2: MUST_USE_ATTRIBUTE,
65 - x: MUST_USE_ATTRIBUTE,
39 xlinkActuate: MUST_USE_ATTRIBUTE,
40 xlinkArcrole: MUST_USE_ATTRIBUTE,
41 xlinkHref: MUST_USE_ATTRIBUTE,
@@ -73,9 +46,6 @@ var SVGDOMPropertyConfig = {
46 xmlBase: MUST_USE_ATTRIBUTE,
47 xmlLang: MUST_USE_ATTRIBUTE,
48 xmlSpace: MUST_USE_ATTRIBUTE,
76 - y1: MUST_USE_ATTRIBUTE,
77 - y2: MUST_USE_ATTRIBUTE,
78 - y: MUST_USE_ATTRIBUTE,
49 },
50 DOMAttributeNamespaces: {
51 xlinkActuate: NS.xlink,
@@ -94,15 +64,9 @@ var SVGDOMPropertyConfig = {
64 fillOpacity: 'fill-opacity',
65 fontFamily: 'font-family',
66 fontSize: 'font-size',
97 - gradientTransform: 'gradientTransform',
98 - gradientUnits: 'gradientUnits',
67 markerEnd: 'marker-end',
68 markerMid: 'marker-mid',
69 markerStart: 'marker-start',
102 - patternContentUnits: 'patternContentUnits',
103 - patternUnits: 'patternUnits',
104 - preserveAspectRatio: 'preserveAspectRatio',
105 - spreadMethod: 'spreadMethod',
70 stopColor: 'stop-color',
71 stopOpacity: 'stop-opacity',
72 strokeDasharray: 'stroke-dasharray',
@@ -110,7 +74,6 @@ var SVGDOMPropertyConfig = {
74 strokeOpacity: 'stroke-opacity',
75 strokeWidth: 'stroke-width',
76 textAnchor: 'text-anchor',
113 - viewBox: 'viewBox',
77 xlinkActuate: 'xlink:actuate',
78 xlinkArcrole: 'xlink:arcrole',
79 xlinkHref: 'xlink:href',
src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
+162
@@ -260,6 +260,75 @@ describe('ReactDOMComponent', function() {
260 expect(container.firstChild.hasAttribute('height')).toBe(false);
261 });
262
263 + it('should remove known SVG camel case attributes', function() {
264 + var container = document.createElement('div');
265 + ReactDOM.render(<svg viewBox="0 0 100 100" />, container);
266 +
267 + expect(container.firstChild.hasAttribute('viewBox')).toBe(true);
268 + ReactDOM.render(<svg />, container);
269 + expect(container.firstChild.hasAttribute('viewBox')).toBe(false);
270 + });
271 +
272 + it('should remove known SVG hyphenated attributes', function() {
273 + var container = document.createElement('div');
274 + ReactDOM.render(<svg clip-path="0 0 100 100" />, container);
275 +
276 + expect(container.firstChild.hasAttribute('clip-path')).toBe(true);
277 + ReactDOM.render(<svg />, container);
278 + expect(container.firstChild.hasAttribute('clip-path')).toBe(false);
279 + });
280 +
281 + it('should remove arbitrary SVG hyphenated attributes', function() {
282 + var container = document.createElement('div');
283 + ReactDOM.render(<svg the-word="the-bird" />, container);
284 +
285 + expect(container.firstChild.hasAttribute('the-word')).toBe(true);
286 + ReactDOM.render(<svg />, container);
287 + expect(container.firstChild.hasAttribute('the-word')).toBe(false);
288 + });
289 +
290 + it('should remove arbitrary SVG camel case attributes', function() {
291 + var container = document.createElement('div');
292 + ReactDOM.render(<svg theWord="theBird" />, container);
293 +
294 + expect(container.firstChild.hasAttribute('theWord')).toBe(true);
295 + ReactDOM.render(<svg />, container);
296 + expect(container.firstChild.hasAttribute('theWord')).toBe(false);
297 + });
298 +
299 + it('should remove SVG attributes that should have been hyphenated', function() {
300 + spyOn(console, 'error');
301 + var container = document.createElement('div');
302 + ReactDOM.render(<svg clipPath="0 0 100 100" />, container);
303 + expect(console.error.argsForCall.length).toBe(1);
304 + expect(console.error.argsForCall[0][0]).toContain('clipPath');
305 + expect(console.error.argsForCall[0][0]).toContain('clip-path');
306 +
307 + expect(container.firstChild.hasAttribute('clip-path')).toBe(true);
308 + ReactDOM.render(<svg />, container);
309 + expect(container.firstChild.hasAttribute('clip-path')).toBe(false);
310 + });
311 +
312 + it('should remove namespaced SVG attributes', function() {
313 + var container = document.createElement('div');
314 + ReactDOM.render(
315 + <svg>
316 + <image xlinkHref="http://i.imgur.com/w7GCRPb.png" />
317 + </svg>,
318 + container
319 + );
320 +
321 + expect(container.firstChild.firstChild.hasAttributeNS(
322 + 'http://www.w3.org/1999/xlink',
323 + 'href'
324 + )).toBe(true);
325 + ReactDOM.render(<svg><image /></svg>, container);
326 + expect(container.firstChild.firstChild.hasAttributeNS(
327 + 'http://www.w3.org/1999/xlink',
328 + 'href'
329 + )).toBe(false);
330 + });
331 +
332 it('should remove properties', function() {
333 var container = document.createElement('div');
334 ReactDOM.render(<div className="monkey" />, container);
@@ -331,6 +400,99 @@ describe('ReactDOMComponent', function() {
400 expect(container.childNodes[0].getAttribute('myattr')).toBe('myval');
401 });
402
403 + it('should update known hyphenated attributes for SVG tags', function() {
404 + var container = document.createElement('div');
405 +
406 + var beforeUpdate = React.createElement('svg', {}, null);
407 + ReactDOM.render(beforeUpdate, container);
408 +
409 + var afterUpdate = <svg clip-path="url(#starlet)" />;
410 + ReactDOM.render(afterUpdate, container);
411 +
412 + expect(container.childNodes[0].getAttribute('clip-path')).toBe(
413 + 'url(#starlet)'
414 + );
415 + });
416 +
417 + it('should update camel case attributes for SVG tags', function() {
418 + var container = document.createElement('div');
419 +
420 + var beforeUpdate = React.createElement('svg', {}, null);
421 + ReactDOM.render(beforeUpdate, container);
422 +
423 + var afterUpdate = <svg viewBox="0 0 100 100" />;
424 + ReactDOM.render(afterUpdate, container);
425 +
426 + expect(container.childNodes[0].getAttribute('viewBox')).toBe(
427 + '0 0 100 100'
428 + );
429 + });
430 +
431 + it('should warn camel casing hyphenated attributes for SVG tags', function() {
432 + spyOn(console, 'error');
433 + var container = document.createElement('div');
434 +
435 + var beforeUpdate = React.createElement('svg', {}, null);
436 + ReactDOM.render(beforeUpdate, container);
437 +
438 + var afterUpdate = <svg clipPath="url(#starlet)" />;
439 + ReactDOM.render(afterUpdate, container);
440 +
441 + expect(container.childNodes[0].getAttribute('clip-path')).toBe(
442 + 'url(#starlet)'
443 + );
444 + expect(console.error.argsForCall.length).toBe(1);
445 + expect(console.error.argsForCall[0][0]).toContain('clipPath');
446 + expect(console.error.argsForCall[0][0]).toContain('clip-path');
447 + });
448 +
449 + it('should update arbitrary hyphenated attributes for SVG tags', function() {
450 + var container = document.createElement('div');
451 +
452 + var beforeUpdate = React.createElement('svg', {}, null);
453 + ReactDOM.render(beforeUpdate, container);
454 +
455 + var afterUpdate = <svg the-word="the-bird" />;
456 + ReactDOM.render(afterUpdate, container);
457 +
458 + expect(container.childNodes[0].getAttribute('the-word')).toBe('the-bird');
459 + });
460 +
461 + it('should update arbitrary camel case attributes for SVG tags', function() {
462 + var container = document.createElement('div');
463 +
464 + var beforeUpdate = React.createElement('svg', {}, null);
465 + ReactDOM.render(beforeUpdate, container);
466 +
467 + var afterUpdate = <svg theWord="theBird" />;
468 + ReactDOM.render(afterUpdate, container);
469 +
470 + expect(container.childNodes[0].getAttribute('theWord')).toBe('theBird');
471 + });
472 +
473 + it('should update namespaced SVG attributes', function() {
474 + var container = document.createElement('div');
475 +
476 + var beforeUpdate = (
477 + <svg>
478 + <image xlinkHref="http://i.imgur.com/w7GCRPb.png" />
479 + </svg>
480 + );
481 + ReactDOM.render(beforeUpdate, container);
482 +
483 + var afterUpdate = (
484 + <svg>
485 + <image xlinkHref="http://i.imgur.com/JvqCM2p.png" />
486 + </svg>
487 + );
488 + ReactDOM.render(afterUpdate, container);
489 +
490 + expect(container.firstChild.firstChild.getAttributeNS(
491 + 'http://www.w3.org/1999/xlink',
492 + 'href'
493 + )).toBe('http://i.imgur.com/JvqCM2p.png');
494 + });
495 +
496 it('should clear all the styles when removing `style`', function() {
497 var styles = {display: 'none', color: 'red'};
498 var container = document.createElement('div');