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

Empty string as attribute name causes exception. #141

Open Carlsson87 opened 5 years ago

Carlsson87 commented 5 years ago

I just upgraded to version 1.0.1 and ran in to problems when using Html.attribute "" "". It didn't use to cause an exception, and now it does. Let me know if you need more info.

SSCCE: https://ellie-app.com/3kjwsTXvQNTa1

norpan commented 5 years ago

Caused by https://github.com/elm/virtual-dom/commit/d32f9b4a2f6ceb1d30097818e39b33d3435bafe0

Probably fix would be to also check if attribute key is not empty, since that is not allowed.

harrysarson commented 5 years ago

Here is @Carlsson87's sscce:

module Main exposing (main)

import Html
import Html.Attributes

main =
    Html.div [ Html.Attributes.attribute "" "" ] [] 
Carlsson87 commented 5 years ago

To be extra specific. The problem seems to occur when the key is an empty string. The value being an empty string does not cause any problems for me.

evancz commented 5 years ago

Why are you doing this?

Carlsson87 commented 5 years ago

When I conditionally want to set an attribute on an element. I use the same approach with Html.text "" when I want to conditionally render some elements.

showButton : Maybe msg -> Html msg
showButton clickMsg =
    Html.button
        [ case clickMsg of
            Just msg ->
                Html.Events.onClick msg
            Nothing ->
                Html.Attributes.attribute "" ""
        ]
        [ Html.text "Click here"
        ]

I'd love to know if there is a better way.

hpate-omicron commented 5 years ago

We use a similar pattern for conditional attributes, we have it defined as

none : Attribute msg
none =
    Html.Attributes.property "" Json.Encode.null

Which seems to be valid still https://ellie-app.com/3ks2h7XJWnTa1

reiner-dolp commented 5 years ago

~@norpan how is d32f9b4 causing this bug?~ ah ok. I understand now. The bug was present before d32f9b4 for all cases Html.Attributes.attribute "" val except for val = "". Now using an empty string also triggers the bug.