Skip to content

Commit

Permalink
Merge pull request #5811 from jontewks/warn-nan-style
Browse files Browse the repository at this point in the history
Warn when a style object has NaN for a value
  • Loading branch information
zpao committed Jan 19, 2016
2 parents ae5ff24 + 7a6000c commit 31d3bfa
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 1 deletion.
18 changes: 18 additions & 0 deletions src/renderers/dom/shared/CSSPropertyOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ if (__DEV__) {

var warnedStyleNames = {};
var warnedStyleValues = {};
var warnedForNaNValue = false;

var warnHyphenatedStyleName = function(name) {
if (warnedStyleNames.hasOwnProperty(name) && warnedStyleNames[name]) {
Expand Down Expand Up @@ -94,6 +95,19 @@ if (__DEV__) {
);
};

var warnStyleValueIsNaN = function(name, value) {
if (warnedForNaNValue) {
return;
}

warnedForNaNValue = true;
warning(
false,
'`NaN` is an invalid value for the `%s` css style property',
name
);
};

/**
* @param {string} name
* @param {*} value
Expand All @@ -106,6 +120,10 @@ if (__DEV__) {
} else if (badStyleValueWithSemicolonPattern.test(value)) {
warnStyleValueWithSemicolon(name, value);
}

if (typeof value === 'number' && isNaN(value)) {
warnStyleValueIsNaN(name, value);
}
};
}

Expand Down
13 changes: 13 additions & 0 deletions src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,4 +166,17 @@ describe('CSSPropertyOperations', function() {
expect(console.error.argsForCall[0][0]).toContain('Try "backgroundColor: blue" instead');
expect(console.error.argsForCall[1][0]).toContain('Try "color: red" instead');
});

it('should warn about style containing a NaN value', function() {
spyOn(console, 'error');

CSSPropertyOperations.createMarkupForStyles({
fontSize: NaN,
});

expect(console.error.calls.length).toBe(1);
expect(console.error.argsForCall[0][0]).toEqual(
'Warning: `NaN` is an invalid value for the `fontSize` css style property'
);
});
});
5 changes: 4 additions & 1 deletion src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,11 @@ describe('ReactDOMComponent', function() {
ReactDOM.render(<span style={style}></span>, div);
ReactDOM.render(<span style={style}></span>, div);

expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall.length).toBe(2);
expect(console.error.argsForCall[0][0]).toEqual(
'Warning: `NaN` is an invalid value for the `fontSize` css style property',
);
expect(console.error.argsForCall[1][0]).toEqual(
'Warning: `span` was passed a style object that has previously been ' +
'mutated. Mutating `style` is deprecated. Consider cloning it ' +
'beforehand. Check the `render` using <span>. Previous style: ' +
Expand Down

0 comments on commit 31d3bfa

Please sign in to comment.