Feliz.Router.BasePath stopped navigation

[SCullman]

After a dotnet paket update I noticed that navigation with Feliz.Router.BasePath stopped working, or better, the hook stopped setting a new state with the current page.

I was able to track it down to Fable.Promise 3.1. When I downgraded to 2.2.2, it worked again.

I still have no idea what is happening, and whether Feliz.Router or the React.useElmish hook is also affected.

[alfonsogarciacaro]

I guess it's difficult/not possible to provide some code to reproduce. This is strange, the only use of Fable.Promise I can see in all the dependencies of Feliz.Router.BasePath is this call to Promise.start in Feliz.UseElmish. It's true we've have changed the code emitted for Promise.start in 3.1 from $0 to void $0. This should be more correct because the signature of the function returns unit but maybe Feliz.UseElmish depended on this somehow?

[alfonsogarciacaro]

It would be good to know if other projects using Feliz.UseElmish are having issues after upgrading to Fable.Promise 3.1.

[SCullman]

@alfonsogarciacaro, I know my description was not helpful for further investigation. I was already happy to determine Promise as the cause since no error or warning came up during compilation.

[SCullman]

"UseElmish" counter works fine with Fable.Promise 3.1

[SCullman]

...and Feliz.Router has no dependency on Fable.Promise.

[TWith2Sugars]

I'm getting the same issue and I think I've narrowed it down to Feliz.UseElmish when the update function returns a state AND another Cmd that isn't Cmd.none. It seems that the returned Cmd isn't processed until something else triggers the update. I'll keep digging to pin it down.

[alfonsogarciacaro]

Thanks for that @TWith2Sugars! Would you happen to have some code you can share? I could try to compile and see if there's something out of place in the generated JS.

[TWith2Sugars]

I do but it's part of a rather large private project, let me try and distill it to something smaller.

[TWith2Sugars]

Right, I've managed to create a repo from the Feliz template and isolate the issue:

https://github.com/TWith2Sugars/FelizPromiseIssue

I've taken the counter example and add an extra Msg IncrementAgain https://github.com/TWith2Sugars/FelizPromiseIssue/blob/main/src/Main.fs#L10

The update function will increment the state and return a Cmd.ofMsg Increment https://github.com/TWith2Sugars/FelizPromiseIssue/blob/main/src/Main.fs#L20

When you hit IncrementAgain you'll see the state only increases by 1.

[TWith2Sugars]

Let me know if there is anything else I can do to help.

[alfonsogarciacaro]

Thanks a lot for this @TWith2Sugars! This is super helpful. By testing your repo I realized the reason of the different behaviour was the change in the Run method of the promise CE. If I go back to the previous code, IncrementAgain works.

...However, I'm unsure if going back is the right thing to do for Fable.Promise. Actually after investigating I concluded I was actually using an implementation of Delay/Run that may not be a good fit for JS promises. See #25

TBH, even after spotting the code that's causing the wrong behaviour I still don't know what's actually happening. It's difficult to follow the program flow in UseElmish because it uses multiple React hooks. My intuition is that UseElmish somehow "relied" on a quirk of the previous Fable.Promise implementation. For example, if I replace promise with async here, the problem is still happening:

let rec dispatch (msg: 'Msg) =
    async {
        let mutable nextMsg = Some msg

        while nextMsg.IsSome && not (token.current.IsCancellationRequested) do
            let msg = nextMsg.Value
            let (state', cmd') = update msg state.current
            cmd' |> List.iter (fun sub -> sub dispatch)
            nextMsg <- ring.current.Pop()
            state.current <- state'
            setChildState()
    }
    |> Async.StartImmediate

It'd be ideal if the issue could be solved on the UseElmish side (if we cannot get a failing test for Fable.Promise). I came up with a draft alternative implementation (that's missing things and can definitely be improved) that doesn't use async/promise (as the original Elmish) and it's working:

