Skip to content

Fix hydration of non-string dangerousSetInnerHTML.__html #13353

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

Merged
merged 2 commits into from
Aug 9, 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
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