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

Why self-closing-comp allows empty divs? #572

Closed
alex35mil opened this issue Apr 25, 2016 · 10 comments
Closed

Why self-closing-comp allows empty divs? #572

alex35mil opened this issue Apr 25, 2016 · 10 comments

Comments

@alex35mil
Copy link

I noticed that self-closing-comp doesn't report empty divs (eg <div></div>). Is there any particular reason that it allows empty html tags? Or it's valid b/c it's valid html?

@ljharb ljharb added the bug label Apr 25, 2016
@ljharb
Copy link
Member

ljharb commented Apr 25, 2016

Seems like a bug to me. That should be <div />.

@alex35mil
Copy link
Author

@ljharb I'd be sure it is, if there wasn't this in the doc:

screen shot 2016-04-25 at 21 56 39

@ljharb
Copy link
Member

ljharb commented Apr 25, 2016

That doesn't make much sense to me - but if it's documented, then it's not a bug.

However, I'd love to see an option added to this rule that requires self-closing tags for every kind of tag, including HTML tags. @yannickcr?

@yannickcr
Copy link
Member

This is the intended behavior. This rule only enforce empty React Components to be self-closed, and HTML tags are not concerned.

I think we could modify this rule to check all type of tags by default, then add some options to filter by type (components and/or HTML tags) if needed.

It seems to be the more logical behavior to me, but it will be a breaking change.

@ljharb
Copy link
Member

ljharb commented Apr 25, 2016

Airbnb's React styleguide requires every empty element to be self-closed.

Alternatively, we could add an option in a minor version, and then flip the default behavior later, in a major?

@gitim
Copy link
Contributor

gitim commented May 14, 2016

Have the same issue. Is anybody already working on it? If not I'll prepare a pull request, which will add the option suggested by @ljharb and @yannickcr to the rule.

@GeoffGodwin
Copy link

Just wanted to leave my comment in here for future developers reaching this page. <div/> is not a valid HTML 4.x tag. It's the only base HTML tag (that I know of) that cannot be legally self-closing. If you use a <div/> it can cause very strange issues that can bubble up through a variety of frameworks (looking at you jQuery) making it difficult to trace as it does not throw distinct errors. One particular example being in IE8 where it breaks the DOM tree fairly silently.

It is always better practice to use <div></div> even if no content exists between the tags.

As a note, <div/> is legitimate XHTML 1.x. However for the sake of compatibility across browsers and versions there is no reason to use it.

I realize this is ooooold world HTML but it's still very valid today. Cheers!

@ljharb
Copy link
Member

ljharb commented Jun 26, 2018

jsx isn’t quite html.

@GeoffGodwin
Copy link

GeoffGodwin commented Jun 26, 2018

No arguments there, I just figure underlying it may be trying to generate a real DOM <div/> which would then cause problems. I haven't gone through the project to know how it's generating the actual DOM element so for all I know it's properly making <div></div> but I figured this was a good place to drop the knowledge in anyway.

@ljharb
Copy link
Member

ljharb commented Jun 26, 2018

lol, React generates proper HTML, that's not something anyone needs to worry about :-)

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

No branches or pull requests

5 participants