Skip to content

Document justification for dangerouslySetInnerHTML, fixes #2256#2518

Merged
jimfb merged 1 commit into
facebook:masterfrom
jimfb:documentation-for-dangerouslySetInnerHtml
Jan 22, 2015
Merged

Document justification for dangerouslySetInnerHTML, fixes #2256#2518
jimfb merged 1 commit into
facebook:masterfrom
jimfb:documentation-for-dangerouslySetInnerHtml

Conversation

@jimfb
Copy link
Copy Markdown
Contributor

@jimfb jimfb commented Nov 13, 2014

I put this into the "tips" section. Perhaps this should be in the "reference" section - let me know if you have a preference.

@browniefed
Copy link
Copy Markdown

Thoughts on also mentioning that the HTML must be formed correctly? At one point we had an issue with truncated HTML that was missing some closing div tags. We had to eventually create a document fragment and set the innerhtml so the browser would fix it then get the innerhtml again and pass it to dangerous set inner html.
This a total personal problem but based on the name I assumed dangerouslySetInnerHtml would be able to handle poorly formed HTML.

@jimfb
Copy link
Copy Markdown
Contributor Author

jimfb commented Nov 14, 2014

Cool, I added a sentence saying the HTML passed in should be well formed / pass validation (which implicitly requires tags be closed, since that's required for validation).

@jimfb jimfb force-pushed the documentation-for-dangerouslySetInnerHtml branch from 84371f9 to d3b4d5a Compare November 14, 2014 02:50
@browniefed
Copy link
Copy Markdown

👍 @JSFB awesome that looks good to me

@jimfb jimfb force-pushed the documentation-for-dangerouslySetInnerHtml branch from d3b4d5a to 290b670 Compare November 15, 2014 01:44
@jimfb
Copy link
Copy Markdown
Contributor Author

jimfb commented Nov 15, 2014

Modified to better explain the reasoning behind {__html}

Comment thread docs/tips/18-dangerouslySetInnerHTML.md Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

innerHTML (and probably use ` to codeify it)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@jimfb jimfb force-pushed the documentation-for-dangerouslySetInnerHtml branch from 290b670 to 78dade6 Compare November 26, 2014 23:13
@jimfb
Copy link
Copy Markdown
Contributor Author

jimfb commented Jan 5, 2015

@zpao Ping. Ready to merge?

@jimfb jimfb force-pushed the documentation-for-dangerouslySetInnerHtml branch from 78dade6 to dbdf7d4 Compare January 20, 2015 20:11
@jimfb
Copy link
Copy Markdown
Contributor Author

jimfb commented Jan 20, 2015

@zpao Ping. Since we've been waiting on this one for months, merge conflicts have appeared, which have been resolved. Can we get this one merged?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is this link broken now?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch, link changed in previous CR change, fixed.

@sophiebits
Copy link
Copy Markdown
Collaborator

lgtm otherwise

Conflicts:
	docs/_data/nav_tips.yml
	docs/tips/17-children-undefined.md
@jimfb jimfb force-pushed the documentation-for-dangerouslySetInnerHtml branch from dbdf7d4 to 6f44f60 Compare January 21, 2015 20:48
jimfb added a commit that referenced this pull request Jan 22, 2015
…nerHtml

Document justification for dangerouslySetInnerHTML, fixes #2256
@jimfb jimfb merged commit 54b565d into facebook:master Jan 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants