elmish / hmr

Hot Module Replacement for Elmish apps
https://elmish.github.io/hmr
Other
28 stars 9 forks source link

Bool expressions are unexpectedly false when a module is reloaded. #42

Closed davidbruce closed 6 months ago

davidbruce commented 1 year ago

Description

Bool expressions are returning false unexpectedly.

For example the following will always return false when reloaded with HMR:

prop.classes [if state.Filter = Completed then "is-active"]

As a workaround I'm having to use match instead:

 prop.classes [(match state.Filter with | Completed -> "is-active" | _ -> "")]

I noticed this issue when trying out HMR on my Todo App from the Elmish Book and the filter tabs were not getting the "is-active" class.

let filterTabs (state: State) (dispatch: Msg -> unit) =
    div ["tabs"; "is-toggle"; "is-fullwidth"] [
        Html.ul [
            Html.li [
                prop.classes [(match state.Filter with | All -> "is-active" | _ -> "")]
                // prop.classes [if highlight = "All" then "is-active"]
                prop.onClick (fun _ -> dispatch ShowAll)
                prop.children [
                    Html.a [
                        prop.text "All"
                    ]
                ]
            ]
            Html.li [
                prop.classes [(match state.Filter with | Completed -> "is-active" | _ -> "")]
                // prop.classes [if state.Filter = Completed then "is-active"]
                prop.onClick (fun _ -> dispatch ShowCompleted)
                prop.children [
                    Html.a [
                        prop.text "Completed"
                    ]
                ]
            ]
            Html.li [
                prop.classes [(match state.Filter with | NotCompleted -> "is-active" | _ -> "")]
                // prop.classes [if state.Filter = NotCompleted then "is-active"]
                prop.onClick (fun _ -> dispatch ShowNotCompleted)
                prop.children [
                    Html.a [
                        prop.text "Not Completed!"
                    ]
                ]
            ]
        ]
    ]

Repro code

module App

open Elmish
open Elmish.React
open Feliz
open System
open Elmish.HMR

type Todo = {
    Id: Guid 
    Description: string
    Completed: bool
}

type TodoBeingEdited = {
    Id: Guid 
    Description: string
}

type Filter =
    | All
    | Completed
    | NotCompleted

type State = {
    TodoList: Todo list
    NewTodo: string
    TodoBeingEdited: TodoBeingEdited option
    Filter: Filter
}

type Msg = 
    | SetNewTodo of string
    | AddNewTodo
    | ToggleCompleted of Guid 
    | DeleteTodo of Guid
    | StartEditingTodo of Guid 
    | SetEditedDescription of string
    | CancelEdit
    | ApplyEdit
    | ShowAll
    | ShowCompleted
    | ShowNotCompleted

let init(): State = {
    TodoList = [ 
        { Id = Guid.NewGuid(); Description = "Learn F#"; Completed = true } 
        { Id = Guid.NewGuid(); Description = "Learn Elmish"; Completed = false } 
    ]
    NewTodo = ""
    TodoBeingEdited = None
    Filter = All
}

let update msg state = 
    match msg with
    | SetNewTodo todoText -> { state with NewTodo = todoText }
    | AddNewTodo when state.NewTodo = "" -> state
    | AddNewTodo ->
        {
            state with
                NewTodo = ""
                TodoList = List.append state.TodoList [
                    { 
                        Id = Guid.NewGuid()
                        Description = state.NewTodo
                        Completed = false
                    }
                ]
        }
    | DeleteTodo todoId -> 
        {
            state with TodoList = 
                        state.TodoList
                        |> List.filter (fun todo -> todo.Id <> todoId) 
        }
    | ToggleCompleted todoId ->
        {
            state with TodoList = 
                        state.TodoList
                        |> List.map (fun todo ->
                                if todo.Id = todoId 
                                then { todo with Completed = not todo.Completed } 
                                else todo
                            )
        }
    | StartEditingTodo todoId -> 
        {
            state with TodoBeingEdited = 
                        state.TodoList
                        |> List.tryFind (fun todo -> todo.Id = todoId)
                        |> Option.map (fun todoBeingEdited -> { Id = todoId; Description = todoBeingEdited.Description })
        }
    | SetEditedDescription newText ->
        {
            state with TodoBeingEdited =
                        state.TodoBeingEdited 
                        |> Option.map (fun todoBeingEdited -> { todoBeingEdited with Description = newText })
        }
    | ApplyEdit ->
        { 
            state with 
                TodoBeingEdited = None;
                TodoList = 
                        match state.TodoBeingEdited with 
                        | None -> state.TodoList 
                        | Some todoBeingEdited when todoBeingEdited.Description = "" -> state.TodoList
                        | Some todoBeingEdited -> 
                                    state.TodoList
                                    |> List.map (fun todo -> if todo.Id = todoBeingEdited.Id 
                                                                    then { todo with Description = todoBeingEdited.Description} 
                                                                    else todo ) 
        }
    | CancelEdit -> { state with TodoBeingEdited = None}
    | ShowAll -> { state with Filter = All }
    | ShowCompleted -> { state with Filter = Completed }
    | ShowNotCompleted -> { state with Filter = NotCompleted }

