React should throw on nested <p> tags

[petehunt]

Browsers do special things on self-closing tags like <p>:

http://jsfiddle.net/bbrGq/1/

React should warn or throw if it detects a nested React.DOM.p.

[sophiebits]

(Inline elements are allowed inside a <p>; other <p> tags and block-level elements are not.)

[petehunt]

√ react has no idea though

[sophiebits]

React also does badly in this case with things like text nodes inside <table> elements (outside of a cell). It ends up throwing the error:

Uncaught Error: The framework has attempted to either insert zero or multiple markup roots into a single location when it should not. This is a serious error - a fault of the framework - please report immediately.

which is confusing and doesn't help anyone. This happens because in dangerouslyReplaceNodeWithMarkup, setting innerHTML to <table>oops!</table> gives two child nodes: oops! and <table></table>.

Demo: http://jsfiddle.net/spicyj/ENxaB/

We should work on better messaging here.

[tomocchino]

👍 to better error messages.

I think this could get into dangerous territory quickly, though @petehunt. Pretty soon we'll want full-on child validation (like we have in XHP).

[sophiebits]

React also gets confused by <tbody>. :(

When you click on the table at http://jsfiddle.net/Gzm5N/1/, it should produce output of:

Row 0
Row 1
Row 2
...
Row 2
Row 1
Row 0

but doesn't because

dangerouslyInsertMarkupAt(table, "<tr>...</tr>", i)

inserts a <tbody><tr>...</tr></tbody> instead. :(

jQuery has a hardcoded list of places where it does stuff differently https://github.com/jquery/jquery/blob/master/src/manipulation.js#L12-L30 so maybe we need the same.

[sophiebits]

(Tables still broken in React 0.4: http://jsfiddle.net/spicyj/Gzm5N/2/)

[brianr]

@spicyj The tables issue appears to be fixed, at least partially, in master. See this jsfiddle: http://jsfiddle.net/6pBnK/2/

It looks like commit fixed it: adffa9b

There's still some weirdness because the original <tr>s are inside a <tbody> and the rest are directly in the <table>, but the order is correct.

[xixixao]

+1, I had non-dangerous looking:

render: ->
  _div {},
    _p _h2 'Source'
    _p {},
      _textarea cols: 152, rows: 6, onChange: @handleChange
    _p {},
      _button onClick: @translate, 'Traslate'
      _button onClick: @run, 'Run'
    _p _h2 'Translation'
    _p {},
      _pre @state.result

(blindly copied from some old html). Somehow baking @spicyj 's comment above into the error messages would be useful.

[xixixao]

Just ran into the table case (oh boy that's hard to debug!), simply by not using tbody (penalty for learning HTML before people started actually using tbody). Looking forward to 735.

[syranide]

Related #1987

[zpao]

I'm just going to close this out. We left it open a long time but we haven't made any progress apart from improved error messages. It's still a hard problem to solve in a reasonable way, so I'm calling it off.

[syranide]

@zpao Like I mentioned in my "no data-reactid"-PR, it is possible to catch this immediately as it occurs really quite cheaply (so it's a good fit for DEV at the very least) and it's just a few lines of code as well. Considering that PROD-behavior would stay lenient, the DEV/PROD inconsistency shouldn't be an issue either.