Skip to content

Commit

Permalink
Merge pull request #1662 from gaearon/warn-when-using-hyphenated-styl…
Browse files Browse the repository at this point in the history
…e-names

Warn when using hyphenated style property names

Closes #1662

Conflicts:
	src/browser/ui/dom/CSSPropertyOperations.js
  • Loading branch information
zpao committed Aug 11, 2014
2 parents 0e5316f + e625b8b commit 986f5a1
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 5 deletions.
5 changes: 3 additions & 2 deletions docs/tips/02-inline-styles.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@ In React, inline styles are not specified as a string. Instead they are specifie
var divStyle = {
color: 'white',
backgroundImage: 'url(' + imgUrl + ')',
WebkitTransition: 'all' // note the capital 'W' here
WebkitTransition: 'all', // note the capital 'W' here
msTransition: 'all' // 'ms' is the only lowercase vendor prefix
};

React.renderComponent(<div style={divStyle}>Hello World!</div>, mountNode);
```

Style keys are camelCased in order to be consistent with accessing the properties on DOM nodes from JS (e.g. `node.style.backgroundImage`). Vendor prefixes should begin with a capital letter. This is why `WebkitTransition` has an uppercase "W".
Style keys are camelCased in order to be consistent with accessing the properties on DOM nodes from JS (e.g. `node.style.backgroundImage`). Vendor prefixes [other than `ms`](http://www.andismith.com/blog/2012/02/modernizr-prefixed/) should begin with a capital letter. This is why `WebkitTransition` has an uppercase "W".
29 changes: 29 additions & 0 deletions src/browser/ui/dom/CSSPropertyOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@
var CSSProperty = require('CSSProperty');
var ExecutionEnvironment = require('ExecutionEnvironment');

var camelizeStyleName = require('camelizeStyleName');
var dangerousStyleValue = require('dangerousStyleValue');
var hyphenateStyleName = require('hyphenateStyleName');
var memoizeStringOnly = require('memoizeStringOnly');
var warning = require('warning');

var processStyleName = memoizeStringOnly(function(styleName) {
return hyphenateStyleName(styleName);
Expand All @@ -38,6 +40,23 @@ if (ExecutionEnvironment.canUseDOM) {
}
}

if (__DEV__) {
var warnedStyleNames = {};

var warnHyphenatedStyleName = function(name) {
if (warnedStyleNames.hasOwnProperty(name) && warnedStyleNames[name]) {
return;
}

warnedStyleNames[name] = true;
warning(
false,
'Unsupported style property ' + name + '. Did you mean ' +
camelizeStyleName(name) + '?'
);
};
}

/**
* Operations for dealing with CSS properties.
*/
Expand All @@ -61,6 +80,11 @@ var CSSPropertyOperations = {
if (!styles.hasOwnProperty(styleName)) {
continue;
}
if (__DEV__) {
if (styleName.indexOf('-') > -1) {
warnHyphenatedStyleName(styleName);
}
}
var styleValue = styles[styleName];
if (styleValue != null) {
serialized += processStyleName(styleName) + ':';
Expand All @@ -83,6 +107,11 @@ var CSSPropertyOperations = {
if (!styles.hasOwnProperty(styleName)) {
continue;
}
if (__DEV__) {
if (styleName.indexOf('-') > -1) {
warnHyphenatedStyleName(styleName);
}
}
var styleValue = dangerousStyleValue(styleName, styles[styleName]);
if (styleName === 'float') {
styleName = styleFloatAccessor;
Expand Down
28 changes: 28 additions & 0 deletions src/browser/ui/dom/__tests__/CSSPropertyOperations-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,4 +116,32 @@ describe('CSSPropertyOperations', function() {
expect(/style=".*"/.test(root.innerHTML)).toBe(false);
});

it('should warn when using hyphenated style names', function() {
spyOn(console, 'warn');

expect(CSSPropertyOperations.createMarkupForStyles({
'background-color': 'crimson'
})).toBe('background-color:crimson;');

expect(console.warn.argsForCall.length).toBe(1);
expect(console.warn.argsForCall[0][0]).toContain('backgroundColor');
});

it('should warn when updating hyphenated style names', function() {
spyOn(console, 'warn');

var root = document.createElement('div');
var styles = {
'-ms-transform': 'translate3d(0, 0, 0)',
'-webkit-transform': 'translate3d(0, 0, 0)'
};

React.renderComponent(<div />, root);
React.renderComponent(<div style={styles} />, root);

expect(console.warn.argsForCall.length).toBe(2);
expect(console.warn.argsForCall[0][0]).toContain('msTransform');
expect(console.warn.argsForCall[1][0]).toContain('WebkitTransform');
});

});
47 changes: 47 additions & 0 deletions src/vendor/core/camelizeStyleName.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/**
* Copyright 2014 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 camelizeStyleName
* @typechecks
*/

"use strict";

var camelize = require('camelize');

var msPattern = /^-ms-/;

/**
* Camelcases a hyphenated CSS property name, for example:
*
* > camelizeStyleName('background-color')
* < "backgroundColor"
* > camelizeStyleName('-moz-transition')
* < "MozTransition"
* > camelizeStyleName('-ms-transition')
* < "msTransition"
*
* As Andi Smith suggests
* (http://www.andismith.com/blog/2012/02/modernizr-prefixed/), an `-ms` prefix
* is converted to lowercase `ms`.
*
* @param {string} string
* @return {string}
*/
function camelizeStyleName(string) {
return camelize(string.replace(msPattern, 'ms-'));
}

module.exports = camelizeStyleName;
6 changes: 3 additions & 3 deletions src/vendor/core/hyphenateStyleName.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ var msPattern = /^ms-/;
/**
* Hyphenates a camelcased CSS property name, for example:
*
* > hyphenate('backgroundColor')
* > hyphenateStyleName('backgroundColor')
* < "background-color"
* > hyphenate('MozTransition')
* > hyphenateStyleName('MozTransition')
* < "-moz-transition"
* > hyphenate('msTransition')
* > hyphenateStyleName('msTransition')
* < "-ms-transition"
*
* As Modernizr suggests (http://modernizr.com/docs/#prefixed), an `ms` prefix
Expand Down

0 comments on commit 986f5a1

Please sign in to comment.