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

Don't self-close void elements, because invalid when using HTML5 #8923

Closed
jasongitmail opened this issue Jul 6, 2023 · 13 comments
Closed

Don't self-close void elements, because invalid when using HTML5 #8923

jasongitmail opened this issue Jul 6, 2023 · 13 comments

Comments

@jasongitmail
Copy link

The W3C validator complains about SvelteKit's built HTML, given it uses an HTML5 doc type.

w3c

These items clean up after removing the closing slash from sample HTML input.

Browsers also automatically convert it from that format into correct HTML format after interpreting it, but it'd be good to have prettier aligned with the correct version.

From MDN: https://developer.mozilla.org/en-US/docs/Glossary/Void_element "Self-closing tags (<tag />) do not exist in HTML. The void elements are as follows":

@dummdidumm
Copy link
Member

We're following Prettier's code style here which also does this. Where is that HTML output from, server side rendering with Svelte? In that case I think it would be better to adjust Svelte core to not do this.

@jasongitmail
Copy link
Author

jasongitmail commented Jul 6, 2023

I see it within the head of my app.html and between svelte:head elements on my main +layout.svelte. All my routes are prerendered, if that helps. (I haven't inspected any body html elements.)

@dummdidumm dummdidumm transferred this issue from sveltejs/prettier-plugin-svelte Jul 6, 2023
@henrikvilhelmberglund
Copy link

It is a bit strange that W3C validator complains about that since interacts badly with unquoted attribute values is only true if the / is next to the attribute which is never the case with Prettier formatting. After that it's just Trailing slash on void elements has no effect which is true I guess, not sure why the validator feels that I need to know that.

@hanszoons
Copy link
Contributor

This was on HN yesterday: https://news.ycombinator.com/item?id=36615691

@jasongitmail
Copy link
Author

From the post:

I think it's a confusing relic from a time past, and I don't think tools like Prettier should be pushing it. ... I respect Prettier's "our way or the highway" approach, but I don't think it's consistent here.

+1 thanks for sharing

@gtm-nayan
Copy link
Contributor

@jasongitmail Can you provide the source that produces these tags? It doesn't output the slash for me if I try to ssr <svelte:head><link rel="alternate" href="/" type="application/atom+xml" /></svelte:head>.

The first two warnings come from the kit templates which should be trivial to change.

@jasongitmail
Copy link
Author

@gtm-nayan I'm prerendering, not using SSR. That might be the difference there. This is Prettier related and Prettier wouldn't run on SSR-generated HTML. The code repro is to use any similar line from the screenshot in
<svelte:head></svelte:head> within a site's main +layout.svelte and export const prerender = true; in a sibling +layout.js in my case.

@gtm-nayan
Copy link
Contributor

gtm-nayan commented Jul 8, 2023

I also tried it on a prerendered page and it still didn't insert the slash. Are you using @html there anywhere or inserting the link tag in the app.html or in transformPageChunk?

@jasongitmail
Copy link
Author

jasongitmail commented Jul 8, 2023

@gtm-nayan No, I'm not. This is about the prettier config, not the output, so you can see what we're talking about in the editor. On Stackblitz, you can repro with:

  1. Create a new SK project--click this Stackblitz. Notice the self closing tags in app.html.
  2. In src/routes/+layout.svelte, add the following and save:
<svelte:head>
	<meta name="description" content="Foo bar" /> <!-- intentionally added slash -->
	<link rel="canonical" href="https://example.com" > <!-- intentionally no slash -->
</svelte:head>
  1. Stackblitz doesn't format on save, so run npm run format. Inspect both +layout.svelte and app.html and you'll see Prettier keeps any existing closing /, in our link and meta elements in this case, and adds it where we intentionally left it out.

This demonstrates what Prettier is doing for both .html and .svelte files.


It's not necessary to build it to see what Prettier is doing. But as a side note, if you're looking at browser's Elements tab in dev console, you're not looking at the HTML string sent over the wire. The Elements tab shows parsed HTML based on the doc type, and for SK's HTML5 doc type will remove the self close on items we're talking about because it's not supposed to exist. But, visit your SK website in Chrome and hit "command+option+u" (Mac) and you can see the HTML sent over the wire, including the trailing slashes added by Prettier.

@gtm-nayan
Copy link
Contributor

Hmm, I thought the warnings were for the HTML in the prerendered HTML files. Prettier shouldn't have any effect on those. Do you have a link to the page where this occurs?

@Rich-Harris
Copy link
Member

Apart from possibly cleaning up https://github.com/sveltejs/kit/blob/main/packages/create-svelte/templates/skeleton/src/app.html, this doesn't seem like a Svelte issue at all — the rendered HTML is correct. If Prettier is subsequently running on that HTML and introducing errors, that's a Prettier bug. So I'm pretty sure this issue can be closed?

That said, I'm glad this was opened because that's how I learned that Svelte incorrectly parses self-closing non-void HTML elements. I've opened #11052 to address that.

@SelenIT
Copy link

SelenIT commented Apr 6, 2024

Technically, trailing slashes in void HTML elements are valid in HTML5. The validator doesn't actually complain about them (these messages aren't errors or even warnings, they are marked as just info), it only tries to tell the author that the meaning of this code might be different from what is intended (e.g. that these slashes don't make these elements "self-closed").

@jasongitmail
Copy link
Author

@Rich-Harris Yes, I think this can be closed, since there does not seem to be a config option exposed by Prettier to be changed.

The Prettier repo has an open issue for this same topic, if anyone wants to follow it there. The comment thread links to a plugin that can clean this up until Prettier does so officially (prettier-plugin-void-html).

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

No branches or pull requests

7 participants