dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.21k stars 9.95k forks source link

Blazor - OnRenderAsync being fired twice with PreRendering enabled #11876

Closed phinett closed 5 years ago

phinett commented 5 years ago

When i have PreRendering enabled on my blazor app, i am getting 2 calls to OnAfterRenderAsync for my page.

I am trying to call some JS interop in this event to initialise a javascript plugin, but its getting called twice causing some issues.

I have tried creating a bool flag and doing a if(alreadyPreRendered) around my JsInterop call but the bool is always reset on the second time around.

Disabling PreRendering fixes the issue (it still calls the event twice, but my bool flag now works).

 private bool _hasRenderedAlready;

        protected override async Task OnAfterRenderAsync()
        {
            // TEMPORARY: Currently we need this guard to avoid making the interop
            // call during prerendering. Soon this will be unnecessary because we
            // will change OnAfterRenderAsync so that it won't run during the
            // prerendering phase.
            if (!ComponentContext.IsConnected || _hasRenderedAlready)  
            {
                return;
            }

            // init our flowjs on the client
            await _jsRuntime.InvokeAsync<dynamic>("FlowJsUpload_Init", new Dictionary<string, string>(), "/upload/file");

            _hasRenderedAlready = true;

            await base.OnAfterRenderAsync();
        }
  1. Using this version of ASP.NET Core 3.0 preview (version 6)
mkArtakMSFT commented 5 years ago

Thank you for filing this issue. In order for us to investigate this issue, please provide a minimalistic repro project that illustrates the problem.

javiercn commented 5 years ago

@phinett do you have a minimal repro project we can use to further investigate this?

Andrzej-W commented 5 years ago

I'm not the author of this issue but I have just made two tests: one with default template for Blazor server side application and one with Blazor client side with prerendering enabled (see this repo: https://github.com/danroth27/ClientSideBlazorWithPrerendering).

In both tests I have this in index.razor file:

@page "/"
@inject IComponentContext ComponentContext

<h1>Hello, world!</h1>

Welcome to your new app.
@code
{
    static int counter = 0;
    protected override Task OnAfterRenderAsync()
    {
        string prerendering = ComponentContext.IsConnected ? "rendered in the browser" : "prerendering";
        Console.WriteLine($"OnAfterRender: {++counter} {prerendering}");
        return base.OnAfterRenderAsync();
    }
}

Compile the application and hit Ctrl+F5 to run it without debugger.

In sever side version I see only one message from my app: "OnAfterRender: 1 rendered in the browser". I see it in VS output window. It looks that OnAfterRenderAsync is called only once and only after browser connects to the server. This is expected.

In client side version I see this message in VS output window "OnAfterRender: 1 prerendering" and then this message in browser's console "WASM: OnAfterRender: 1 rendered in the browser". It looks that in client side version OnAfterRender is really called twice: once on the server when page is prerendered and second time when page is rendered in the browser. To be consistent with server side version OnAfterRender should be called only once and only in the browser.

javiercn commented 5 years ago

n client side version I see this message in VS output window "OnAfterRender: 1 prerendering" and then this message in browser's console "WASM: OnAfterRender: 1 rendered in the browser". It looks that in client side version OnAfterRender is really called twice: once on the server when page is prerendered and second time when page is rendered in the browser. To be consistent with server side version OnAfterRender should be called only once and only in the browser.