let appTitle = 
    Html.p [
        prop.className "title"
        prop.text "Elmish To-Do List"
    ]

let inputField (state: State) (dispatch: Msg -> unit) =
    Html.div [
        prop.classes [ "field"; "has-addons" ]
        prop.children [
            Html.div [
                prop.classes [ "control"; "is-expanded" ]
                prop.children [
                    Html.input [
                        prop.classes [ "input"; "is-medium" ]
                        prop.valueOrDefault state.NewTodo
                        prop.onChange (SetNewTodo >> dispatch)
                        prop.onKeyPress (key.enter, fun event -> dispatch AddNewTodo )
                    ]
                ]
            ]

            Html.div [
                prop.className "control"
                prop.children [
                    Html.button [
                        prop.classes [ "button"; "is-primary"; "is-medium" ]
                        prop.onClick (fun _ -> dispatch AddNewTodo)
                        prop.children [
                            Html.i [
                                prop.classes [ "fa"; "fa-plus" ]
                            ]
                        ]
                    ]
                ]
            ]
        ]
    ]

let div (classes: string list) (children: Fable.React.ReactElement list) =
    Html.div [
        prop.classes classes
        prop.children children
    ]

let renderTodoEdit (todoBeingEdited: TodoBeingEdited) (dispatch: Msg -> unit) =
    div [ "box" ] [
        div [ "columns"; "is-mobile"; "is-vcentered" ] [
            div [ "column" ] [
                div [ "control"; "is-expanded" ] [
                    Html.input [
                        prop.classes [ "input"; ]
                        prop.valueOrDefault todoBeingEdited.Description 
                        prop.onChange (SetEditedDescription >> dispatch)
                        prop.onKeyDown (fun event -> 
                                                match event.key with 
                                                | "Enter" -> dispatch ApplyEdit 
                                                | "Escape" -> dispatch CancelEdit 
                                                | _ ->  ())
                    ]
                ]
            ]
            div [ "column"; "is-narrow" ] [
                div [ "buttons"] [
                    Html.button [
                        prop.classes [ "button"; "is-success"]
                        prop.onClick (fun _ -> dispatch (ApplyEdit))
                        prop.children [
                            Html.i [ prop.classes [ "fa"; "fa-floppy-disk" ] ]
                        ]
                    ]

                    Html.button [
                        prop.classes [ "button"; "is-danger" ]
                        prop.onClick (fun _ -> dispatch CancelEdit)
                        prop.children [
                            Html.i [ prop.classes [ "fa"; "fa-times" ] ]
                        ]
                    ]
                ] 
            ]
        ]
    ]

let renderTodo (todo: Todo) (dispatch: Msg -> unit) =
    div [ "box" ] [
        div [ "columns"; "is-mobile"; "is-vcentered" ] [
            div [ "column" ] [
                Html.p [
                    prop.style [ if todo.Completed then style.textDecoration.lineThrough ]
                    prop.className "subtitle"
                    prop.text todo.Description
                ]
            ]

            div [ "column"; "is-narrow" ] [
                div [ "buttons"] [
                    Html.button [
                        prop.classes [ "button"; if todo.Completed then "is-success"]
                        prop.onClick (fun _ -> dispatch (ToggleCompleted todo.Id))
                        prop.children [
                            Html.i [ prop.classes [ "fa"; "fa-check" ] ]
                        ]
                    ]

                    Html.button [
                        prop.classes [ "button"; ]
                        prop.onClick (fun _ -> dispatch (StartEditingTodo todo.Id))
                        prop.children [
                            Html.i [ prop.classes [ "fa"; "fa-pencil" ]]
                        ]
                    ]

                    Html.button [
                        prop.classes [ "button"; "is-danger" ]
                        prop.onClick (fun _ -> dispatch (DeleteTodo todo.Id))
                        prop.children [
                            Html.i [ prop.classes [ "fa"; "fa-trash-can" ] ]
                        ]
                    ]
                ] 
            ]
        ]
    ]

