RickStrahl / Westwind.AspnetCore.LiveReload

ASP.NET Core Live Reload Middleware that monitors file changes in your project and automatically reloads the browser's active page
Other
469 stars 42 forks source link

Refresh is too quick for Razor Runtime Compilation #40

Closed GravlLift closed 3 years ago

GravlLift commented 3 years ago

When using services.AddRazorPages().AddRazorRuntimeCompilation(); as described here, alongside the suggested ClientFileExtensions, edits to .cshtml pages (and possibly .razor pages?) result in the page instantly refreshing, usually before the server can recompile the changes. Any subsequent manual refreshes contain the new changes.

It seems like a slight delay (< a second usually) prior to refreshing the page is in order. Perhaps since the DelayRefresh event seems no longer in use, that could be repurposed for a slight delay following .cshtml updates.

RickStrahl commented 3 years ago

Hmmmm... I have not seen this fail. And this even though the very first Razor page/view request is usually very slow (upwards of 5 seconds) as the RazorRuntime and compiler have to load into the application.

I'm actually not quite sure why that is working but I think it's because the Web Socket fires and the page is waiting to load as the server tries to recompile the page. This should work fine for server side Razor Pages/Views.

It's probably a problem with Blazor since that compilation is handled differently...

GravlLift commented 3 years ago

I'm not using Blazor, just speculating about it in the initial post. This definitely affects Razor pages though. I went ahead and set up a minimal reproduction here. It's basically just the AspNetCore 3.1 Razor template with Microsoft.AspNetCore.Mvc.Razor.RuntimeCompilation and Westwind.AspNetCore.LiveReload added and configured. I reproduced by changing the content of Pages/Index.cshtml.

I think I may not have been clear about the issue. The page view you are describing is slower (Chrome clocked it at 1.77s as opposed to the initial 7ms), however I have to reload it manually. By having .cshtml in the ClientFileExtensions option as the readme suggests, the reload is triggered immediately, and since the entire request only takes 7ms, the razor runtime compilation hasn't even started by the time the live reload completes.

Removing .cshtml from the ClientFileExtensions options "fixes" the problem, but then the entire application has to restart, which defeats the purpose of RuntimeCompilation.

RickStrahl commented 3 years ago

Hmmm... I understand what you're saying but I'm puzzled why it works for me when the initial Razor engine load for the first page takes upwards of 5 seconds to work and it does actually auto-refresh.

What I see is this:

IOW, it looks to me like ASP.NET Core doesn't serve the Razor page while it's being updated/compiled which puts the active request into an HTTP wait state just like any other still processing request until the compilation completes.

It seems to me this is the expected and also ideal behavior. This waits as long as it takes to get the page updated and no longer. That behavior is why I removed the delay switch. This only works if Runtime Compilation is set up correctly though - is it possible you don't have it set up right? Without runtime compilation razor changes dotnet watch causes the app to re-compile and restart, which would also refresh the page but obviously would take much longer.

Just to be sure: You're using 3.1 or 5.0 and EndPoint Routing and RazorCompilation enabled on Pages/Views? could you try your failing scenario with the 3.1 sample application and see what you get?

One thing that I could think of here: I suppose it's possible that the refresh happens so fast that it occurs before ASP.NET Core recognizes the file has changed. So you change the page, the refresh happens first which would effectively give you the old page, then ASP.NET Core recompiles the page and the page is not updated. In that case a short delay might help.

I also checked the code and as you point out I removed the delayed call because of the implied behavior I describe above which seems to work reliably on my end. I think the only time the default behavior becomes a problem if the timeout exceeds the HTTP request timeout of the server.

Just for kicks I've added the delay back in now so that the delay is respected for .cshtml and .razor files, so at least one can play around with those settings. I'm defaulting the delay to 0 though.

        /// <summary>
        /// The timeout to wait before refreshing the page in the browser
        /// for Razor page re-compilation requests that don't restart the server
        ///
        /// This shouldn't be necessary as Razor server recompilation should block
        /// the page from being served while it's being recompiled. You'll see the
        /// browser 'waiting to connect' until the page is ready to load
        ///
        /// Only bump this value if you have problems with Razor page refreshes
        /// and it's suggested you bump this value up slowly to find your sweet
        /// spot - it should only have to be long enough for ASP.NET to get to the
        /// file to recompile before the page refreshes.
        /// </summary>
        public int ServerRefreshTimeout { get; set; } = 0;

and then:

if (inclusionMode == FileInclusionModes.ForceRefresh ||
    _extensionList.Contains(ext,StringComparer.OrdinalIgnoreCase))
{
    // Razor Pages don't restart the server, so we need a slight delay
    bool delayed = LiveReloadConfiguration.Current.ServerRefreshTimeout > 0 &&
                   (ext == ".cshtml" || ext == ".razor");

    if (_isFolderCreated)
    {
        _throttler.Debounce(2000, param =>
        {
            _ = LiveReloadMiddleware.RefreshWebSocketRequest(delayed);
            _isFolderCreated = false;
        });
    }
    else
    {
        _ = LiveReloadMiddleware.RefreshWebSocketRequest(delayed);
    }
}

