Skip to content

Commit

Permalink
Warn for inline style mismatches
Browse files Browse the repository at this point in the history
I use the technique of generating a style string and comparing that against the
attribute.
  • Loading branch information
sebmarkbage committed Jul 1, 2017
1 parent 79868a9 commit 14b4866
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 5 deletions.
7 changes: 7 additions & 0 deletions scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1531,6 +1531,13 @@ src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js
* should error reconnecting missing attributes
* should error reconnecting added attributes
* should error reconnecting different attribute values
* should error reconnecting missing style attribute
* should error reconnecting added style attribute
* should error reconnecting empty style attribute
* should error reconnecting added style values
* should error reconnecting different style values
* should reconnect number and string versions of a number
* should error reconnecting reordered style values
* should error reconnecting different text
* should reconnect a div with a number and string version of number
* should error reconnecting different numbers
Expand Down
8 changes: 7 additions & 1 deletion src/renderers/dom/fiber/ReactDOMFiberComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -915,7 +915,13 @@ var ReactDOMFiberComponent = {
} else if (propKey === STYLE) {
// $FlowFixMe - Should be inferred as not undefined.
extraAttributeNames.delete(propKey);
// TOOD: Validate style sheets.
const expectedStyle = CSSPropertyOperations.createDangerousStringForStyles(
nextProp,
);
serverValue = domElement.getAttribute('style');
if (expectedStyle !== serverValue) {
warnForPropDifference(propKey, serverValue, expectedStyle);
}
} else if (
isCustomComponentTag ||
DOMProperty.isCustomAttribute(propKey)
Expand Down
36 changes: 34 additions & 2 deletions src/renderers/dom/shared/CSSPropertyOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,6 @@ var CSSPropertyOperations = {
serialized += dangerousStyleValue(
styleName,
styleValue,
component,
isCustomProperty,
);

Expand All @@ -217,6 +216,40 @@ var CSSPropertyOperations = {
return serialized || null;
},

/**
* This creates a string that is expected to be equivalent to the style
* attribute generated by server-side rendering. It by-passes warnings and
* security checks so it's not safe to use this value for anything other than
* comparison. It is only used in DEV for SSR validation. This is duplicated
* from createMarkupForStyles because createMarkupForStyles is expected to
* move out of the client-side renderer and it would be nice to make a clean
* break.
*/
createDangerousStringForStyles: function(styles) {
if (__DEV__) {
var serialized = '';
var delimiter = '';
for (var styleName in styles) {
if (!styles.hasOwnProperty(styleName)) {
continue;
}
var styleValue = styles[styleName];
if (styleValue != null) {
var isCustomProperty = styleName.indexOf('--') === 0;
serialized += delimiter + hyphenateStyleName(styleName) + ':';
serialized += dangerousStyleValue(
styleName,
styleValue,
isCustomProperty,
);

delimiter = ';';
}
}
return serialized || null;
}
},

/**
* Sets the value for multiple styles on a node. If a value is specified as
* '' (empty string), the corresponding style property will be unset.
Expand All @@ -240,7 +273,6 @@ var CSSPropertyOperations = {
var styleValue = dangerousStyleValue(
styleName,
styles[styleName],
component,
isCustomProperty,
);
if (styleName === 'float') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2075,6 +2075,44 @@ describe('ReactDOMServerIntegration', () => {
expectMarkupMismatch(<div id="foo" />, <div id="bar" />));
});

describe('inline styles', function() {
it('should error reconnecting missing style attribute', () =>
expectMarkupMismatch(<div style={{width: '1px'}} />, <div />));

it('should error reconnecting added style attribute', () =>
expectMarkupMismatch(<div />, <div style={{width: '1px'}} />));

it('should error reconnecting empty style attribute', () =>
expectMarkupMismatch(
<div style={{width: '1px'}} />,
<div style={{}} />,
));

it('should error reconnecting added style values', () =>
expectMarkupMismatch(
<div style={{}} />,
<div style={{width: '1px'}} />,
));

it('should error reconnecting different style values', () =>
expectMarkupMismatch(
<div style={{width: '1px'}} />,
<div style={{width: '2px'}} />,
));

it('should reconnect number and string versions of a number', () =>
expectMarkupMatch(
<div style={{width: '1px', height: 2}} />,
<div style={{width: 1, height: '2px'}} />,
));

it('should error reconnecting reordered style values', () =>
expectMarkupMismatch(
<div style={{width: '1px', fontSize: '2px'}} />,
<div style={{fontSize: '2px', width: '1px'}} />,
));
});

describe('text nodes', function() {
it('should error reconnecting different text', () =>
expectMarkupMismatch(<div>Text</div>, <div>Other Text</div>));
Expand Down
3 changes: 1 addition & 2 deletions src/renderers/dom/shared/dangerousStyleValue.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,9 @@ var isUnitlessNumber = CSSProperty.isUnitlessNumber;
*
* @param {string} name CSS property name such as `topMargin`.
* @param {*} value CSS property value such as `10px`.
* @param {ReactDOMComponent} component
* @return {string} Normalized style value with dimensions applied.
*/
function dangerousStyleValue(name, value, component, isCustomProperty) {
function dangerousStyleValue(name, value, 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 Down

0 comments on commit 14b4866

Please sign in to comment.