Void elements render without a closing /

[preactjs]

Void elements render without a closing /

[developit]

@elliotdavies would that let us drop the whitelist of self-closing elements? If so, I'm 100% in favor of always outputting / for void elements. Happy to merge that PR if you're good to create it.

[elliotdavies]

@developit Not that I can see – you still need some way to know whether to self-close e.g. <br/> or instead do a full closing tag e.g. <div>...</div>, and I think a whitelist is the only reliable way to check that.

Still, this should be a pretty minimal change. I'll write up the PR and you can take a look.

[developit]

Works for me.

[Download]

Lol I was just changing the tests for preact-helmet to take this difference into account but it looks like I'm going to have to be changing them back pretty soon...

[developit]

@Download that's odd - how come you had to change the tests? If you're rendering JSX to strings in tests, I'd recommend using preact-render-to-string/jsx - it preserves non-string props.

[Download]

@developit
I am porting react-helmet to preact. It's tests tend to render some component, then check that the output matches some literal string.

I noticed two big differences with the output from Preact vs the one expected in the tests:

1. React outputs ="true" for boolean properties, but Preact does not (I actually prefer Preact's more concise style). E.g. <meta data-preact-helmet="true" ... /> vs <meta data-preact-helmet ... />
2. React self-closes elements that Preact does/did not. E.g: <base href="https://localhost"/> vs <base href="https://localhost">

There also seem to be some differences between the output of render (when using innerHTML or outerHTML to serialize to String) and that of renderToString. Again nothing serious but it would be better/simpler for testing if the results would be more consistent. It seems render does render the self-closing tag for example. And it's only renderToString that omits the ="true" for booleans.

Personally I like the more concise form; it's more readable (imho) and it saves precious bytes. :)

[developit]

@Download render() + innerHTML would actually return / depending on if the document was XHTML or HTML.

[Download]

@developit Yes I think it's just renderToString which makes it shorter. The test is using/testing both forms. But it's also most important for renderToString to be short (to get low TTI). So even though it is annoying when testing that both Strings are subtly different, I think it is an advantage for Preact to keep it this way.

It might be interesting to have a test that just renders all kinds of markups in both ways and compare. Only differences that are beneficial should stay (or maybe give renderToString a flag to toggle).

[developit]

Flag to toggle would be good, that way it has the option to return either XHTML or HTML.

[elliotdavies]

Hi @developit – just wondering if there's any chance you could bump the release version so as to include this?

[developit]

Can do, yup.

[developit]

Released as 3.6.0!

[elliotdavies]

Thanks!

[brandonpittman]

When using htm, the auto slash feature doesn't seem to work. Unclosed elements return undefined in the output.

[marvinhagemeister]

@brandonpittman Just to clarify: Are you using preact-render-to-string in combination with htm?

[brandonpittman]

@marvinhagemeister Yep.

[marvinhagemeister]

@brandonpittman Can you share a snippet or a codesandbox which shows the problem?

[marvinhagemeister]

@brandonpittman Just spoke with @jviide which is the maintainer of htm. Unclosed void elements are not supported in htm and therefore will lead to undefined behaviour. In htm you must always use a closing /.

[brandonpittman]

@marvinhagemeister Yeah, I figured that out on my own. Wasted a lot of time figuring it out though. ☹️

[marvinhagemeister]

@brandonpittman Did you import preact/debug somewhere? That's where all our debug warnings are and it should catch undefined vnodes. It only needs to be included once (best at the top of your app, before rendering starts).