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

Remove empty value for boolean attributes in SSR #11708

Merged

Conversation

aweary
Copy link
Contributor

@aweary aweary commented Nov 29, 2017

Turns out this was super easy @gaearon!

For boolean attributes and overloaded boolean attributes with a value of true we can just render the attribute name and save people a few bytes.

I'm pretty sure this is safe for all attribute/value pairs that would hit this path.

Copy link
Contributor

@nhunzaker nhunzaker left a comment

Choose a reason for hiding this comment

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

Wow. Very cool!

Copy link
Contributor

@jquense jquense left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Collaborator

@gaearon gaearon left a comment

Choose a reason for hiding this comment

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

ok

@aweary aweary merged commit e0c3113 into facebook:master Nov 29, 2017
@aweary aweary deleted the remove-empty-value-boolean-attributes branch November 29, 2017 18:00
madeinfree pushed a commit to madeinfree/react that referenced this pull request Nov 30, 2017
@gaearon
Copy link
Collaborator

gaearon commented Dec 7, 2017

@aweary Do you mind reverting this? Apparently it was done intentionally: #2842. We can argue it may not be an important use case to support but we can't drop the support in the middle of a major.

@aweary aweary restored the remove-empty-value-boolean-attributes branch December 7, 2017 16:37
aweary pushed a commit to aweary/react that referenced this pull request Dec 7, 2017
aweary pushed a commit to aweary/react that referenced this pull request Dec 7, 2017
aweary pushed a commit to aweary/react that referenced this pull request Dec 7, 2017
gaearon pushed a commit that referenced this pull request Dec 7, 2017
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.

5 participants