Add support for hot reload to dotnet-watch #16401

pranavkm · 2021-03-17T17:22:32Z

Host a Roslyn workspace to produce EnC deltas and apply to app
Add agents for ASP.NET Core and Blazor WebAssembly apps

ghost · 2021-03-17T17:22:36Z

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

* Host a Roslyn workspace to produce EnC deltas and apply to app * Add "agents" to support reloading ASP.NET Core and Blazor WebAssembly apps

pranavkm · 2021-03-17T23:17:36Z

I was hoping we could yolo this change for preview3, and I can do a walkthru of this next week. There's some design issues that might be nice to get some ideas on, plus it would be good if we can have others follow what's happening here.

pranavkm · 2021-03-18T01:50:31Z

@wli3 since this involves a changes to the redist, NuGet.config and the version files.

src/BuiltInTools/AspNetCoreDeltaApplier/Microsoft.Extensions.AspNetCoreDeltaApplier.csproj

captainsafia · 2021-03-18T02:00:28Z

src/BuiltInTools/AspNetCoreDeltaApplier/StartupHook.cs

+        foreach (var assembly in AppDomain.CurrentDomain.GetAssemblies())
+        {
+            var customAttributes = assembly.GetCustomAttributes<AssemblyMetadataAttribute>();
+            var deltaReceiverAttributes = customAttributes.Where(a => a.Key == "ReceiveHotReloadDeltaNotification" && !string.IsNullOrEmpty(a.Value));


Who is responsible for applying the ReceiveHotReloadDeltaNotification on the app's assemblies. The compiler?

This will eventually get replaced by a runtime API: dotnet/runtime#49361. This is a place holder until those APIs come online.

captainsafia · 2021-03-18T02:04:38Z

src/BuiltInTools/AspNetCoreDeltaApplier/StartupHook.cs

+        }
+        catch (TimeoutException)
+        {
+            Log("Unable to connect to hot-reload server.");


Does the hot-reload server log out info that would help determine why it failed to connect here?

Running watch --verbose should emits a fair bit of the diagnostics (for instance if the named pipe is in use), so that should probably help piecing this together.

captainsafia · 2021-03-18T02:07:17Z

src/BuiltInTools/AspNetCoreDeltaApplier/StartupHook.cs

+                    }
+                }
+
+                // We want to base this off of mvids, but we'll figure that out eventually.


As in eventually we'll be doing something like update.ChangedMvid?

Yup. Right now it's an empty collection, but it's a hypothetical idea I'd been playing around with to do different things based on what function gets updated.

captainsafia · 2021-03-18T02:08:15Z

src/BuiltInTools/BrowserRefresh/WebSocketScriptInjection.js

