Fix a couple of issues with dotnet-watch #17608
Conversation
pranavkm
commented
May 13, 2021
pranavkm
commented
- Print compilation errors in red and don't say hot reload succeeded when there's errors
- Fix an issue where the browser does not refresh after changes
- Fix an issue where deltas are not applied on assemblies loaded after the fact.
- Fix an issue where hot reload no longer works after an app restart.
- Fix a local dev issue to avoid using the local SDK when running dotnet-watch as an executable
* Print compilation errors in red and don't say hot reload succeeded when there's errors * Fix an issue where the browser does not refresh after changes * Fix an issue where deltas are not applied on assemblies loaded after the fact. * Fix an issue where hot reload no longer works after an app restart. * Fix a local dev issue to avoid using the local SDK when running dotnet-watch as an executable
|
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. |
| @@ -196,6 +196,10 @@ public void ApplyDeltas(IReadOnlyList<UpdateDelta> deltas) | |||
| { | |||
| System.Reflection.Metadata.AssemblyExtensions.ApplyUpdate(assembly, item.MetadataDelta, item.ILDelta, ReadOnlySpan<byte>.Empty); | |||
| } | |||
|
|
|||
| // Additionally stash the deltas away so it may be applied to assemblies loaded later. | |||
| var cachedDeltas = _deltas.GetOrAdd(item.ModuleId, static _ => new()); | |||
There was a problem hiding this comment.
We were applying these deltas, but I missed storing them.
| @@ -53,11 +53,6 @@ public static async Task ReceiveDeltas(HotReloadAgent hotReloadAgent) | |||
| Log("Attempting to apply deltas."); | |||
|
|
|||
| hotReloadAgent.ApplyDeltas(update.Deltas); | |||
|
|
|||
| // We want to base this off of mvids, but we'll figure that out eventually. | |||
| var applyResult = update.ChangedFile is string changedFile && changedFile.EndsWith(".razor", StringComparison.Ordinal) ? | |||
There was a problem hiding this comment.
I changed the heuristic about when we refresh the browser in web apps - we won't it if it looks like a Blazor app (presence of any .razor file). Works better than relying on individual files.
| @@ -171,7 +179,7 @@ private ImmutableArray<string> GetDiagnostics(Solution solution, CancellationTok | |||
| if (item.Severity == DiagnosticSeverity.Error) | |||
| { | |||
| var diagnostic = CSharpDiagnosticFormatter.Instance.Format(item); | |||
| _reporter.Output(diagnostic); | |||
| _reporter.Output("\x1B[40m\x1B[31m" + diagnostic); | |||
| #if DEBUG | ||
| // In the usual case, use the SDK that contains the dotnet-watch. However during local testing, it's | ||
| // much more common to run dotnet-watch from a different SDK. Use the ambient SDK in that case. | ||
| MSBuildLocator.RegisterDefaults(); |
There was a problem hiding this comment.
Annoying local build issue where it would use the dogfood SDK. This was causing the annoying FailFast error that I poked you about.
| { | ||
| if (_pipe is not null) |
There was a problem hiding this comment.
Why was this removed? It seems like good hygiene to have it?
There was a problem hiding this comment.
Turns out we were newing up these instances in every hot reload loop. So this was never true. Also why the named pipe name is a static because we were only configuring it once.
