fsbolero / Bolero

Bolero brings Blazor to F# developers with an easy to use Model-View-Update architecture, HTML combinators, hot reloaded templates, type-safe endpoints, advanced routing and remoting capabilities, and more.
https://fsbolero.io
Apache License 2.0
1.06k stars 54 forks source link

Page Blinking issue in WASM mode. #112

Open OnurGumus opened 4 years ago

OnurGumus commented 4 years ago

A new problem has emerged with latest bolero 0.11. Steps to produce

OnurGumus commented 4 years ago

Strangely it doesn't always happen. I am adding a gif here: bolero

Tarmil commented 4 years ago
  • (You may need to install Microsoft.AspNetCore.Components 3.1.1 if you upgraded to .net core 3.1.1, this is another bug)

Ouch, yeah somehow the Bolero solution installs 3.1.1 but creates a package that depends on >= 3.1.0, I don't know how that didn't surface when testing the template :/ Sorry, I'll fix that.

  • You will see the page loads fine then it becomes white and it loads again.

Right, what's happening is that the page is statically rendered in HTML (because of the RenderMode.Static in Pages/_Host.cshtml), and then once the wasm has loaded, it is re-rendered by the client-side. This used to happen more smoothly, but I guess there have been changes in the rendering engine that made it clear the static content earlier or something like that. I think I'll remove the prerendering from the project template for now.

OnurGumus commented 4 years ago

@Tarmil here are my additional findings:

assuming you add the tag helpers to the top

@addTagHelper *, Microsoft.AspNetCore.Mvc.TagHelpers
OnurGumus commented 4 years ago

@Tarmil I found the culprit. It's basically the async program thing causes this problem https://github.com/fsbolero/Bolero/commit/fa37b7e30e4e2ba9cd02d6eaeb32d62971b5dbc7#diff-42537e1b023213573ef3e20b1e6fa305

--removed
  override this.OnInitialized() =
        base.OnInitialized()
        let program = this.Program
        let setDispatch dispatch =
            this.Dispatch <- dispatch
        { program with
            setState = fun model dispatch ->
                this.SetState(program, model, dispatch)
            init = fun arg ->
                let model, cmd = program.init arg
                model, setDispatch :: cmd
    member internal this._OnInitializedAsync() =
        base.OnInitializedAsync()
-- added
    override this.OnInitializedAsync() =
        async {
            do! this._OnInitializedAsync() |> Async.AwaitTask
            let! program = this.AsyncProgram
            let setDispatch dispatch =
                this.Dispatch <- dispatch
            { program with
                setState = fun model dispatch ->
                    this.SetState(program, model, dispatch)
                init = fun arg ->
                    let model, cmd = program.init arg
                    model, setDispatch :: cmd
            }
            |> Program.runWith this

I am not sure what part of this causes the problem, but I think we don't need this async program feature as of now , blazor supports parametrized prerendering. Unless you found a better solution I recommend revert above change, since this flickering is a pretty annoying issue.

OnurGumus commented 4 years ago

On the second thought, I think we still need this feature if possible. In WASM mode, even if we prerender the data, we still need to reload the data before the first rendering occurs on browser side. But still flickering is just bad. A proper solution is required.

OnurGumus commented 4 years ago

@Tarmil another finding this there is a problem with this call: https://github.com/fsbolero/Bolero/blob/47e14bf8f8fa868f7bf09b3bb50de26bd323f698/src/Bolero/Components.fs#L155

Since elmish initially calls SetState this causes ForceSetState being called too early and eventually you call this.InvokeAsync(this.StateHasChanged) before the init phase. As a result OnAfterRenderAsync is called before OnInitializedAsync which is against standard lifecycle.

Perhaps you should first check if OnAfterRenderAsync is called before calling this.InvokeAsync(this.StateHasChanged).

Tarmil commented 4 years ago

Thanks for the investigation @OnurGumus! I've been trying various fixes but so far the only way I've managed to reliably get rid of the blink is by reverting to synchronous OnInitialized. I'll publish a hotfix that does that, and then keep trying to see if there's a way to make OnInitializedAsync work because I'd really like to allow people to use AsyncProgram to do some async initialization.

OnurGumus commented 4 years ago

@Tarmil Another finding is You can also check if firstTime true for this call https://github.com/fsbolero/Bolero/blob/47e14bf8f8fa868f7bf09b3bb50de26bd323f698/src/Bolero/Components.fs#L203 There is no point in calling the same thing over and over again

BTW so far I found the only reliable way to initialize data on WASM browser is to use SetParametersAsync which happens before everything. There I was able to do my async loading and set it to a property of the class, then I was able to pass it to model. It's probably not the most natural way but it works.

OnurGumus commented 4 years ago

Fixed with 0.11

OnurGumus commented 4 years ago

@Tarmil , Good news, I have found a better way, just initialize elmish on OnParametersSetAsync Then then everything works as expected and we can keep AsyncProgram. I also believe Initialize is too early. With OnParametersSetAsync you also give some opportunity for parameters being set. It also allows you to write your own Initialization before elmish starts. So I suggest to go with it. Re opening the issue for the sake of book keeping.

OnurGumus commented 4 years ago

Though the bad part is, if you actually do any useful work in AsyncProgram, it flickers again :(