Open lydell opened 2 years ago
This seems like a bigger issue... both elm/html and elm/svg use type aliases of a single Attribute
type found in elm/virtual-dom
. Long-term it seems like it should be fixed in all of these packages -- so, maybe file an issue for better attribute types in elm/virtual-dom
?
Feel free to create such an issue, but shorter-term this issue is still useful!
I know that this package is called HTML extra – not SVG extra – but unfortunately the Elm compiler allows using for example
attributeIf
on an SVG element, so it’s easy to make mistakes.Here’s the problem:
attributeIf
usesempty
if the condition is False.empty
is implemented asHtml.Attributes.classList []
.classList
is implemented in terms ofclass
, soHtml.Attributes.classList []
is equivalent toHtml.Attributes.class ""
. In other words,element.className = ""
will be executed in JS. As documented that’s fine in practice.Except in SVG.
.className
is a read-only property on SVG elements. Elm adds'use strict';
to its JS output, and in strict mode trying to write to a read-only property throws an error.Me and two colleagues recently spent two hours hunting down why Elm’s virtual DOM crashed. No external JS, no browser extensions, just Elm. After commenting out half of the code repeatedly, we found the culprit:
attributeIf
on an SVG element. Really hard to find since the code above is not even clear that there’s any SVG involved!Worse, we didn’t notice this during development. We use
parcel
(v1), and it seems to remove the'use strict';
so no error was thrown. Until we tried to deploy production.So, what do you think? Is there a way we could try to make these functions less nuclear?
Requirements:
empty
should not cause unwanted effectsempty
should not cause performance problemsIdeas:
empty = Html.Attributes.style "elmEmptyAttribute" ""
. After quick check it looks like it’s hard to tell thatelement.style.elmEmptyAttribute = ""
has been called, which is good!empty = Html.Attributes.property "__elmEmptyAttribute" (Json.Encode.string "")
.element.__elmEmptyAttribute
will be readable afterwards, but does it matter? Can it affect performance?Needs more investigation, but it could work!
Somewhat related: https://github.com/elm/html/issues/141 and https://github.com/elm/html/issues/232