static member useElmish<'State,'Msg> (init: 'State * Cmd<'Msg>, update: 'Msg -> 'State -> 'State * Cmd<'Msg>, dependencies: obj[]) =
    let exec dispatch cmd =
        cmd |> List.iter (fun call -> call dispatch)

    let setStateRef = React.useRef(ignore)
    let initState, initCmd, dispatch = React.useMemo(fun () ->
        let initState, initCmd = init
        let rb = RingBuffer(10)
        let mutable reentered = false
        let mutable state = initState

        let rec dispatch msg =
            if reentered then
                rb.Push msg
            else
                reentered <- true
                let mutable nextMsg = Some msg

                while Option.isSome nextMsg do
                    let msg = nextMsg.Value
                    let (model', cmd') = update msg state
                    setStateRef.current model'
                    cmd' |> exec dispatch
                    state <- model'
                    nextMsg <- rb.Pop()

                reentered <- false

        initState, initCmd, dispatch            
    )

    let state, setState = React.useState(initState)
    setStateRef.current <- setState

    React.useEffect((fun () ->
        initCmd |> exec dispatch
        React.createDisposable(fun () ->
            getDisposable state
            |> Option.iter (fun o -> o.Dispose())
        )
    ), [||])

    (state, dispatch)

@Shmew @Zaid-Ajaj @MangelMaxime

[MangelMaxime]

Hello @alfonsogarciacaro,

I am currently working for a client who is using Feliz.UseElmish and was wondering why it is using a custom implementation instead of asking for the standard Program used in Elmish?

My reason is that by not using Program we are losing a lot of stuff like the capability to compose/augment the Program to add new functionality.

Also the Elmish Program has been tested and working since a long time so we would also benefit from that part.

Wouldn't it make sense to try using Elmish Program instead of a custom implementation ?

[TWith2Sugars]

I've tested those replacements in my larger app and it's resolved all of my issues 👍

[SCullman]

@alfonsogarciacaro, this would also fix the initial issue, routing would work again.

@MangelMaxime, I used to work with Elmish Program , but Feliz.useElmish simplifies a lot, if not any of my projects. Hooks like React.useState() or React.useDeferred() are good enough for navigation and basic initialisation with data. And when a ReactComponent requires more logic, useElmish() still can step in.

[alfonsogarciacaro]

I think what @MangelMaxime is saying is we can use React.useElmish but with the original Elmish implementation underneath instead of adding a custom one. And he's probably right 😸 I guess Feliz.UseElmish uses a custom implementation to avoid the chicken-and-egg problem: Program.mkProgram requires you to pass a view function but with useElmish hook you need to initialize Elmish inside the view code. However, there may be a workaround for this.

Following Feliz.UseElmish, I also implemented the hook for Fable.Lit. Then I tried to make it compatible with Elmish Program to take advantage of Elmish add-ons as @MangelMaxime says. But after reading his last comment I tried to use the Elmish program directly and it seems it can be made to work by using an observable-like middle agent: https://github.com/fable-compiler/Fable.Lit/pull/21/files

We can try to do the same for Feliz.UseElmish. The API can be kept the same by building internally the program with the init and update functions (if users want to pass the program directly they can make it with Program.mkHidden init update which is a bit counterintuitive but works).

@TWith2Sugars @SCullman Are you using the alternative useElmish implementation in the comment above? Please note it's not optimized (it retains a reference to the initial state in memory) and it lacks the UseElmish ability to restart the Elmish state when you pass an array of dependencies.

[MangelMaxime]

@MangelMaxime, I used to work with Elmish Program , but Feliz.useElmish simplifies a lot, if not any of my projects. Hooks like React.useState() or React.useDeferred() are good enough for navigation and basic initialisation with data. And when a ReactComponent requires more logic, useElmish() still can step in.

@alfonsogarciacaro Did understand what I was saying. I am not saying we should remove Feliz.useElmish but make it accept an Elmish program as an argument.

The idea being that if you want to augment your Elmish loops with the current Feliz.useElmish implementation you can't do it which is a problem. For example, you can't attach a debugger to it neither can you attach a library like Thoth.Elmish.Toast for example.

[TWith2Sugars]

@TWith2Sugars @SCullman Are you using the alternative useElmish implementation in the comment above? Please note it's not optimized (it retains a reference to the initial state in memory) and it lacks the UseElmish ability to restart the Elmish state when you pass an array of dependencies.

Not in production just as a quick check to see if worked. I've just pinned the Fable.Promise dependency to 2.2.2 for the time being.