Skip to content

Commit

Permalink
Fix hydration of non-string dangerousSetInnerHTML.__html (#13353)
Browse files Browse the repository at this point in the history
* Consistently handle non-string dangerousSetInnerHTML.__html in SSR

* Add another test
  • Loading branch information
gaearon authored Aug 9, 2018
1 parent 0072b59 commit be4533a
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -485,16 +485,97 @@ describe('ReactDOMServerIntegration', () => {
expect(e.tagName).toBe('BUTTON');
});

itRenders('a div with dangerouslySetInnerHTML', async render => {
const e = await render(
<div dangerouslySetInnerHTML={{__html: "<span id='child'/>"}} />,
);
itRenders('a div with dangerouslySetInnerHTML number', async render => {
// Put dangerouslySetInnerHTML one level deeper because otherwise
// hydrating from a bad markup would cause a mismatch (since we don't
// patch dangerouslySetInnerHTML as text content).
const e = (await render(
<div>
<span dangerouslySetInnerHTML={{__html: 0}} />
</div>,
)).firstChild;
expect(e.childNodes.length).toBe(1);
expect(e.firstChild.nodeType).toBe(TEXT_NODE_TYPE);
expect(e.textContent).toBe('0');
});

itRenders('a div with dangerouslySetInnerHTML boolean', async render => {
// Put dangerouslySetInnerHTML one level deeper because otherwise
// hydrating from a bad markup would cause a mismatch (since we don't
// patch dangerouslySetInnerHTML as text content).
const e = (await render(
<div>
<span dangerouslySetInnerHTML={{__html: false}} />
</div>,
)).firstChild;
expect(e.childNodes.length).toBe(1);
expect(e.firstChild.nodeType).toBe(TEXT_NODE_TYPE);
expect(e.firstChild.data).toBe('false');
});

itRenders(
'a div with dangerouslySetInnerHTML text string',
async render => {
// Put dangerouslySetInnerHTML one level deeper because otherwise
// hydrating from a bad markup would cause a mismatch (since we don't
// patch dangerouslySetInnerHTML as text content).
const e = (await render(
<div>
<span dangerouslySetInnerHTML={{__html: 'hello'}} />
</div>,
)).firstChild;
expect(e.childNodes.length).toBe(1);
expect(e.firstChild.nodeType).toBe(TEXT_NODE_TYPE);
expect(e.textContent).toBe('hello');
},
);

itRenders(
'a div with dangerouslySetInnerHTML element string',
async render => {
const e = await render(
<div dangerouslySetInnerHTML={{__html: "<span id='child'/>"}} />,
);
expect(e.childNodes.length).toBe(1);
expect(e.firstChild.tagName).toBe('SPAN');
expect(e.firstChild.getAttribute('id')).toBe('child');
expect(e.firstChild.childNodes.length).toBe(0);
},
);

itRenders('a div with dangerouslySetInnerHTML object', async render => {
const obj = {
toString() {
return "<span id='child'/>";
},
};
const e = await render(<div dangerouslySetInnerHTML={{__html: obj}} />);
expect(e.childNodes.length).toBe(1);
expect(e.firstChild.tagName).toBe('SPAN');
expect(e.firstChild.getAttribute('id')).toBe('child');
expect(e.firstChild.childNodes.length).toBe(0);
});

itRenders(
'a div with dangerouslySetInnerHTML set to null',
async render => {
const e = await render(
<div dangerouslySetInnerHTML={{__html: null}} />,
);
expect(e.childNodes.length).toBe(0);
},
);

itRenders(
'a div with dangerouslySetInnerHTML set to undefined',
async render => {
const e = await render(
<div dangerouslySetInnerHTML={{__html: undefined}} />,
);
expect(e.childNodes.length).toBe(0);
},
);

describe('newline-eating elements', function() {
itRenders(
'a newline-eating tag with content not starting with \\n',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,40 @@ describe('ReactDOMServerIntegration', () => {
<div dangerouslySetInnerHTML={{__html: "<span id='child2'/>"}} />,
));

it('should error reconnecting a div with different text dangerouslySetInnerHTML', () =>
expectMarkupMismatch(
<div dangerouslySetInnerHTML={{__html: 'foo'}} />,
<div dangerouslySetInnerHTML={{__html: 'bar'}} />,
));

it('should error reconnecting a div with different number dangerouslySetInnerHTML', () =>
expectMarkupMismatch(
<div dangerouslySetInnerHTML={{__html: 10}} />,
<div dangerouslySetInnerHTML={{__html: 20}} />,
));

it('should error reconnecting a div with different object dangerouslySetInnerHTML', () =>
expectMarkupMismatch(
<div
dangerouslySetInnerHTML={{
__html: {
toString() {
return 'hi';
},
},
}}
/>,
<div
dangerouslySetInnerHTML={{
__html: {
toString() {
return 'bye';
},
},
}}
/>,
));

it('can explicitly ignore reconnecting a div with different dangerouslySetInnerHTML', () =>
expectMarkupMatch(
<div dangerouslySetInnerHTML={{__html: "<span id='child1'/>"}} />,
Expand Down
7 changes: 5 additions & 2 deletions packages/react-dom/src/client/ReactDOMFiberComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -987,9 +987,12 @@ export function diffHydratedProperties(
) {
// Noop
} else if (propKey === DANGEROUSLY_SET_INNER_HTML) {
const rawHtml = nextProp ? nextProp[HTML] || '' : '';
const serverHTML = domElement.innerHTML;
const expectedHTML = normalizeHTML(domElement, rawHtml);
const nextHtml = nextProp ? nextProp[HTML] : undefined;
const expectedHTML = normalizeHTML(
domElement,
nextHtml != null ? nextHtml : '',
);
if (expectedHTML !== serverHTML) {
warnForPropDifference(propKey, serverHTML, expectedHTML);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/react-dom/src/client/ReactDOMHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ export function shouldSetTextContent(type: string, props: Props): boolean {
typeof props.children === 'number' ||
(typeof props.dangerouslySetInnerHTML === 'object' &&
props.dangerouslySetInnerHTML !== null &&
typeof props.dangerouslySetInnerHTML.__html === 'string')
props.dangerouslySetInnerHTML.__html != null)
);
}

Expand Down

2 comments on commit be4533a

@fchienvuhoang
Copy link

Choose a reason for hiding this comment

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

πŸ‘

@fchienvuhoang
Copy link

Choose a reason for hiding this comment

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

πŸ‘ πŸ‘ πŸ‘

Please sign in to comment.