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

Make IE 11 not complain about non-crucial style attribute hydration mismatch #13534

Merged
merged 1 commit into from
Sep 4, 2018
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
61 changes: 61 additions & 0 deletions packages/react-dom/src/__tests__/ReactServerRenderingHydration.js
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,67 @@ describe('ReactDOMServerHydration', () => {
expect(element.firstChild.focus).not.toHaveBeenCalled();
});

it('should warn when the style property differs', () => {
const element = document.createElement('div');
element.innerHTML = ReactDOMServer.renderToString(
<div style={{textDecoration: 'none', color: 'black', height: '10px'}} />,
);
expect(element.firstChild.style.textDecoration).toBe('none');
expect(element.firstChild.style.color).toBe('black');

expect(() =>
ReactDOM.hydrate(
<div
style={{textDecoration: 'none', color: 'white', height: '10px'}}
/>,
element,
),
).toWarnDev(
'Warning: Prop `style` did not match. Server: ' +
'"text-decoration:none;color:black;height:10px" Client: ' +
'"text-decoration:none;color:white;height:10px"',
{withoutStack: true},
);
});

it('should not warn when the style property differs on whitespace or order in IE', () => {
document.documentMode = 11;
const element = document.createElement('div');

// Simulate IE normalizing the style attribute. IE makes it equal to
// what's available under `node.style.cssText`.
element.innerHTML =
'<div style="height: 10px; color: black; text-decoration: none;" data-reactroot=""></div>';

ReactDOM.hydrate(
<div style={{textDecoration: 'none', color: 'black', height: '10px'}} />,
element,
);

delete document.documentMode;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be in try-finally. Even if test fails you want to delete it to avoid breaking other tests

});

it('should warn when the style property differs on whitespace in non-IE browsers', () => {
const element = document.createElement('div');

element.innerHTML =
'<div style="text-decoration: none; color: black; height: 10px;" data-reactroot=""></div>';

expect(() =>
ReactDOM.hydrate(
<div
style={{textDecoration: 'none', color: 'black', height: '10px'}}
/>,
element,
),
).toWarnDev(
'Warning: Prop `style` did not match. Server: ' +
'"text-decoration: none; color: black; height: 10px;" Client: ' +
'"text-decoration:none;color:black;height:10px"',
{withoutStack: true},
);
});

it('should throw rendering portals on the server', () => {
const div = document.createElement('div');
expect(() => {
Expand Down
23 changes: 17 additions & 6 deletions packages/react-dom/src/client/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -999,12 +999,23 @@ export function diffHydratedProperties(
} else if (propKey === STYLE) {
// $FlowFixMe - Should be inferred as not undefined.
extraAttributeNames.delete(propKey);
const expectedStyle = CSSPropertyOperations.createDangerousStringForStyles(
nextProp,
);
serverValue = domElement.getAttribute('style');
if (expectedStyle !== serverValue) {
warnForPropDifference(propKey, serverValue, expectedStyle);

// IE 11 parses & normalizes the style attribute as opposed to other
// browsers. It adds spaces and sorts the properties in some
// non-alphabetical order. Handling that would require sorting CSS
// properties in the client & server versions or applying
// `expectedStyle` to a temporary DOM node to read its `style` attribute
// normalized. Since it only affects IE, we're skipping style warnings
// in that browser completely in favor of doing all that work.
// See https://github.com/facebook/react/issues/11807
if (!document.documentMode) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extract this to a top level variable like canDiffHydratedStyleForWarning

const expectedStyle = CSSPropertyOperations.createDangerousStringForStyles(
nextProp,
);
serverValue = domElement.getAttribute('style');
if (expectedStyle !== serverValue) {
warnForPropDifference(propKey, serverValue, expectedStyle);
}
}
} else if (isCustomComponentTag) {
// $FlowFixMe - Should be inferred as not undefined.
Expand Down