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

Adjacent text nodes in SSR should have comments between them despite components #11109

Merged
merged 4 commits into from Oct 5, 2017

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Oct 5, 2017

This fixes #10598.
See individual commits and test cases I added.

This ensures we don't reset it when we exit components that return strings.
This fixes the {'a'}</div>{'b'} case and prevents an unnecessary comment before 'b'.
@reactjs-bot
Copy link

Deploy preview ready!

Built with commit cd2df6c

https://deploy-preview-11109--reactjs.netlify.com

@gaearon gaearon mentioned this pull request Oct 5, 2017
19 tasks
@gaearon gaearon requested a review from nhunzaker October 5, 2017 13:45
Copy link
Collaborator

@sophiebits sophiebits left a comment

Choose a reason for hiding this comment

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

lgtm. thank you!

@gaearon gaearon merged commit 32ec797 into facebook:master Oct 5, 2017
@gaearon gaearon deleted the fix-ssr-text-node branch October 6, 2017 20:48
@@ -827,6 +830,7 @@ class ReactDOMServerRenderer {
frame.debugElementStack = [];
}
this.stack.push(frame);
this.previousWasTextNode = false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

So is this unnecessary now? Seems like the fix was the second commit, not this, no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If I remember it right I tried commenting both places out separately after adding the tests, and both were necessary. This one resets when we're entering a tag, and another resets when we're leaving a tag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Returning plain string from component renders collapsed text nodes on server
5 participants