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

Setting className to null on an element renders an element with a class of "null" #31

Open
mnquintana opened this issue Aug 2, 2016 · 4 comments
Labels

Comments

@mnquintana
Copy link
Member

@mnquintana mnquintana commented Aug 2, 2016

Steps to Reproduce

  1. Return this JSX in your component's render function
<li className={null}></li>

Expected

The component renders <li></li>

Actual

The component renders <li class="null"></li>

@mnquintana mnquintana added the bug label Aug 2, 2016
@nathansobo
Copy link
Contributor

@nathansobo nathansobo commented Aug 2, 2016

This emerges from the way assigning a null className property works on DOM nodes. Try this in your console:

var div = document.createElement('div')
div.className = null
div

The result is as you described. I suppose we could create an exception for this where we completely remove the className from the properties hash if it is defined as null. However, what about the other properties? Should we omit any property with a null value? That seems like it would be surprising.

I can see how this behavior is annoying from a JSX perspective, but I'm unsure about introducing special-case behavior just for className or alternatively deleting any property the user assigns to null on a DOM node.

@BinaryMuse What do you think?

@BinaryMuse
Copy link
Contributor

@BinaryMuse BinaryMuse commented Aug 2, 2016

or alternatively deleting any property the user assigns to null on a DOM node

This is what I'd prefer (and expect) to see, given that it's not easy to conditionally include an attribute on a JSX tag... assuming virtual-dom handles it correctly (see a related React bug at facebook/react#1448 and the fix at facebook/react#1510). It seems that boolean properties (and some other properties) must be handled differently than just calling removeAttribute on them.

@nathansobo
Copy link
Contributor

@nathansobo nathansobo commented Aug 2, 2016

@BinaryMuse so you're in favor of deleting any property going to a normal DOM element that is null or undefined? Or just className?

@coreprocess
Copy link

@coreprocess coreprocess commented Jun 23, 2017

What about: any that is undefined?

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

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.