8000 React should throw on nested <p> tags · Issue #101 · facebook/react · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

React should throw on nested <p> tags #101

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.

Sign up for GitHub

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

Jump to bottom
Closed
petehunt opened this issue Jun 17, 2013 · 12 comments · Fixed by #3467
Closed

React should throw on nested <p> tags #101

petehunt opened this issue Jun 17, 2013 · 12 comments · Fixed by #3467
Labels
Component: DOM

Comments

@petehunt
Copy link
Contributor

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
Copy link
Collaborator

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

@petehunt
Copy link
Contributor Author

√ react has no idea though

@sophiebits
Copy link
Collaborator

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
Copy link
Contributor

👍 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
Copy link
Collaborator

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
Copy link
Collaborator

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

@sophiebits sophiebits mentioned this issue Aug 27, 2013
Closed
7 tasks
@zpao zpao mentioned this issue Sep 1, 2013
Closed
@brianr
Copy link
Contributor
brianr commented Sep 29, 2013

@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.

sophiebits added a commit to sophiebits/react that referenced this issue Dec 8, 2013
@sophiebits
Validate node nesting structure
67a19c6 
This solution feels a little gross to me, but I haven't been able to come up with anything better.

Fixes facebook#101.
@sophiebits sophiebits mentioned this issue Dec 8, 2013
Closed
@sophiebits sophiebits mentioned this issue Dec 29, 2013
Closed
@xixixao
Copy link
Contributor
xixixao commented Jan 2, 2014

+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
Copy link
Contributor
xixixao commented Jan 5, 2014

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.

sophiebits added a commit to sophiebits/react that referenced this issue Feb 12, 2014
@sophiebits
Validate node nesting structure
da9193c 
This is a better version of facebook#644. Fixes facebook#101.
This was referenced Jun 8, 2014
Closed
Closed
@sophiebits sophiebits mentioned this issue Aug 6, 2014
Closed
@syranide
Copy link
Contributor

Related #1987

@sophiebits sophiebits mentioned this issue Sep 29, 2014
Closed
@zpao
Copy link
Member
zpao commented Jan 8, 2015

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.

@zpao zpao closed this as completed Jan 8, 2015
@syranide
Copy link
Contributor
syranide commented Jan 9, 2015

@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.

@syranide syranide mentioned this issue Jan 21, 2015
Closed
@sophiebits sophiebits mentioned this issue Mar 20, 2015
Merged
sophiebits added a commit to sophiebits/react that referenced this issue Mar 20, 2015
@sophiebits
Validate node nesting, take 3
f9abf49 
Nicer version of facebook#644 and facebook#735. Fixes facebook#101. Context is neat.
bvaughn added a commit to bvaughn/react that referenced this issue Aug 13, 2019
@bvaughn
Merge pull request facebook#101 from gaearon/find-nit
0decb17 
Pressing next forces search to select
taneliang referenced this issue in MLH-Fellowship/react Aug 17, 2020
@taneliang
[Improve base View][3/n] Add _ to views' private vars and methods (#101)
8b3cc2e
taneliang referenced this issue in MLH-Fellowship/react Aug 19, 2020
@taneliang
[Improve base View][3/n] Add _ to views' private vars and methods (#101)
ec3383b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Component: DOM
Projects
None yet
Milestone
No milestone
7 participants
@sophiebits @zpao @brianr @tomocchino @petehunt @xixixao @syranide
0