It's called twice because there are two renders instead of one, it's consistent. There is no magic happening here. The server prerenders the app (through what we call the "static prerenderer" and then tears it down.

Then the client app boots up and renders the original app again (hence 2 renders 2 calls).

On the prerender+reconnect case (the original server side example) we delay invoking onafterrender until we have established a connection to the server, to provide an experience similar to what happens in the non prerendered case.

We could if we wanted to, never trigger the onafterrender event during static rendering, but that's it. @SteveSandersonMS do you have any thoughts on that? I don't see a big issue given that we dispose the renderer after it.

Andrzej-W commented 5 years ago

In the doc we can read:

OnAfterRenderAsync and OnAfterRender are called after a component has finished rendering. Element and component references are populated at this point. Use this stage to perform additional initialization steps using the rendered content, such as activating third-party JavaScript libraries that operate on the rendered DOM elements.

The question is: do we need OnAfterRender events for something else? If not, it shouldn't be called during prerendering because DOM does not exist at this time. Now in OnAfterRender programmers have to manually test this condition.

SteveSandersonMS commented 5 years ago

We could if we wanted to, never trigger the onafterrender event during static rendering, but that's it. @SteveSandersonMS do you have any thoughts on that? I don't see a big issue given that we dispose the renderer after it.

That's what I thought we were doing. I'm slightly surprised we do call onafterrender in static rendering.

Given that there's virtually nothing useful you can do after a static render has completed (the HTML output is already determined, and if you want teardown logic, use Dispose instead), and calling onafterrender spoils the guidance we give that says you're safe to use IJSRuntime at that time, it seems altogether beneficial if we stopped calling it in static rendering.

@danroth27 @rynowak Do you agree?

javiercn commented 5 years ago

Sounds good to me. Implementation wise, we can simply return a cancelled task to avoid doing any onafterrender work. We can always think of static prerendering as a render that completes partially.

https://github.com/aspnet/AspNetCore/blob/master/src/Components/Components/src/Rendering/Renderer.cs#L501-L506

phinett commented 5 years ago

I have managed to create a small repo of the issue (took some time to narrow down the issue!)

If i have any call in OnInitAsync which does any async/await work, it causes the OnAfterRenderAsync function to be called twice for some unknown reason.

BlazorRepo.zip

Hope this helps.

javiercn commented 5 years ago

@phinett Your repro doesn't repro, but I have an idea why you are seeing it called twice. When the browser (specially chrome) autocompletes, starts requesting the page before you even "commit" to the page so sometimes it ends up with two requests for the index page, resulting in two after render calls.

phinett commented 5 years ago

It definetly is called twice for me, this is on initial page load from when i start debugging. I have tried on MS Edge as well and it reproduces every time.

javiercn commented 5 years ago

@phinett Not sure what to tell you, with the sample you uploaded, its server-side blazor and we have tests for this.

One thing you can try is to use an InPrivate/Incognito window and see if you observe it there. The other thing that might be happening (based on your sample) is that you have an async OnInit, so there are going to be two renders and the onafterrender method is going to be called once for each render.

Remove your async OnInit and see that you can't repro it. (I did that because the code was not compiling).

SteveSandersonMS commented 5 years ago

If i have any call in OnInitAsync which does any async/await work, it causes the OnAfterRenderAsync function to be called twice for some unknown reason.

I would expect it to be called twice, because there would be two renders. The first would be synchronously when your OnInitAsync yields the thread, and the second would be when the returned Task completes.

phinett commented 5 years ago

So how can i call JsInterop to register my client side JS plugin only once?

I have tried everything and i can't find a way.

I have even tried adding a boolean to do a check such as bool hasAlreadyPreRendered which is set to true after running, but the second time its called it's still set to false.

SteveSandersonMS commented 5 years ago

bool hasAlreadyPreRendered which is set to true after running, but the second time its called it's still set to false.

Does your repro show that happening? @javiercn, were you able to see that? I looked at the repro code and couldn't see any flag such as this.

phinett commented 5 years ago

I have attached a modified repo, i really hope you can see it reproducing this behavior.

Thanks for taking the time to look into this!

BlazorRepo.zip

javiercn commented 5 years ago

I took another look. This is again expected behavior, and the reason is that you have an oninitasync method that produces two render batches.

        using (HttpClient client = new HttpClient())
        {
            client.BaseAddress = new Uri("https://localhost:44391/");
            client.DefaultRequestHeaders.Accept.Clear();
            client.DefaultRequestHeaders.Accept.Add(
                new MediaTypeWithQualityHeaderValue("application/json"));

            // First render batch gets produced here.
            HttpResponseMessage response = await client.GetAsync("api/test");
            if (response.IsSuccessStatusCode)
            {
                TestString = await response.Content.ReadAsStringAsync();
            }
            // Second render batch gets produced here
        }

That's why you see two calls to onafterrenderasync

Andrzej-W commented 5 years ago

@javiercn , @SteveSandersonMS This issue was closed but in response to my example code https://github.com/aspnet/AspNetCore/issues/11876#issuecomment-508877095 Steve wrote:

Given that there's virtually nothing useful you can do after a static render has completed (the HTML output is already determined, and if you want teardown logic, use Dispose instead), and calling onafterrender spoils the guidance we give that says you're safe to use IJSRuntime at that time, it seems altogether beneficial if we stopped calling it in static rendering.

Is it fixed? Is there any other issue to track this problem?

iguanaware commented 5 years ago

Preview #8 It's not calling the methods twice on "a" component. It actually creates two components that each have their events fired. But the prerender one fails to get dotnet references.

Remove Html.RenderStaticComponentAsync and the problem goes away.

The bigger problem is that OnAfterRenderAsync is firing on the prerendering (8 3.0.100-preview8-013656)

dotnet new blazorserver

add to Index.razor @code{ protected override async Task OnAfterRenderAsync() { Console.WriteLine("\n\n\nHello There"); } }

Below is the output (Hello there appears twice)

info: Microsoft.AspNetCore.DataProtection.KeyManagement.XmlKeyManager[0] User profile is available. Using 'C:\Users\xxx\AppData\Local\ASP.NET\DataProtection-Keys' as key repository and Windows DPAPI to encrypt keys at rest. info: Microsoft.Hosting.Lifetime[0] Now listening on: https://localhost:5001 info: Microsoft.Hosting.Lifetime[0] Now listening on: http://localhost:5000 info: Microsoft.Hosting.Lifetime[0] Application started. Press Ctrl+C to shut down. info: Microsoft.Hosting.Lifetime[0] Hosting environment: Development info: Microsoft.Hosting.Lifetime[0] Content root path: C:\temp\bl2 info: Microsoft.AspNetCore.Hosting.Diagnostics[1] Request starting HTTP/2 GET https://localhost:5001/ info: Microsoft.AspNetCore.Routing.EndpointMiddleware[0] Executing endpoint '/_Host' info: Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure.PageActionInvoker[3] Route matched with {page = "/_Host"}. Executing page /_Host info: Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure.PageActionInvoker[103] Executing an implicit handler method - ModelState is Valid info: Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure.PageActionInvoker[104] Executed an implicit handler method, returned result Microsoft.AspNetCore.Mvc.RazorPages.PageResult.

Hello There

info: Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure.PageActionInvoker[4] Executed page /_Host in 75.21430000000001ms info: Microsoft.AspNetCore.Routing.EndpointMiddleware[1] Executed endpoint '/_Host' info: Microsoft.AspNetCore.Hosting.Diagnostics[1] Request starting HTTP/2 GET https://localhost:5001/_framework/blazor.server.js info: Microsoft.AspNetCore.Hosting.Diagnostics[1] Request starting HTTP/2 GET https://localhost:5001/css/site.css info: Microsoft.AspNetCore.Hosting.Diagnostics[1] Request starting HTTP/2 GET https://localhost:5001/css/bootstrap/bootstrap.min.css info: Microsoft.AspNetCore.Hosting.Diagnostics[2] Request finished in 157.7618ms 200 text/html; charset=utf-8 info: Microsoft.AspNetCore.StaticFiles.StaticFileMiddleware[6] The file /_framework/blazor.server.js was not modified info: Microsoft.AspNetCore.Hosting.Diagnostics[2] Request finished in 23.8781ms 304 application/javascript info: Microsoft.AspNetCore.StaticFiles.StaticFileMiddleware[2] Sending file. Request path: '/css/site.css'. Physical path: 'C:\temp\bl2\wwwroot\css\site.css' info: Microsoft.AspNetCore.Hosting.Diagnostics[2] Request finished in 29.692300000000003ms 200 text/css info: Microsoft.AspNetCore.Hosting.Diagnostics[1] Request starting HTTP/2 GET https://localhost:5001/css/open-iconic/font/css/open-iconic-bootstrap.min.css info: Microsoft.AspNetCore.StaticFiles.StaticFileMiddleware[2] Sending file. Request path: '/css/open-iconic/font/css/open-iconic-bootstrap.min.css'. Physical path: 'C:\temp\bl2\wwwroot\css\open-iconic\font\css\open-iconic-bootstrap.min.css' info: Microsoft.AspNetCore.Hosting.Diagnostics[2] Request finished in 7.6515ms 200 text/css info: Microsoft.AspNetCore.StaticFiles.StaticFileMiddleware[2] Sending file. Request path: '/css/bootstrap/bootstrap.min.css'. Physical path: 'C:\temp\bl2\wwwroot\css\bootstrap\bootstrap.min.css' info: Microsoft.AspNetCore.Hosting.Diagnostics[2] Request finished in 59.975ms 200 text/css info: Microsoft.AspNetCore.Hosting.Diagnostics[1] Request starting HTTP/2 POST https://localhost:5001/_blazor/negotiate text/plain;charset=UTF-8 0 info: Microsoft.AspNetCore.Routing.EndpointMiddleware[0] Executing endpoint '/_blazor/negotiate' info: Microsoft.AspNetCore.Hosting.Diagnostics[1] Request starting HTTP/2 GET https://localhost:5001/css/open-iconic/font/fonts/open-iconic.woff info: Microsoft.AspNetCore.StaticFiles.StaticFileMiddleware[2] Sending file. Request path: '/css/open-iconic/font/fonts/open-iconic.woff'. Physical path: 'C:\temp\bl2\wwwroot\css\open-iconic\font\fonts\open-iconic.woff' info: Microsoft.AspNetCore.Hosting.Diagnostics[2] Request finished in 4.8011ms 200 application/font-woff info: Microsoft.AspNetCore.Routing.EndpointMiddleware[1] Executed endpoint '/_blazor/negotiate' info: Microsoft.AspNetCore.Hosting.Diagnostics[2] Request finished in 19.5062ms 200 application/json info: Microsoft.AspNetCore.Hosting.Diagnostics[1] Request starting HTTP/1.1 GET https://localhost:5001/_blazor?id=vAU0RDA4fpgS2edLCkhHGw info: Microsoft.AspNetCore.Routing.EndpointMiddleware[0] Executing endpoint '/_blazor' info: Microsoft.AspNetCore.SignalR.HubConnectionContext[1] Completed connection handshake. Using HubProtocol 'blazorpack'.

Hello There

javiercn commented 5 years ago

This was already fixed as part of preview9