EdCharbeneau / BlazorSize

Blazor browser size interop for matchMedia and browser window size at runtime.
336 stars 39 forks source link

MediaQueryService.RemoveQuery calls JS #78

Closed rofenix2 closed 2 years ago

rofenix2 commented 2 years ago

Hi, I've been using this great library, just one bug:

When i create two MediaQuery with the same Media breakpoint and the components get Disposed in pre rendering (Blazor Server) it will trigger the Dispose of MediaQuery.razor and in turn will try to eliminate the registered breakpoints calling JS which trigger a exception since JavaScript cant be used while pre rendering.

The method that causes the bug is the following in MediaQueryService.cs:

public async Task RemoveQuery(MediaQuery mediaQuery)
{
     if (mediaQuery == null) return;

     var cache = GetMediaQueryFromCache(byMedia: mediaQuery.Media);

     if (cache == null) return;

     cache.MediaQueries.Remove(mediaQuery);
     if (cache.MediaQueries.Any())
     {
         mediaQueries.Remove(cache);
         var module = await moduleTask.Value;
         await module.InvokeVoidAsync($"removeMediaQuery", DotNetInstance, mediaQuery.InternalMedia.Media);
     }
}

Maybe it needs to check if we are on pre rendering and avoid calling JS or delay it till we are. I tried the following patch:

cache.MediaQueries.Remove(mediaQuery);
if (cache.MediaQueries.Any())
{
   mediaQueries.Remove(cache);
   if (DotNetInstance != null)
   {
       var module = await moduleTask.Value;
       await module.InvokeVoidAsync($"removeMediaQuery", DotNetInstance, mediaQuery.InternalMedia.Media);
    }
}

But i could not test it since when i compile and run the project i get a weird Microsoft.JSInterop.JSException: error loading dynamically imported module. I believe i've read that annoying bug before here in the Issues section, but that's another topic.

How to reproduce:

<MediaQuery Media="@Breakpoints.SmallDown">
        <Matched>
            <div>Matched</div>
        </Matched>
        <Unmatched>
            <div>Ummatched</div>
        </Unmatched>
    </MediaQuery>
    <MediaQuery Media="@Breakpoints.SmallDown">
        <Matched>
            <div>Matched</div>
        </Matched>
        <Unmatched>
            <div>Ummatched</div>
        </Unmatched>
    </MediaQuery>
Microsoft.AspNetCore.Server.Kestrel: Error: Connection id "0HMHI6QI9R07S", Request id "0HMHI6QI9R07S:0000000F": An unhandled exception was thrown by the application.

System.InvalidOperationException: JavaScript interop calls cannot be issued at this time. This is because the component is being statically rendered. When prerendering is enabled, JavaScript interop calls can only be performed during the OnAfterRenderAsync lifecycle method.
   at Microsoft.AspNetCore.Components.Server.Circuits.RemoteJSRuntime.BeginInvokeJS(Int64 asyncHandle, String identifier, String argsJson, JSCallResultType resultType, Int64 targetInstanceId)
   at Microsoft.JSInterop.JSRuntime.InvokeAsync[TValue](Int64 targetInstanceId, String identifier, CancellationToken cancellationToken, Object[] args)
   at Microsoft.JSInterop.JSRuntime.InvokeAsync[TValue](Int64 targetInstanceId, String identifier, Object[] args)
   at BlazorPro.BlazorSize.MediaQueryService.RemoveQuery(MediaQuery mediaQuery)
   at BlazorPro.BlazorSize.MediaQueryList.RemoveQuery(MediaQuery mediaQuery)
   at BlazorPro.BlazorSize.MediaQuery.DisposeAsync()
   at Microsoft.AspNetCore.Components.RenderTree.Renderer.<>c__DisplayClass69_0.<<Dispose>g__HandleAsyncExceptions|1>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Components.Rendering.HtmlRenderer.HandleException(Exception exception)
   at Microsoft.AspNetCore.Components.RenderTree.Renderer.<>c__DisplayClass69_0.<Dispose>g__NotifyExceptions|2(List`1 exceptions)
   at Microsoft.AspNetCore.Components.RenderTree.Renderer.<>c__DisplayClass69_0.<<Dispose>g__HandleAsyncExceptions|1>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Components.RenderTree.Renderer.DisposeAsync()
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceProviderEngineScope.<DisposeAsync>g__Await|22_0(Int32 i, ValueTask vt, List`1 toDispose)
   at Microsoft.AspNetCore.Http.Features.RequestServicesFeature.<DisposeAsync>g__Awaited|9_0(RequestServicesFeature servicesFeature, ValueTask vt)
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.<FireOnCompleted>g__ProcessEvents|227_0(HttpProtocol protocol, Stack`1 events)
The program '[25464] TestWeb.exe' has exited with code 4294967295 (0xffffffff).

You might be wondering why add two tags with the same breakpoint, but thats not how i originally got the bug, the reason is i use two different components (one after the another) and both check for the same breakpoints, this is just a easy way to recreate the bug.

Thanks.

EdCharbeneau commented 2 years ago

I haven't done a lot of testing with Server Side Rendering. Thanks for reporting the issue so I can try to reproduce it and find a solution.

EdCharbeneau commented 2 years ago

For some reason I'm not able to reproduce it. Let's talk about your setup a bit more.

How is pre-rendering enabled? Is this a Blazor WebAssembly project or Razor components via ASP.NET Core MVC or Razor Pages?

EdCharbeneau commented 2 years ago

Please see my sample for reference: https://github.com/EdCharbeneau/BlazorSizePrerendering

rofenix2 commented 2 years ago

The project its a default Blazor Server App template from VS2022 (Razor pages). The pre rendering comes enabled by default in _Host.cshtml:

<component type="typeof(App)" render-mode="ServerPrerendered" />

Ive uploaded a default project with only the minimal changes to reproduce the bug. Also the link you provided doesnt work it throws me a 404 page not found.

Btw i believe Blazor documentation says: "Disposal can occur at any time, including during component initialization", so when the components get disposed in turn they will dispose the MediaQuery from the library and make a call to JS which will trigger a exception since its still in pre rendering phase.

TestWeb.zip

rofenix2 commented 2 years ago

Not sure if its relevant but i have on my IIS configuration the DefaultAppPool with a Shutdown Time Limit of 1 second, since i need to deploy a lot of times for testing and IIS will keep my files locked cause the browser keeps sending reconnection request so i lowered to 1 second in order to avoid this.

EdCharbeneau commented 2 years ago

I finally got a break to check this out. This is a standard Blazor Server app. It should work as expected, there must be something in your configuration. I tested the zip file and everything works locally.