HotReload is not working in ASP.NET Core in VS2022 or dotnet watch run when ServiceWorkers are installed/running

[ChaosEngine]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

When created brand new ASP.NET Core 6 project and added ServiceWorker JavaScript code (for caching, offline handling etc.) HotReload functionally is not loading.
IMVHO it is related to the fact that key messing ingredient SignlaR script:

    <script src="/_framework/aspnetcore-browser-refresh.js"></script>

is not loaded. Why ?? don't know.
So in order to make it behave properly user is obliged to load manually this script (for example by including it in _Layout)
After that all is back to normal again. The script is simply not emitted when Service Worker are installed and running.

Same behavior is observer when run with Ctrl-F5 in VS2022 and through CLI -> dotnet watch run

Rerpro project demonstrating the issue

Expected Behavior

When ServiceWorker is installed and running hotReload should work out of the box in VS2022 and through dotnet watch run

Steps To Reproduce

https://github.com/ChaosEngine/NoHotReloadWithServiceWorker

Important pieces:
NoHotReloadWithServiceWorker\wwwroot\sw.js - Service worker
NoHotReloadWithServiceWorker\wwwroot\js\site.js - code loading sw.js

NoHotReloadWithServiceWorker\Pages\Shared_Layout.cshtml - place where <script src="/_framework/aspnetcore-browser-refresh.js"></script> is injected manually
NoHotReloadWithServiceWorker\Pages\Index.cshtml - page which aftter modification should auto-refresh in SignalR-connected browser

Exceptions (if any)

No response

.NET Version

6.0.101

Anything else?

VS2022 - Professional Version 17.0.5

$ dotnet --info
Zestaw .NET SDK (odzwierciedlenie dowolnego pliku global.json):
 Version:   6.0.101
 Commit:    ef49f6213a

Środowisko uruchomieniowe:
 OS Name:     Windows
 OS Version:  10.0.19043
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\6.0.101\

Host (useful for support):
  Version: 6.0.1
  Commit:  3a25a7f1cc

.NET SDKs installed:
  5.0.404 [C:\Program Files\dotnet\sdk]
  6.0.101 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.All 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 3.1.22 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 3.1.22 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.13 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.1.22 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 5.0.13 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 6.0.1 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

To install additional .NET runtimes or SDKs:
  https://aka.ms/dotnet-download

[ChaosEngine]

I think I've found root cause: here
Service Worker served requests have sec-fetch-dest: empty while "normal-requests" are sec-fetch-dest: document.

[mkArtakMSFT]

Thanks for contacting us, @ChaosEngine.
Can you try to simplify the service worker code to no-op (no logic almost) and then confirm that the behavior persists with that too?

Hi @ChaosEngine. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

[SteveSandersonMS]

BTW the minimal service worker file would look something like just this:

self.addEventListener('fetch', () => { });

If you could let us know if things work with that service worker (remember to delete all state for your site from the browser to make sure it takes effect) then we'll have more insight into whether this is specific to your service worker logic. Thanks!

[ChaosEngine]

BTW the minimal service worker file would look something like just this:

Thank you for quick reaction.

self.addEventListener('fetch', () => { });

If you could let us know if things work with that service worker (remember to delete all state for your site from the browser to make sure it takes effect) then we'll have more insight into whether this is specific to your service worker logic. Thanks!

This minimalist and simplistic approach from above will work properly because it does not use fetch underneath.
It is the use of fetch() as indicated here or here that is changing sec-fetch-dest to empty which in turn blocks emitting of script inclusion.

And most usage of service workers is with fetch() and caching it's responses.
PS. I simplified the sw.js in repro project to be more readable yet still use fetch(0 and repro the issue

[SteveSandersonMS]

Thanks @ChaosEngine, that clarifies things a lot.

So it seems like a valid fix would be to change https://github.com/dotnet/sdk/blob/14b117b7088653b694e16ac2071fcbf634a2a9ab/src/BuiltInTools/BrowserRefresh/BrowserRefreshMiddleware.cs#L71 to explicitly allow Sec-Fetch-Dest: empty. Or even change it so it explicitly blocks iframe but leaves everything else. Do you agree?

@pranavkm Do you know if there was any reason we couldn't allow everything except iframe?

[pranavkm]

Do you know if there was any reason we couldn't allow everything except iframe?

Some of the destinations such as images, videos, audio etc seem like the wrong place to be injecting scripts in to. Somebody also brought up a bunch of other scenarios that might not be appropriate, so I'd prefer if we started conservatively.