let todoList (state: State) (dispatch: Msg -> unit) = 
    Html.ul [
        prop.children [
            let todos = 
                state.TodoList
                |> List.filter (fun todo -> 
                    match state.Filter with 
                        | Completed -> todo.Completed = true 
                        | NotCompleted -> todo.Completed <> true 
                        | All -> true 
                )
            for todo in todos ->
                    match state.TodoBeingEdited with 
                    | Some todoBeingEdited when todo.Id = todoBeingEdited.Id -> renderTodoEdit todoBeingEdited dispatch
                    | _ -> renderTodo todo dispatch
        ]
    ]

let filterTabs (state: State) (dispatch: Msg -> unit) =
    div ["tabs"; "is-toggle"; "is-fullwidth"] [
        Html.ul [
            Html.li [
                prop.classes [(match state.Filter with | All -> "is-active" | _ -> "")]
                // prop.classes [if highlight = "All" then "is-active"]
                prop.onClick (fun _ -> dispatch ShowAll)
                prop.children [
                    Html.a [
                        prop.text "All"
                    ]
                ]
            ]
            Html.li [
                prop.classes [(match state.Filter with | Completed -> "is-active" | _ -> "")]
                // prop.classes [if state.Filter = Completed then "is-active"]
                prop.onClick (fun _ -> dispatch ShowCompleted)
                prop.children [
                    Html.a [
                        prop.text "Completed"
                    ]
                ]
            ]
            Html.li [
                prop.classes [(match state.Filter with | NotCompleted -> "is-active" | _ -> "")]
                // prop.classes [if state.Filter = NotCompleted then "is-active"]
                prop.onClick (fun _ -> dispatch ShowNotCompleted)
                prop.children [
                    Html.a [
                        prop.text "Not Completed!"
                    ]
                ]
            ]
        ]
    ]

let render (state: State) (dispatch: Msg -> unit) =
    Html.div [
        prop.style [ style.padding 20 ]
        prop.children [
            appTitle
            inputField state dispatch
            filterTabs state dispatch
            todoList state dispatch
        ]
    ]

Program.mkSimple init update render
|> Program.withReactSynchronous "elmish-app"
|> Program.run

Expected and actual results

Using if statements with HMR the is-active class is not reapplied:

image

Using match with HMR the is-active class is reapplied:

image

Related information

MangelMaxime commented 1 year ago

Hum, I think this "bug" has been around since a long time because I saw this behaviour for a long time.

However, I am not sure if this is a bug or not. The things, is that Completed is a class instance and when doing the HMR replacement the class reference is probably replaced.

Meaning that when the view is re-run the comparison is done between the old class instance stored in state.Filter and the new class available coming from Completed.

A way to check if what I am saying is correct is to move Completed definition to another file. And trigger an HMR call from the file having the code state.Filter = Completed. This should only hot replace the "view" file and not the file with the definition of the Completed type.

MangelMaxime commented 6 months ago

As hinted in my previous message this is not really bug but an unfortunate effect of how Fable/F# equality works.

It is possible to remove this limitation for DUs with no arguments by transforming them into StringEnum like that Fable will generate a string representation of the DU case against which the equality check will works between HMR calls.

[<RequireQualifiedAccess>]
[<StringEnum>]
// Because Fable equality test the constructor reference, in watch mode we
// will get a new constructor reference every time we change the code.
// Which leads to invalid visual menu state.
// Using a string enum we can avoid this problem.
type Filter =
    | All
    | Completed
    | NotCompleted

In F# 9, we will have access to IsXXX members on DUs which will be implemented by checking the flag value (which is an integer) so it will be possible to use this members to work around the limitation. But it will probably means using if statement instead of pattern matching.

I am closing as I don't think we can do something inside of Elmish.HMR to improve the situation unfortunately.