Skip to content

Conversation

@sophiebits
Copy link
Collaborator

This is a better version of #644. Fixes #101.

@ghost ghost assigned sebmarkbage Dec 31, 2013
sophiebits added a commit to sophiebits/react that referenced this pull request Dec 31, 2013
Fixes facebook#767. This essentially reverts 738de8c.

We could store some sort of flag to silence the console error here but since we've made significant improvements in markup wrapping, this error is fairly rare now. We'll also have validation of node structure soon in facebook#735.
@sophiebits
Copy link
Collaborator Author

@sebmarkbage Just updated with a pretty complete validateNodeNesting function.

@syranide
Copy link
Contributor

Got notified by locks in the IRC-channel, https://www.irccloud.com/pastebin/16CNdpLZ

Someone else came across it (and I think this PR is the fix?).

@sophiebits
Copy link
Collaborator Author

Yeah, this will warn when putting a <tr> directly in a <table>.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is always an array, right? You can just use .forEach and .join. No need for the added dependencies and extra helper module

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case of dangerouslySetInnerHTML it's a string. I can get rid of that and make it an array always if you prefer.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(@sebmarkbage Just checking in on this.)

@markijbema
Copy link
Contributor

👍 Just ran into this while playing around with React myself. Pretty sure our production code got this problem as well ;)

@zpao
Copy link
Member

zpao commented Mar 12, 2014

After talking with @sebmarkbage, I'm going to close this out since I think we don't want to do this this way. There's work being done to make this possible without inspecting markup.

@zpao zpao closed this Mar 12, 2014
@mikemorton
Copy link

I just wanted to throw in my two cents that I came across this issue today as a react n00b and would've been completely lost without this post from @spicyj I found on the google group. I'm not sure how high of a priority this kind of a warning is but it is helpful to people who are new to the project.

@syranide
Copy link
Contributor

@mikemorton If #1550 is agreed upon then it should provide us with the ability to cheaply warn/throw when the browser mucks with the DOM.

sophiebits added a commit to sophiebits/react that referenced this pull request Mar 20, 2015
Nicer version of facebook#644 and facebook#735. Fixes facebook#101. Context is neat.
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

Successfully merging this pull request may close these issues.

React should throw on nested <p> tags

6 participants