Warn when using dangerouslySetInnerHTML incorrectly

[zpao]

It's happened a few times resulting in confusion. Let's either warn or maybe allow a string (cc @syranide)

[syranide]

From my perspective it seems like the name should be enough to persuade lost souls to turn back. It's in no way easily guessed so the only way you'd know about it in the first place was if you had explicitly looked for it (but not necessarily read the docs). So to me it seems like {__html: ""} only serves to clutter and confuse a perfectly valid feature (and possibly presume it's broken, or not be a solution to your problem).

It is obviously technically very easy fix to allow both the old syntax and just strings, but perhaps you guys have "philosophical" reasons for wanting to keep it this way?

PS. I don't mind providing a PR, I just need to know if you're ok with sending plain strings.

[masonicboom]

How about making the syntax less awkward, but issuing a warning, by default, each time it is used, unless a "waiver" has been set (React.acceptFullConsequencesOfDangerouslySettingInnerHTML(true)) to disable the warning.

[syranide]

It's a noble thought, but it does not play well with 3rd party components.

Sent from IPhone

[sophiebits]

In addition, the point of the cumbersome name is to make you think each time you use it. Setting a global config option once won't have that effect.

[syranide]

This is an old issue btw, I'm not sure where we stand on this right now. Nobody seemed opposed to #1515. I guess if we really want to enlighten people we could put a warning in the console for the first use of dangerouslySetInnerHTML that details all the dangers and pitfalls, that way people would be unlikely to miss it, but perhaps it would just add to the console spam.

[zpao]

@sebmarkbage expressed some concern making it easier. We came to an arrangement, if only I could remember what it was...

[sebmarkbage]

By wrapping it in an object you can pass the result from blessed interfaces to unblessed interfaces.

We could however also introduce innerHTML that takes a string. That would be linted against at Facebook because we wouldn't allow anyone to do manual string manipulation without going through the appropriate security channels.

[zpao]

ah right, leave current interface, but also add innerHTML=string.

[sophiebits]

I don't see any advantage to adding innerHTML; people will invariably use it to introduce XSS holes.

[syranide]

unsafeInnerHTML or dangerousInnerHTML perhaps?

[sophiebits]

What's wrong with dangerouslySetInnerHTML? It sounds scary and it is.

[syranide]

Sounded like they wanted a different name, also naming-wise it uses Set which is quite weird as it is implied for all other props.