Ideally it would be nice if there was a way to tell a Razor View/Page has been recompiled and it's ready but I doubt there's a way to actually check for that.

GravlLift commented 3 years ago

This only works if Runtime Compilation is set up correctly though - is it possible you don't have it set up right?

Just to be sure: You're using 3.1 or 5.0 and EndPoint Routing and RazorCompilation enabled on Pages/Views? could you try your failing scenario with the 3.1 sample application and see what you get?

I'd encourage you to review the minimal reproduction repository I linked in my previous comment. It's a fairly barebones configuration that reproduces on my machine, and while I do think it's configured correctly, reviewing that would be the best way to confirm that this isn't simply my misconfiguring something.

One thing that I could think of here: I suppose it's possible that the refresh happens so fast that it occurs before ASP.NET Core recognizes the file has changed. So you change the page, the refresh happens first which would effectively give you the old page, then ASP.NET Core recompiles the page and the page is not updated. In that case a short delay might help.

Yes, I think this is almost certainly exactly what's happening. Given that you clock your refresh at ~5 seconds, while mine takes closer to 2, I'm thinking that maybe this is only an issue if the hosting machine is fast enough to serve an entire request between the cshtml save and the start of the razor compilation.

Ideally it would be nice if there was a way to tell a Razor View/Page has been recompiled and it's ready but I doubt there's a way to actually check for that.

It may be possible to pull one of the Dependency Injection objects used by Microsoft.AspNetCore.Mvc.Razor.RuntimeCompilation and read some combination of properties exposed by those objects, to determine that but I don't know of any off the top of my head. I'll be sure to post here if I do come across anything like that though.

The solution you've got there looks like it would work. If you want to add that to a branch on this repo I'd be happy to clone and test it for you.

RickStrahl commented 3 years ago

I've just pushed it up on Master.

RickStrahl commented 3 years ago

So I gave your sample project a try and here's what I see:

So that confirms what you've been saying.

So then I tried the same thing on my sample application which isn't very different except I go into an MVC view by default instead of a Razor Page. There the first change is detected and the page refreshes.

So then I restarted dotnet watch run and navigated to the about Razor page and repeated. And lo and behold - that doesn't work. So it looks like MVC views and Razor Pages are doing something slightly different in behavior with their views.

I then added a short server timeout (500) with the updated setting discussed above restarted and it looks like now the Razor page too refreshes.

So based on that I think my previous theory that somehow the actual browser refresh occurs before ASP.NET gets a hold of the file change is the key. Meaning the server timeout doesn't need to be very long - just long enough so that ASP.NET can pick up the change itself.

GravlLift commented 3 years ago

I cloned your master branch and replaced the package dependency in my reproduction repo. Works perfectly now.

the server timeout doesn't need to be very long - just long enough so that ASP.NET can pick up the change itself.

Yeah, you're not kidding. I set the value to 1ms and that was enough to fix the issue. I guess the amount of time it takes to evaluate if setTimeout has expired on the browser is enough.

I was interested in if this was behavior of the setTimeout function itself, so I went and removed your LiveReloadConfiguration.Current.ServerRefreshTimeout > 0 check, such that a "DelayRefresh" message was always sent if the file extension was .cshtml or .razor...

bool delayed = (ext == ".cshtml" || ext == ".razor");

and then set the ServerRefreshTimeout to 0, resulting in the following JS getting generated/executed:

if (message.data == 'DelayRefresh') {
    console.log('Live Reload Delayed Reload.');
    setTimeout( 
        function() { 
            location.reload(); 
        },0);
}

This JS caused the quick reload issue to return. I can only speculate that chrome has some slightly different behavior for setTimeout(..., 0) and setTimeout(..., >0) essentially causes a 0 delay to function as though you didn't wrap setTimeout at all. Or the other possibility is that it legitimately takes <1 ms to start the razor compilation, which I suppose is very possible as well.

Given these findings, I don't know if this is necessarily something you need to have a dedicated setting around. It seems like hardcoding that JS timeout value to 1 would serve just as well.

RickStrahl commented 3 years ago

I played around with this a bit, but I found that this doesn't work reliably for me if the timeout is too short. It works but not consistently.

I've changed the script to always do the reload on a timeout even on the non-delayed version, but keep the timeout very short. So if this helps most of the time I guess that's better than not at all :smile:

if (message.data == 'DelayRefresh') {{
            console.log('Live Reload Delayed Reload.');
            setTimeout( 
                function() {{ 
                    location.reload(); 
                }},{config.ServerRefreshTimeout});
        }}
        if (message.data == 'Refresh')           
          setTimeout(function()  {{ location.reload(); }}, 10);

It's odd - I tried earlier this before you wrote your note and it didn't work unless the timeout was at least 500ms. Now I just retried this again and sure enough I see it working with the 1ms and 10ms - pretty much consistently. Not sure why that would be.

IAC this should help with teh first Razor page refresh. This didn't make it into the latest update - it'll go into the next one.

+++ Rick ---