Skip to content

Commit 2d2468b

Browse files
committed
Support arbitrary attributes on elements with dashes in the tag name.
1 parent db82ed0 commit 2d2468b

File tree

5 files changed

+152
-7
lines changed

5 files changed

+152
-7
lines changed

src/renderers/dom/client/ReactDOMIDOperations.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,24 @@ var ReactDOMIDOperations = {
6666
}
6767
},
6868

69+
/**
70+
* Updates a DOM node with new property values.
71+
*
72+
* @param {string} id ID of the node to update.
73+
* @param {string} name A valid property name.
74+
* @param {*} value New value of the property.
75+
* @internal
76+
*/
77+
updateAttributeByID: function(id, name, value) {
78+
var node = ReactMount.getNode(id);
79+
invariant(
80+
!INVALID_PROPERTY_ERRORS.hasOwnProperty(name),
81+
'updatePropertyByID(...): %s',
82+
INVALID_PROPERTY_ERRORS[name]
83+
);
84+
DOMPropertyOperations.setValueForAttribute(node, name, value);
85+
},
86+
6987
/**
7088
* Updates a DOM node to remove a property. This should only be used to remove
7189
* DOM properties in `DOMProperty`.

src/renderers/dom/shared/DOMPropertyOperations.js

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,30 @@ var DOMProperty = require('DOMProperty');
1717
var quoteAttributeValueForBrowser = require('quoteAttributeValueForBrowser');
1818
var warning = require('warning');
1919

20+
var VALID_ATTRIBUTE_NAME_REGEX = /^[a-zA-Z_][a-zA-Z_\.\-\d]*$/; // Simplified subset
21+
var illegalAttributeNameCache = {};
22+
var validatedAttributeNameCache = {};
23+
24+
function isAttributeNameSafe(attributeName) {
25+
if (validatedAttributeNameCache.hasOwnProperty(attributeName)) {
26+
return true;
27+
}
28+
if (illegalAttributeNameCache.hasOwnProperty(attributeName)) {
29+
return false;
30+
}
31+
if (VALID_ATTRIBUTE_NAME_REGEX.test(attributeName)) {
32+
validatedAttributeNameCache[attributeName] = true;
33+
return true;
34+
}
35+
illegalAttributeNameCache[attributeName] = true;
36+
warning(
37+
false,
38+
'Invalid attribute name: `%s`',
39+
attributeName
40+
);
41+
return false;
42+
}
43+
2044
function shouldIgnoreValue(name, value) {
2145
return value == null ||
2246
(DOMProperty.hasBooleanValue[name] && !value) ||
@@ -110,6 +134,23 @@ var DOMPropertyOperations = {
110134
return null;
111135
},
112136

137+
/**
138+
* Creates markup for a custom property.
139+
*
140+
* @param {string} name
141+
* @param {*} value
142+
* @return {?string} Markup string, or null if the property was invalid.
143+
*/
144+
createMarkupForCustomAttribute: function(name, value) {
145+
if (!isAttributeNameSafe(name)) {
146+
return '';
147+
}
148+
if (value == null) {
149+
return '';
150+
}
151+
return name + '=' + quoteAttributeValueForBrowser(value);
152+
},
153+
113154
/**
114155
* Sets the value for a property on a node.
115156
*
@@ -147,16 +188,23 @@ var DOMPropertyOperations = {
147188
}
148189
}
149190
} else if (DOMProperty.isCustomAttribute(name)) {
150-
if (value == null) {
151-
node.removeAttribute(name);
152-
} else {
153-
node.setAttribute(name, '' + value);
154-
}
191+
DOMPropertyOperations.setValueForAttribute(node, name, value);
155192
} else if (__DEV__) {
156193
warnUnknownProperty(name);
157194
}
158195
},
159196

197+
setValueForAttribute: function(node, name, value) {
198+
if (!isAttributeNameSafe(name)) {
199+
return;
200+
}
201+
if (value == null) {
202+
node.removeAttribute(name);
203+
} else {
204+
node.setAttribute(name, '' + value);
205+
}
206+
},
207+
160208
/**
161209
* Deletes the value for a property on a node.
162210
*

src/renderers/dom/shared/ReactDOMComponent.js

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -333,8 +333,12 @@ ReactDOMComponent.Mixin = {
333333
}
334334
propValue = CSSPropertyOperations.createMarkupForStyles(propValue);
335335
}
336-
var markup =
337-
DOMPropertyOperations.createMarkupForProperty(propKey, propValue);
336+
var markup = null;
337+
if (this._tag != null && this._tag.indexOf('-') >= 0) {
338+
markup = DOMPropertyOperations.createMarkupForCustomAttribute(propKey, propValue);
339+
} else {
340+
markup = DOMPropertyOperations.createMarkupForProperty(propKey, propValue);
341+
}
338342
if (markup) {
339343
ret += ' ' + markup;
340344
}
@@ -535,6 +539,12 @@ ReactDOMComponent.Mixin = {
535539
} else if (lastProp) {
536540
deleteListener(this._rootNodeID, propKey);
537541
}
542+
} else if (this._tag.indexOf('-') >= 0) {
543+
BackendIDOperations.updateAttributeByID(
544+
this._rootNodeID,
545+
propKey,
546+
nextProp
547+
);
538548
} else if (
539549
DOMProperty.isStandardName[propKey] ||
540550
DOMProperty.isCustomAttribute(propKey)) {

src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,21 @@ describe('DOMPropertyOperations', function() {
179179

180180
});
181181

182+
describe('createMarkupForProperty', function() {
183+
184+
it('should allow custom properties on web components', function() {
185+
expect(DOMPropertyOperations.createMarkupForCustomAttribute(
186+
'awesomeness',
187+
5
188+
)).toBe('awesomeness="5"');
189+
190+
expect(DOMPropertyOperations.createMarkupForCustomAttribute(
191+
'dev',
192+
'jim'
193+
)).toBe('dev="jim"');
194+
});
195+
});
196+
182197
describe('setValueForProperty', function() {
183198
var stubNode;
184199

src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ describe('ReactDOMComponent', function() {
2121
var ReactTestUtils;
2222

2323
beforeEach(function() {
24+
require('mock-modules').dumpCache();
2425
React = require('React');
2526
ReactTestUtils = require('ReactTestUtils');
2627
});
@@ -203,6 +204,59 @@ describe('ReactDOMComponent', function() {
203204
expect(stubStyle.color).toEqual('green');
204205
});
205206

207+
it('should reject haxors on initial markup', function() {
208+
spyOn(console, 'error');
209+
for (var i = 0; i < 3; i++) {
210+
var container = document.createElement('div');
211+
var element = React.createElement(
212+
'x-foo-component',
213+
{'blah" onclick="beevil" noise="hi': 'selected'},
214+
null
215+
);
216+
React.render(element, container);
217+
}
218+
expect(console.error.argsForCall.length).toBe(1);
219+
expect(console.error.argsForCall[0][0]).toEqual(
220+
'Warning: Invalid attribute name: `blah" onclick="beevil" noise="hi`'
221+
);
222+
});
223+
224+
it('should reject haxors on update', function() {
225+
spyOn(console, 'error');
226+
for (var i = 0; i < 3; i++) {
227+
var container = document.createElement('div');
228+
var beforeUpdate = React.createElement('x-foo-component', {}, null);
229+
React.render(beforeUpdate, container);
230+
231+
var afterUpdate = React.createElement(
232+
'x-foo-component',
233+
{'blah" onclick="beevil" noise="hi': 'selected'},
234+
null
235+
);
236+
React.render(afterUpdate, container);
237+
}
238+
expect(console.error.argsForCall.length).toBe(1);
239+
expect(console.error.argsForCall[0][0]).toEqual(
240+
'Warning: Invalid attribute name: `blah" onclick="beevil" noise="hi`'
241+
);
242+
});
243+
244+
it('should update arbitrary attributes for tags containnig dashes', function() {
245+
var container = document.createElement('div');
246+
247+
var beforeUpdate = React.createElement('x-foo-component', {}, null);
248+
React.render(beforeUpdate, container);
249+
250+
var afterUpdate = React.createElement(
251+
'x-foo-component',
252+
{'myattr': 'myval'},
253+
null
254+
);
255+
React.render(afterUpdate, container);
256+
257+
expect(container.childNodes[0].getAttribute('myattr')).toBe('myval');
258+
});
259+
206260
it('should clear all the styles when removing `style`', function() {
207261
var styles = {display: 'none', color: 'red'};
208262
var container = document.createElement('div');

0 commit comments

Comments
 (0)