search

elm / virtual-dom

The foundation of HTML and SVG in Elm.
https://package.elm-lang.org/packages/elm/virtual-dom/latest
BSD 3-Clause "New" or "Revised" License
209 stars 80 forks source link

property "innerHTML" doesn't work #131

Closed wildlyinaccurate closed 6 years ago

wildlyinaccurate commented 6 years ago

In elm-lang/virtual-dom@2.0.4, you could do the following:

node "div" [ property "innerHTML" (Json.string "This is the innerHTML property") ] []

and the output would be

<div>This is the innerHTML property</div>

However in elm/virtual-dom@1.0.0, this does not work and the output is just

<div></div>

I can see that this is on purpose. Is there an alternative way to set a node's innerHTML property? Something similar to React's dangerouslySetInnerHTML?

wildlyinaccurate commented 6 years ago

Sorry, if I had played around for a minute longer I would have realised that Elm.Kernel.VirtualDom.property is available in userland!

evancz commented 6 years ago

Sorry, if I had played around for a minute longer I would have realised that Elm.Kernel.VirtualDom.property is available in userland!

What do you mean by this? There should not be a thing like dangerouslySetInnerHTML

wildlyinaccurate commented 6 years ago

@evancz I mean that the following code sets the innerHTML property as I would expect:

node "div" [ Elm.Kernel.VirtualDom.property "innerHTML" (Json.string "This is the innerHTML property") ] []

This is a useful feature for me; I use it to insert HTML generated from Markdown into the page.

rtfeldman commented 6 years ago

This is a useful feature for me

@wildlyinaccurate This is definitely not an intentional feature, but rather a recently discovered compiler bug, and I would strongly recommend against relying on it! Once the bug is fixed, that code will definitely stop working.

I've seen several people on Elm Slack who were previously using innerHTML and who have found other solutions that will continue working once this bug is fixed. I'd recommend asking in the #general channel what has worked for people!

wildlyinaccurate commented 6 years ago

Thanks @rtfeldman. I discussed this with some of the folks in Slack, and it seems like the rationale behind this change is to prevent packages with potential XSS vulnerabilities from being published. Is that correct?

I think there's a balance to be struck between safety and developer friction. There are plenty of cases where it is perfectly safe to set innerHTML and the overhead of a custom element or an HTML parser is not appropriate.

Could we meet in the middle by creating a Elm.VirtualDom.dangerouslySetProperty function? This could be a "banned import" in a prepublish check.

evancz commented 6 years ago

We had a bunch of discussion about the tradeoffs in this thread https://github.com/elm/html/issues/172. It addresses the specific concerns and recommendation you mention here.