Skip to content
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.

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

Switching from dangerouslySetInnerHTML to children #1232

Closed
kcarnold opened this issue Mar 7, 2014 · 17 comments · Fixed by #4427
Closed

Switching from dangerouslySetInnerHTML to children #1232

kcarnold opened this issue Mar 7, 2014 · 17 comments · Fixed by #4427

Comments

@kcarnold
Copy link

kcarnold commented Mar 7, 2014

I tripped on a strange error when, in one state, a component uses HTML to specify children (from rendering Markdown) and in another state it uses child components (for interactive content, itself with some inner HTML). I reduced the testcase down to this: http://jsfiddle.net/BCp36/1/

@syranide
Copy link
Contributor

syranide commented Mar 7, 2014

It only happens when __html: '' is empty too.

@kcarnold It may be good to know that AFAIK the recommended way of dealing with children is div({}, child1, child2, [listOfChildren]), etc.

@kcarnold
Copy link
Author

kcarnold commented Mar 7, 2014

If __html is not empty, you don't get an exception, but the first child element goes missing: http://jsfiddle.net/BCp36/2/

If I use the way you recommend (which I'd prefer!), I get a warning: Each child in an array should have a unique "key" prop. Check the render method of undefined. e.g., http://jsfiddle.net/BCp36/3/

@sophiebits
Copy link
Collaborator

Thanks for reporting. I verified that #1157 doesn't fix this (even though it fixes another bug that produces the same error message).

@sophiebits
Copy link
Collaborator

This happens because we enqueue a removal of the children, then set the HTML before the queued removal happens. When the queue is processed, the element is missing which confuses React. If we queue the innerHTML set as well then this will be fixed.

@kcarnold As a workaround for this bug, you can assign a different key prop to each div and React will create a new element instead of reusing the existing one.

@syranide
Copy link
Contributor

syranide commented Oct 8, 2014

@spicyj Do you know if this is fixed in master? I think it is...

@sophiebits
Copy link
Collaborator

No, it's still broken (but it throws a slightly different error).

@steadicat
Copy link

Just spent a few hours chasing this bug down in our codebase. Would be good to fix this soon, before someone else runs into this. I can repro in all versions of React from 0.9 to 0.12. (This is what I ended up btw: http://jsfiddle.net/yw2t9hcm/)

@mattmo
Copy link

mattmo commented Apr 7, 2015

Any update on this issue? We're running into a similar problem.

@Zenwolf
Copy link

Zenwolf commented Apr 7, 2015

I also ran into this issue recently. I had to wrap the child in a <span> to fix the bug.

@ahmetabdi
Copy link

Issue still occurring for me

sophiebits added a commit to sophiebits/react that referenced this issue Jul 20, 2015
With this, ReactMultiChild handles all of the children-related operations for ReactDOMComponent so that we don't process operations out of order. This is necessary because ReactMultiChild does its own batching so there's no way without its cooperation to get the timing right here.

Ideally we'll factor this logic out a bit better in subsequent updates but this is the simplest way to fix facebook#1232 which has embarrassingly been open for over a year.
sophiebits added a commit that referenced this issue Jul 20, 2015
Fix switching between dangerouslySetInnerHTML and children
@juztinlazaro
Copy link

Hi guys, got an issue on dangerouslySetInnerHTML my problem is it decode the HTML entities but it doesn't apply the style or properties.

@jimfb
Copy link
Contributor

jimfb commented Dec 7, 2015

@juztinlazaro If you believe you've found a bug in the React core, please open a new issue and provide a jsfiddle (simplified testcase/example) that demonstrates the bug.

@juztinlazaro
Copy link

@jimfb thanks sir, here's my https://jsfiddle.net/up8j9aqz/1/

@jimfb
Copy link
Contributor

jimfb commented Dec 7, 2015

@juztinlazaro It doesn't decode the entities. It puts them into the markup, directly (that's what dangerouslySetInnerHTML does). The browser sees the entities and renders them as text (that's what browsers do). This is not a bug. I modified your fiddle to do what I believe you wanted: https://jsfiddle.net/b5Lt87kb/

@juztinlazaro
Copy link

@jimfb oh sorry, okay i understand now, newbie here. Okay maybe i need to decode it first before using dangerously. thanks sir.

@jimfb
Copy link
Contributor

jimfb commented Dec 7, 2015

@juztinlazaro Ok, for future reference, usage questions are better answered on StackOverflow, as we try to keep github issues for tracking bugs in the React core.

@juztinlazaro
Copy link

@jimfb okay thanks sir.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants