Skip to content

Commit

Permalink
Prevents adding units to css custom properties (#9966)
Browse files Browse the repository at this point in the history
* Prevents adding units to css custom properties

* Fix code style

* Optimize custom property checking

* Prevents adding units to css custom properties in markup creation

* Update passing tests

* Fix argument name and reuse check in DEV
  • Loading branch information
TrySound authored and gaearon committed Jun 14, 2017
1 parent ae94ea7 commit 80255d1
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 10 deletions.
24 changes: 16 additions & 8 deletions src/renderers/dom/shared/CSSPropertyOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,6 @@ if (__DEV__) {
* @param {ReactDOMComponent} component
*/
var warnValidStyle = function(name, value, component) {
// Don't warn for CSS variables
if (name.indexOf('--') === 0) {
return;
}
var owner;
if (component) {
owner = component._currentElement._owner;
Expand Down Expand Up @@ -173,14 +169,22 @@ var CSSPropertyOperations = {
if (!styles.hasOwnProperty(styleName)) {
continue;
}
var isCustomProperty = styleName.indexOf('--') === 0;
var styleValue = styles[styleName];
if (__DEV__) {
warnValidStyle(styleName, styleValue, component);
if (!isCustomProperty) {
warnValidStyle(styleName, styleValue, component);
}
}
if (styleValue != null) {
serialized += processStyleName(styleName) + ':';
serialized +=
dangerousStyleValue(styleName, styleValue, component) + ';';
dangerousStyleValue(
styleName,
styleValue,
component,
isCustomProperty,
) + ';';
}
}
return serialized || null;
Expand Down Expand Up @@ -208,18 +212,22 @@ var CSSPropertyOperations = {
if (!styles.hasOwnProperty(styleName)) {
continue;
}
var isCustomProperty = styleName.indexOf('--') === 0;
if (__DEV__) {
warnValidStyle(styleName, styles[styleName], component);
if (!isCustomProperty) {
warnValidStyle(styleName, styles[styleName], component);
}
}
var styleValue = dangerousStyleValue(
styleName,
styles[styleName],
component,
isCustomProperty,
);
if (styleName === 'float' || styleName === 'cssFloat') {
styleName = styleFloatAccessor;
}
if (styleName.indexOf('--') === 0) {
if (isCustomProperty) {
style.setProperty(styleName, styleValue);
} else if (styleValue) {
style[styleName] = styleValue;
Expand Down
23 changes: 22 additions & 1 deletion src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,14 @@ describe('CSSPropertyOperations', () => {
).toBe('-ms-transition:none;-moz-transition:none;');
});

it('should create markup with unitless css custom property', () => {
expect(
CSSPropertyOperations.createMarkupForStyles({
'--foo': 5,
}),
).toBe('--foo:5;');
});

it('should set style attribute when styles exist', () => {
var styles = {
backgroundColor: '#000',
Expand Down Expand Up @@ -254,7 +262,7 @@ describe('CSSPropertyOperations', () => {
);
});

it('should not warn when setting CSS variables', () => {
it('should not warn when setting CSS custom properties', () => {
class Comp extends React.Component {
render() {
return <div style={{'--foo-primary': 'red', backgroundColor: 'red'}} />;
Expand All @@ -267,4 +275,17 @@ describe('CSSPropertyOperations', () => {

expect(console.error.calls.count()).toBe(0);
});

it('should not add units to CSS custom properties', () => {
class Comp extends React.Component {
render() {
return <div style={{'--foo': 5}} />;
}
}

var root = document.createElement('div');
ReactDOM.render(<Comp />, root);

expect(root.children[0].style.Foo).toEqual('5');
});
});
3 changes: 2 additions & 1 deletion src/renderers/dom/shared/dangerousStyleValue.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ var styleWarnings = {};
* @param {ReactDOMComponent} component
* @return {string} Normalized style value with dimensions applied.
*/
function dangerousStyleValue(name, value, component) {
function dangerousStyleValue(name, value, component, isCustomProperty) {
// Note that we've removed escapeTextForBrowser() calls here since the
// whole string will be escaped when the attribute is injected into
// the markup. If you provide unsafe user data here they can inject
Expand All @@ -45,6 +45,7 @@ function dangerousStyleValue(name, value, component) {

var isNonNumeric = isNaN(value);
if (
isCustomProperty ||
isNonNumeric ||
value === 0 ||
(isUnitlessNumber.hasOwnProperty(name) && isUnitlessNumber[name])
Expand Down

0 comments on commit 80255d1

Please sign in to comment.