rtfeldman / elm-css

Typed CSS in Elm.
https://package.elm-lang.org/packages/rtfeldman/elm-css/latest
BSD 3-Clause "New" or "Revised" License
1.23k stars 196 forks source link

Runtime type error when using CSS in an SVG element's attributes #543

Closed colinlogue closed 2 years ago

colinlogue commented 2 years ago

The following code compiles but throws Uncaught TypeError: Cannot set property className of #<SVGElement> which has only a getter in Chrome (and a similar error in Firefox):

module Main exposing (main)

import Html.Styled exposing (toUnstyled)
import Html.Styled.Attributes exposing (css)
import Svg.Styled exposing (svg)

main =
    svg [ css [] ] []
        |> toUnstyled

Ellie: https://ellie-app.com/fCySBLMNKcWa1

There is no error when importing the css function from Svg.Styled.Attributes.

Confidenceman02 commented 2 years ago

I got the exact same error when updating a side project to latest elm-css. Everything compiles fine but viewing in the browser I get the error mentioned.

My use case was -

import Svg.Styled.Attributes exposing (css)

svg
    [  viewBox "0 0 200 400"
    ,  css [  Css.width (Css.px 140)  ]
    ]
    []

I can understand why this looks dodgy as svg attributes like width tend to just be string values, not px. It was however working fine with versions <17.

My fix was to use the following -

 svg
    [  viewBox "0 0 200 400"
    ,  SvgAttribs.width (Css.num 140).value
    ]
    []

It worries me that the compiler doesn't protect against this. Will definitely need to go through projects and ensure everything still works.

Confidenceman02 commented 2 years ago

This seems far more serious than I thought actually. All of the css I have added to svg elements just blows up the browser when they are rendered.

I have had to work around this by adding styles to wrapping elements, including animations. Eeesh..

Confidenceman02 commented 2 years ago

Think this could be the issue. From what I can understand className is deprecated for svg elements. Seems this was introduced in 17.0.2 VDOM

Confidenceman02 commented 2 years ago

I have a working fork here https://github.com/Confidenceman02/elm-css.

All tests passing but not totally sure what the ramifications are when using class intead of className for all DOM elements. className will for sure not work with svg.

Confidenceman02 commented 2 years ago

After some talks with @robinheghan I tried out using VirtualDom.attribute and that does the job nicely. Will Open PR.

rtfeldman commented 2 years ago

Fixed in 17.0.3!