Filter Json.Decode.keyValuePairs with hasOwnProperty

[lsjroberts]

If I wish to decode target.childNodes[...].textContent I would expect the following to work:

childrenContentDecoder : Decode.Decoder (List String)
childrenContentDecoder =
    Decode.at [ "target", "childNodes" ] (Decode.keyValuePairs textContentDecoder)
        |> Decode.map (\nodes -> List.map (\( key, text ) -> text) nodes)

textContentDecoder : Decode.Decoder String
textContentDecoder =
    Decode.field "textContent" Decode.string

However, since target.childNodes is an instance of NodeList this silently fails due to it having some inherited props, such as .length, .entries etc, in addition to the expected indices.

(Unless I'm missing something) the solution to this in Elm is:

childrenContentDecoder : Decode.Decoder (List String)
childrenContentDecoder =
    Decode.at [ "target", "childNodes" ] (Decode.keyValuePairs textContentDecoder)
        |> Decode.map
            (\nodes ->
                nodes
                    |> List.filterMap
                        (\( key, text ) ->
                            case String.toInt key of
                                Err msg ->
                                    Nothing

                                Ok value ->
                                    Just text
                        )
                    |> List.reverse
            )

textContentDecoder : Decode.Decoder String
textContentDecoder =
    Decode.oneOf [ Decode.field "textContent" Decode.string, Decode.succeed "nope" ]

This is rather convoluted, and indeed since no errors were thrown I had to step debugger through Elm itself to discover the cause. I believe doing a .hasOwnProperty check would solve this for the common use case where the expected values are all of the same type.

(The List.reverse is a possibly unrelated curiosity, but does appear to be required in all my tests).

(This may be worth a separate issue, but using Decode.list instead of Decode.keyValuePairs also resulted in a silent error).

I can provide a full demo app if required.

[process-bot]

Thanks for the pull request! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

[process-bot]

Thanks for the pull request! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

[mgold]

+1, this is stupid JS garbage. Whitespace diff is instructive: append ?w=1 to the URL.

[mgold]

+1, this is stupid JS garbage. Whitespace diff is instructive: append ?w=1 to the URL.

[rtfeldman]

Yep, this seems like the right thing to do here. 👍

Screenshot of the diff without whitespace, per Max's comment:

[rtfeldman]

Yep, this seems like the right thing to do here. 👍

Screenshot of the diff without whitespace, per Max's comment:

[lsjroberts]

Just rebasing from master to get the elm-test fixes and put the brace on a new line.

[lsjroberts]

Just rebasing from master to get the elm-test fixes and put the brace on a new line.

[evancz]

Thank you for the suggestion! The code moved around and changed a bit for 0.19 so I added the check myself.

[evancz]

Thank you for the suggestion! The code moved around and changed a bit for 0.19 so I added the check myself.