-          location.reload();
-          return;
-        }
+        if (parsed.type == 'UpdateStaticFile') {


Nit: might be time for a switch statement here.

captainsafia · 2021-03-18T02:27:16Z

src/BuiltInTools/dotnet-watch/HotReload/AspNetCoreDeltaApplier.cs

+                context.ProcessSpec.EnvironmentVariables.DotNetStartupHooks.Add(deltaApplier);
+
+                // Configure the app for EnC
+                context.ProcessSpec.EnvironmentVariables["DOTNET_MODIFIABLE_ASSEMBLIES"] = "debug";


What values can DOTNET_MODIFIABLE_ASSEMBLIES have?

I think it's the only possible value for it: dotnet/runtime#47274

captainsafia · 2021-03-18T02:33:39Z

src/BuiltInTools/dotnet-watch/HotReload/CompilationHandler.cs

+        {
+            if (_deltaApplier is null)
+            {
+                _deltaApplier = context.DefaultLaunchSettingsProfile.HotReloadProfile == "blazorwasm" ?


How do things get set up for a hosted Blazor WASM app?

TBD: dotnet/aspnetcore#30815

src/BuiltInTools/dotnet-watch/HotReload/CompilationHandler.cs

…spNetCoreDeltaApplier.csproj

src/BuiltInTools/AspNetCoreDeltaApplier/StartupHook.cs

src/BuiltInTools/dotnet-watch/HotReload/AspNetCoreContract.cs

jkotalik · 2021-03-18T17:12:58Z

src/BuiltInTools/dotnet-watch/HotReload/AspNetCoreDeltaApplier.cs

+            await _pipe.FlushAsync(cancellationToken);
+
+            var result = ApplyResult.Failed;
+            var bytes = ArrayPool<byte>.Shared.Rent(1);


Seems unnecessary to rent an array of size 1 here. I'd bet the array returned will be size 4096. If only one call to Apply is being run at a time, you could have it in a field.

src/BuiltInTools/dotnet-watch/HotReload/BlazorWebAssemblyDeltaApplier.cs

src/BuiltInTools/dotnet-watch/HotReload/CompilationHandler.cs

jkotalik · 2021-03-18T17:18:11Z

src/BuiltInTools/dotnet-watch/HotReload/CompilationHandler.cs

+        {
+            // FSW events sometimes appear before the file has been completely written to disk. Provide a small delay before we read the file contents
+            // to ensure we are not contending with partial writes or write locks.
+            await Task.Delay(20);


Is the goal here to maximize the chance of the first call succeeding?

Yup. This is obviously very twitchy, but so far it seems to work. I should remove the Task.Delay and see if I can make this work.

jkotalik · 2021-03-18T17:21:34Z

src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs

+
+                if (!fileSet.Project.IsNetCoreApp60OrNewer())
+                {
+                    _reporter.Error($"Hot reload based watching is only supported in .NET 6.0 or newer apps. Update the project's launchSettings.json to disable this feature.");


This seems bad. Are you saying that anyone who has a mix of projects (5.0, 6.0) will need to manually go into their launchsettings.json to disable it? Could it be a property in the csproj instead?

The launching project must be a net6 project since the APIs to apply hot reload are only available in net6.

Can we make this fail earlier for 5.0 and earlier?

You have to explicitly modify the launchSettings to enable hot reload. So it's a bit unusual to get in a bad state to begin with. This is the earliest we know of their tfm.

src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs

src/BuiltInTools/dotnet-watch/Internal/ProcessRunner.cs

Co-authored-by: Justin Kotalik <[email protected]>

captainsafia

pranavkm force-pushed the prkrishn/hot-reload branch from 5d68ec1 to 1395942 Compare March 17, 2021 17:25

pranavkm added the Area-AspNetCore RazorSDK, BlazorWebAssemblySDK, dotnet-watch label Mar 17, 2021

pranavkm force-pushed the prkrishn/hot-reload branch 3 times, most recently from 216dc0c to 05126e0 Compare March 17, 2021 23:12

pranavkm added 2 commits March 17, 2021 16:12

Add support for hot reload to dotnet-watch

d0d7c51

* Host a Roslyn workspace to produce EnC deltas and apply to app * Add "agents" to support reloading ASP.NET Core and Blazor WebAssembly apps

Add vs-impl feed

d9c1ecc

pranavkm force-pushed the prkrishn/hot-reload branch from 05126e0 to d9c1ecc Compare March 17, 2021 23:12

pranavkm requested review from BrennanConroy, captainsafia and jkotalik March 17, 2021 23:15

pranavkm requested a review from wli3 March 18, 2021 01:49

captainsafia reviewed Mar 18, 2021

View reviewed changes

pranavkm added 2 commits March 18, 2021 10:00

Merge branch 'master' into prkrishn/hot-reload

edada18

Update src/BuiltInTools/AspNetCoreDeltaApplier/Microsoft.Extensions.A…

fcb8815

…spNetCoreDeltaApplier.csproj

jkotalik reviewed Mar 18, 2021

View reviewed changes

Update src/BuiltInTools/dotnet-watch/HotReload/AspNetCoreContract.cs

bdc53ae

Co-authored-by: Justin Kotalik <[email protected]>

Base automatically changed from master to main March 18, 2021 21:05

Merge branch 'main' into prkrishn/hot-reload

a28c6d0

captainsafia approved these changes Mar 19, 2021

View reviewed changes

pranavkm merged commit fbcfcd8 into main Mar 19, 2021

pranavkm deleted the prkrishn/hot-reload branch March 19, 2021 04:09

pranavkm mentioned this pull request Jun 4, 2021

Allow multiple connections during dotnet watch run dotnet/aspnetcore#30208

Closed

eerhardt mentioned this pull request Oct 13, 2021

Build microsoft/MSBuildLocator during source-build dotnet/source-build#2510

Closed

ViktorHofer pushed a commit that referenced this pull request May 7, 2024

Bump xunit/Newtonsoft.Json libs in SB SmokeTests (#16401)

8e9a611

Add support for hot reload to dotnet-watch #16401

Add support for hot reload to dotnet-watch #16401

Uh oh!

Conversation

pranavkm commented Mar 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Mar 17, 2021

Uh oh!

pranavkm commented Mar 17, 2021

Uh oh!

pranavkm commented Mar 18, 2021

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

captainsafia left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pranavkm commented Mar 17, 2021 •

edited

Loading