React: Feature Request: innerHTML alternative

[DylanPiercey]

I think many people agree that the dangerouslySetInnerHTML={{ __html: ... }} api is gross even though there is great reasoning behind it.

I have a few issues with it beyond it's verbosity that I think could be added as static method on the react class.

My Issues with dangerouslySetInnerHTML:

1) Verbose and not always dangerous.
2) Can't mix safe and unsafe html.
3) Can't mix html with react elements.
4) Have to manually concat html strings.

---

A better solution would be to provide a way to mark html as "safe".

var React = require("react");

// Use #markSafe method to bypass reacts automatic html escaping.
var myReactEl = <div>{ React.markSafe("<br/>") }</div>;

This is still explicitly telling react that we trust the html but solves all of the problems above.

// Mixing safe and unsafe html.
(
    <div>
        { React.markSafe("<br/>") }
        { "<br/>" }
    </div>
);

// Mixing html with react elements.
(
    <div>
        { React.markSafe("<br/>") }
        <span>Hello!</span>
    </div>
);

// Multiple innerHTML sets.
(
    <div>
        { React.markSafe("<br/>") }
        { React.markSafe("<hr/>") }
    </div>
);

I think this api would be much friendlier than the current html api and probably wouldn't even require a major version bump.

Thoughts?

[jimfb]

At first glance, I like this API much better than our current API.

The function name would need a bit of bikeshedding. At the very least, it would need to live on ReactDOM. It should also be given a name that indicates that it's dangerous and that it is NOT making the HTML safe, but rather dangerously marking it as trusted.

cc @syranide @sebmarkbage Thoughts on such an API?

[DylanPiercey]

@jimfb I agree the function name could use some work. I think it should still be simpler than "dangerouslySetInnerHTML" though. I think something like trustHTML would be great.

[syranide]

@jimfb I think it's the "correct API", it was my intention to propose this some time ago so I'm :+1:. But it seems technically prohibitive to do, by using dangerouslyInnerHTML we guarantee that there's a single React container housing all the user nodes, which the user modifies outside of our control and that's fine, it's all isolated. But with this API user nodes can be mixed with React nodes (which is tricky and potentially fragile) and multiple user nodes can be adjacent and updated independently (which doesn't really seem solvable in all cases, EDIT: perhaps comments would work as delimiters actually). The HTML can also represent one or more root nodes (which can be text too) which complicates it even further.

But that doesn't mean we can't change the API to be <div>{...}</div> but allowing it only if it is the only child. Alternatively <div>{ReactDOM.safeHTML('div', attrs, innerHTML)}{ReactDOM.safeHTML('div', attrs, innerHTML)}</div> would also be workable (and you could even implement this today yourself in user-land), but it only makes it worse really.

EDIT: However, given our new render this might actually be realistic?

[brigand]

Is this proposal just a sugar wrapper around React.createElement, or does it do something differently?

Bikeshedding: the name implies it makes the html safe, which it doesn't, but that can be done.

[DylanPiercey]

@syranide could do something like wrapping the user html with two comment tags? I think the "allowing only one child" thing although still pretty looses all of the advantages I mentioned.

@brigand its different in that it allows user html to exist with react html within the same parent container. It's not something that is acheivable in userland since you must return a single react element.

[sthzg]

FWIW, in Django it's called mark_safe (Python has a snake case convention), which I like because it is short and clear. Regardless of the name, +1 for a change into that direction.

[DylanPiercey]

@sthzg I like mark_safe, I updated the example to markSafe to avoid all of the initial bikeshedding.

@syranide Although I initially agreed with ReactDOM.markSafeI think ergonomically it would be annoying. Typically "ReactDOM" is only required on root component's that need to render but this would require it anywhere html is to be trusted. I'm not really sure if it would make sense in the context of rendering outside of the DOM though since I don't have much/any mobile experience. Also markSafe is a more generic that safeHTML.

[brigand]

@DylanPiercey thanks for the clarification.

I like markSafe or markAsSafeHTML.

[syranide]

Not really my expertise, but it seems logical to me that the API would be ReactDOM.createHTMLFragment or something in that direction rather. It's no longer about marking HTML as safe, but creating a HTML data-type that is understood by React and rendered as such. But again, I haven't really followed the API discussions lately so it might not make sense for other reasons.

I.e. to put it differently, IMHO ReactDOM.createHTMLFragment(ReactDOM.safeHTML(...)) would make sense in context but perhaps not in practice.

[DylanPiercey]

@syranide I understand restricting the api to be on ReactDOM but what is the internal benefit of creating this fragment api you are proposing? Not very familiar with the internals of React but as an outsider that api is certainly uglier.

