Deterministic output order

[andys8]

Issue

Having conflicting properties can lead to non-deterministic results. min-width is set twice. This will have the same css specificity. This is most likely a mistake of the user. The issue is: The result looked fine in dev mode (with create-elm-app), but was broken in production. The precedence changed.

Dev

Prod

Solution

Since the names seem to be deterministic, I think it is the order the classes are printed into the resulting styles. If this is the case, sorting them by name would work.

https://github.com/rtfeldman/elm-css/blob/6c0972d55d8d4046086991eddb78dcfa80b1acce/src/Css/Structure/Output.elm#L7-L15

[rtfeldman]

Interesting!

Sorting the classes before emitting would presumably address this, but at a performance cost. We could also de-dupe the styles by storing them in a dictionary rather than a list, also at a performance cost.

What I'm wondering is: why does the output order differ between development and production? I can't think of a reason why that should be true. The only thing I can think of is https://github.com/elm/compiler/issues/1836 maybe?

[andys8]

I was using create-elm-app. So there are differences between start and build: Different Webpack configuration, hot module reloading (?), --debug, --optimize.

The issue you mentioned is highly interesting. If --optimize would be the cause, sorting lists wouldn't make a difference. For Evan on the other side it's probably surprising someone would depend on the order of style definitions.

[rtfeldman]

Hm, what happens if you keep everything else about the configuration the same, but remove --optimize? Is it still reproducible then?

[andys8]

I'm trying to limit the cause, but I'm currently not able to reproduce it. As of now I can't be sure, that the identical code was used for the example above.

As you can see the class names were the same. Can you give me insights what code changes could change class name order?

[rtfeldman]

Can you give me insights what code changes could change class name order?

That's the thing - I literally can't think of any. 😅

The generated class name is a pure function of the Style values involved, and the output is determined by iterating through the virtual dom nodes, concatenating all their styles, and emitting that to a <style> element.

The elm-css code involved should be exactly the same in development versus production, and I'm not aware of any tools that can affect that.

In general, it should be true that any Elm code built in development and in production should do exactly the same thing in both environments—the only different should be that code built with --optimize should be smaller and run faster. That's why a compiler bug with --optimize is the only potential culprit I can think of.

[andys8]

Yeah, I'm sorry, as long as I can't reproduce it, in a useful way, it doesn't make a lot of sense to investigate.

I noticed, the styles list depends on what is actually shown. The diffs between the styles of multiple pages seem to just add and remove classes. Is there a merge strategy when the multiple elements use the same "class" (inline styles)?

So let's have this imaginary example: Can you think of the scenario, that there are already two conflicting styles resulting on two classes. And there is a code change somewhere, which triggers a different order or output? Can you think of situations?

[rtfeldman]

Is there a merge strategy when the multiple elements use the same "class" (inline styles)?

Sorry, I don't quite follow - why would a merge be necessary?

elm-css should generate exactly one class per element, no matter what. If two elements have the same class, that means their styles are identical.

[andys8]

One class per element? This could be a hint in the right direction. So this is the actual part with the issue.

viewRightButton : ( Asset, msg ) -> Html msg
viewRightButton ( asset, msg ) =
    iconButton asset
        [ css
            [ minWidth (px 13)
            , minHeight (px 13)
            , marginLeft (px 5)
            ]
        ]
        msg

iconButton : Asset -> List (Html.Attribute msg) -> msg -> Html msg
iconButton (Asset asset) attributes msg =
    let
        styles =
            [ minWidth (px 48)
            , minHeight (px 48)
            , cursor pointer
            , outline none
            , border zero
            , backgroundColor transparent
            , backgroundRepeat noRepeat
            , backgroundPosition center
            , backgroundImage (url asset)
            , opacity (num 0.6)
            , transition [ Css.Transitions.opacity 500 ]
            , hover [ opacity (num 1) ]
            , disabled
                [ cursor notAllowed
                , opacity (num 0.2)
                ]
            ]
    in
    Html.button (attributes ++ [ onClick msg, css styles ]) []

Sidenote: Overwriting the values minWidth and minHeight without important doesn't work this way. But this is not what the issue is about. The issue is about what could change the precedence change.

[rtfeldman]

Okay, so that's definitely a bug! It should for sure only be one class per element.

Do you think you could boil that down to a SSCCE?

[andys8]

What about the issue https://github.com/rtfeldman/elm-css/issues/415? It has a 0.18 example, but sounds pretty much like the "multi class" issue, we have here. SSCCE of that issue: https://ellie-app.com/9P7wNngFQa1/1

[andys8]

Minimal example for two classes on one element:

module Main exposing (main)

import Browser
import Css exposing (..)
import Html.Styled as Html exposing (Html, button, text, toUnstyled)
import Html.Styled.Attributes exposing (css)


button1 =
    Html.button
        [ css [ cursor pointer ]
        , css [ border zero ]
        ]
        [ text "button" ]


main =
    button1 |> toUnstyled

https://ellie-app.com/429JgfpfjZsa1

[pdamoc]

Minimal example for two classes on one element:

wrapping the code in a div solves this problem

div [] [ button1 ] |> toUnstyled