Preact: Warn on invalid HTML hierarchy

[jennydaman]

Example

https://github.com/jennydaman/preact-p-problem/blob/707dc6fd35c5422e610202ee7d40a2c3706a6b3e/src/index.js#L10-L18

Elements inside a JavaScript list which are rendered as children of a <p> have their HTML attributes dropped and put onto other elements of the same tag found below.

This bug only affects <p>, does not happen to <div>, <span>, <b>, nor <section>.

Result

Expected

preact-cli actually pre-renders it correctly. We can get the correct result by disabling JavaScript in the browser.

[marvinhagemeister]

That's very interesting! Thanks a bunch for reporting this issue and the excellent reproduction case :tada:

Had a quick look at it and it seems that the issue only occurs in production builds. In dev mode everything looks to be correct, making these even more strange. Will need to look further...

[marvinhagemeister]

Ok so we're in hydration mode and somehow the html in the browser is completely wrong:

<div>
  <p id="problem"></p>
  <div style="color: red">a</div>
  <p style="color: blue">b</p>
  <div style="color: green">c</div>
  <p></p>
  <i>Spring</i>
  <div>break</div>
  <div>is</div>
</div>

[marvinhagemeister]

Alright, I got it :tada: Initially I assumed that there must be a problem in preact-render-to-string but that's not the case. It renders the correct result.

Instead the weird behaviour is actually written down in the HTML spec itself. It states:

Tag omission in text/html:

A p element's end tag can be omitted if the p element is immediately followed by an address, article, aside, blockquote, details, div, dl, fieldset, figcaption, figure, footer, form, h1, h2, h3, h4, h5, h6, header, hgroup, hr, main, menu, nav, ol, p, pre, section, table, or ul element, or if there is no more content in the parent element and the parent element is an HTML element that is not an a, audio, del, ins, map, noscript, or video element, or an autonomous custom element.

So this means that this:

<p id="problem">
  <div style="color: red">a</div>
  <p style="color: blue">b</p>
  <div style="color: green">c</div>
</p>

will be turned into:

<p id="problem"></p>
<div style="color: red">a</div>
<p style="color: blue">b</p>
<div style="color: green">c</div>
<p></p>

At this point the DOM hierarchy has changed substantially and the <p>-tag doesn't wrap the colored children anymore.

What's worse is that the DOM hierarchy you're trying to render is invalid and browsers immediately enter the "quirks"-mode, where the browser tries to guess what the developer tried to achieve with the html document. This differs from browser to browser and is somewhat undefined behaviour. It's safe to say that the outcome is rarely what's expected.

The reason the HTML is invalid is because a p-tag does only allow phrasing content as children, which is defined here: https://html.spec.whatwg.org/multipage/dom.html#phrasing-content-2 Putting a div tag as a child of p is therefore invalid. Same as nesting p tag. That's invalid, too.

Following the guidelines of the spec we can use span elements as children:

export default class App extends Component {
  render() {
    return (
      <div>
        <p id="problem">
          {[
            <span style="color: red">a</span>,
            <span style="color: blue">b</span>,
            <span style="color: green">c</span>
           ]}
        </p>
        <i>Spring</i>
        <div>break</div>
        <div>is</div>
      </div>
    );
  }
}

Proof:

I'm wondering if there is a way for us to warn the user in preact/debug.

[jennydaman]

Interesting find, thank you for digging into it.

I had a hunch that it had to do with an oddity in the HTML spec, however I was unaware that I was misusing <p>. It is strange because this means the tag type of the parent actually matters.

[just-boris]

In case if this helps, here is how React checks the proper DOM nodes nesting: https://github.com/facebook/react/blob/0cf22a56a18790ef34c71bef14f64695c0498619/packages/react-dom/src/client/validateDOMNesting.js

[developit]

Definitely can't go the spec validation route here, so our options are basically limited to bailing out of non-corrective hydration when encountering tree mismatch. I have a WIP version of this locally I'll try to extract from my progressive hydration stuff.

[arty-name]

Looks like this issue still persists in 10.4.1. I encountered it when upgrading preact from 10.2.1, even made this minimal reproducible case.

[marvinhagemeister]

@arty-name This is not an issue in Preact, but of an invalid HTML hierarchy that the user told Preact to render. A div must not be placed inside a p element per the HTML spec. The spec specifically defines that weird behaviour incase a user does it regardless of if it's invalid HTML or not.

We've left this issue open because we would like to print a red warning sometime in the future via preact/debug. Again, this is an issue in the user code, not in Preact.

[arty-name]

Thank you for explaining Marvin! I also figured it out, but the warning is not printed yet, so I thought this minimal case might be useful. Maybe I just didn't want to throw it away :)

[marvinhagemeister]

Awesome, thanks for the sandbox! We'll check it out 👍