Having said that i'd still much rather have even the fragment api than the current dangerouslySetInnerHTML api for reasons 2-4 mentioned in the issue.

[syranide]

It's just a naming proposal, it conceptually replaces dangerouslySetInnerHTML, not the "safe" html-object. There may be ReactDOM.createComment in the future which may map to a special syntax in JSX, or it may need to be called "manually", don't know. It's also possible it will simply be mapped to a regular tag name, say "comment", "!comment" or w/e. It seems to me that HTML among children is the same problem and should thus be solved the same way.

If you want to preserve the "safeness", then that goes on-top of that API, and even though it may look gross I would still be OK with it as it's conceptually about casting a string to a HTML-object/string type which is then understood by createHTMLFragment which would return an element with an imaginary internal type of ReactDOMHTMLFragment.

[jquense]

The api here does seem nicer, naming-wise though it would be a mistake to move away from the "WATCH OUT!" nature of the current api's name. There is a lot of value in signally to users that an action potentially has risks. I like the approach Rust took with its unsafe keyword for outing out of the compiler guarantees for memory safety. The point is you are going to write Safe code inside there but it signals that the compiler isn't going to help you there.

Similarly for this, you are intentionally opting out of the safety provided by the React component model. The notion of danger in the current API is not signally that you _should_ put dangerous HTML there, only that you now CAN.

[sthzg]

@jquense I am wondering though if in a long term it wouldn't be nicer to keep such verbose warnings behind a warning that pops up in the developer tools as long as the app isn't built for production or the user opts out by some means (e.g. some option or flag, or an explicit import like Scala does it if you want to use experimental or advanced compiler features). One advantage of such an approach might be that the warning messages could be better articulated and more expressive than the name of a function/method might ever get.

[jimfb]

I just talked with @sebmarkbage about this, here are the results.

Overall, Sebastian indicated that the new API was probably ok, but a change should probably be low priority. His main concern was performance in the case where React nodes are siblings of the foreign markup (updating the foreign markup might require iterating over the nodes to delete them individually because you need to figure out which nodes are foreign and where the react siblings are located).

However, I now wonder if React should even have a dangerousHtml feature. Setting inner html is a very non-reacty pattern; it's clearly an escape hatch. An alternative is to attach a ref to a DOM node, and set the innerHTML of the DOM node. There is no fundamental reason this can't be handled entirely within userland.

[sthzg]

@jimfb Thanks for the update. Would the second paragraph also hold true for server side rendering? If I understand the approach it would be something like <div ref="foobar"/> and then e.g. in componentDidMount() getting the reference to foobar and setting the innerHTML programmatically. But that would always require a DOM environment, wouldn't it?

[syranide]

There is no fundamental reason this can't be handled entirely within userland.

@jimfb With the caveat that it must be wrapped in a container node, which aren't "transparent" (there is no node you can put that always work). If there one could insert document fragments into the DOM and keep them there I would agree, but you can't.

Realistically, setting inner html is a very non-reacty pattern; it's clearly an escape hatch.

AFAIK that was a large part of the initial sell, that you can leave React and it feels perfectly natural (many other frameworks can't). Also, I wouldn't label it an escape hatch, I would it interoperability.

Overall, Sebastian indicated that the new API was probably ok, but a change should probably be low priority. His main concern was performance in the case where React nodes are siblings of the foreign markup (updating the foreign markup might require iterating over the nodes to delete them individually because you need to figure out which nodes are foreign and where the react siblings are located).

IMHO, implementing this in-terms of #6263 (being able to have DOM nodes as children) in a way makes the most sense to me, then it would be predictable. It could even be an entirely user-land feature if we didn't care about SSR (sigh).

[jimfb]

Ah, thanks @sthzg, we had neglected to consider SSR. You're right, we need some sort of API!

I'm back in favor of this new API proposal.

[DylanPiercey]

Please keep SSR in mind :+1:

[ragebiswas]

+1 to this proposal. A place where this is incredibly useful is when you're getting some cooked HTML from the server and displaying inside a React component. Right now, I have no clean way of treating links inside such HTML to be caught by react-router, and I wish I could just dangerously-set <Link> components.

[syranide]

Now that we are using comment-nodes to delimit text-nodes, perhaps we could use the same principle but simply render raw HTML instead of text. If we replace everything between the comment nodes when the HTML is updated then it should be safe (SSR is always dangerous with malformed HTML, but that's already an issue).

[syranide]

7361 proof-of-concept implementation if anyone is curious

[gaearon]

If you’re interested in this please create an RFC so we can review a specific proposal: https://github.com/reactjs/rfcs

I’ll close the issue as it’s unlikely to progress without an RFC.