Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn for inline style mismatches #10084

Merged
merged 1 commit into from
Jul 1, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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