Skip to content

[Fiber] Fix to render falsy value as dangerouslySetInnerHTML #8652

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.

Sign up for GitHub

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
gaearon merged 2 commits into from
Jan 11, 2017
Merged

[Fiber] Fix to render falsy value as dangerouslySetInnerHTML #8652

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
46d3e81
[Fiber] Fix to render falsy value as dangerouslySetInnerHTML
koba04 Dec 29, 2016
d1ec906
Add more cases and rename test for clarity
gaearon Jan 11, 2017
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
1 change: 1 addition & 0 deletions scripts/fiber/tests-passing.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,7 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
* should allow named slot projection on both web components and regular DOM elements
* should skip reserved props on web components
* should skip dangerouslySetInnerHTML on web components
* should render null and undefined as empty but print other falsy values
* should remove attributes
* should remove properties
* should properly update custom attributes on custom elements
Expand Down
11 changes: 4 additions & 7 deletions src/renderers/dom/fiber/ReactDOMFiberComponent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,13 +398,10 @@ function updateDOMProperties(
} else if (propKey === DANGEROUSLY_SET_INNER_HTML) {
var nextHtml = nextProp ? nextProp[HTML] : undefined;
var lastHtml = lastProp ? lastProp[HTML] : undefined;
if (nextHtml) {
if (lastHtml) {
if (lastHtml !== nextHtml) {
setInnerHTML(domElement, '' + nextHtml);
}
} else {
setInnerHTML(domElement, nextHtml);
// Intentional use of != to avoid catching zero/false.
if (nextHtml != null) {
Copy link
Contributor Author

@koba04 koba04 Jan 6, 2017

Choose a reason for hiding this comment

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

Does this condition still need? It seems to be unnecessary because it is covered by if (lastHtml !== nextHtml).

Copy link
Collaborator

@acdlite acdlite Jan 9, 2017

Choose a reason for hiding this comment

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

Hmm idk why that branch was there. Seems ok to remove it.

if (lastHtml !== nextHtml) {
setInnerHTML(domElement, '' + nextHtml);
}
} else {
// TODO: It might be too late to clear this if we have children
Expand Down
40 changes: 40 additions & 0 deletions src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,46 @@ describe('ReactDOMComponent', () => {
).toBe(false);
});

it('should render null and undefined as empty but print other falsy values', () => {
var container = document.createElement('div');

ReactDOM.render(
<div dangerouslySetInnerHTML={{__html: 'textContent'}} />,
container
);
expect(container.textContent).toEqual('textContent');

ReactDOM.render(
<div dangerouslySetInnerHTML={{__html: 0}} />,
container
);
expect(container.textContent).toEqual('0');

ReactDOM.render(
<div dangerouslySetInnerHTML={{__html: false}} />,
container
);
expect(container.textContent).toEqual('false');

ReactDOM.render(
<div dangerouslySetInnerHTML={{__html: ''}} />,
container
);
expect(container.textContent).toEqual('');

ReactDOM.render(
<div dangerouslySetInnerHTML={{__html: null}} />,
container
);
expect(container.textContent).toEqual('');

ReactDOM.render(
<div dangerouslySetInnerHTML={{__html: undefined}} />,
container
);
expect(container.textContent).toEqual('');
});

it('should remove attributes', () => {
var container = document.createElement('div');
ReactDOM.render(<img height="17" />, container);
Expand Down