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

Fix hydration of non-string dangerousSetInnerHTML.__html #13353

Merged
merged 2 commits into from Aug 9, 2018

Conversation

@gaearon
Copy link
Member

@gaearon gaearon commented Aug 9, 2018

Fixes #11789.

With this PR, non-string dangerousInnerHTML.__html is now handled during hydration the same way as we handle it during a pure client-only or server-only render. Specifically, we coerce any value __html value that isn't null or undefined to a string. Previously, hydration behaved inconsistently.

These are the same tests run against on master:

screen shot 2018-08-09 at 5 54 52 pm

You can see that the only ones that were failing are related to hydration. This tells us that the PR doesn't change the behavior for purely client-only or server-only renders, but it fixes the inconsistency when hydrating on top of good markup.

@@ -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')
typeof props.dangerouslySetInnerHTML.__html != null)
Copy link
Member

@trueadm trueadm Aug 9, 2018

Choose a reason for hiding this comment

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

I don't think you intended to keep the typeof here?

Loading

Copy link
Member Author

@gaearon gaearon Aug 9, 2018

Choose a reason for hiding this comment

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

No. Also there are other reasons why this doesn't work. Will look again.

Loading

trueadm
trueadm approved these changes Aug 9, 2018
Copy link
Member

@trueadm trueadm left a comment

LGTM minus the typo (I think)

Loading

@gaearon gaearon changed the title Consistently handle non-string dangerousSetInnerHTML.__html on hydration Fix false positive warnings when hydrating non-string dangerousSetInnerHTML.__html Aug 9, 2018
@gaearon gaearon changed the title Fix false positive warnings when hydrating non-string dangerousSetInnerHTML.__html Fix hydration of non-string dangerousSetInnerHTML.__html Aug 9, 2018
@gaearon gaearon merged commit be4533a into facebook:master Aug 9, 2018
1 check was pending
Loading
TejasQ pushed a commit to TejasQ/react that referenced this issue Aug 26, 2018
)

* Consistently handle non-string dangerousSetInnerHTML.__html in SSR

* Add another test
@gaearon gaearon mentioned this pull request Sep 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants