-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Implement partial solution update support and fix bugs #49471
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements partial solution update support using new Roslyn APIs and fixes stale warnings by preserving document checksum information.
- Refactors
IncrementalMSBuildWorkspace
andCompilationHandler
to useGetSourceTextAsync
with encoding and checksum, and to call commit/discard on hot-reload updates. - Updates
HotReloadDotNetWatcher
to remove legacy baseline methods and adopt the new prompt signature. - Expands test coverage and utilities: enhances test logging, adds new Aspire no-effect auto-restart tests, and scaffolds a Web project in TestAssets.
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
test/dotnet-watch.Tests/Watch/Utilities/WatchableApp.cs | Added ASPIRE_ALLOW_UNSECURED_TRANSPORT environment variable for Aspire scenarios. |
test/dotnet-watch.Tests/Watch/Utilities/DotNetWatchTestBase.cs | Enhanced Log and UpdateSourceFile to capture caller file/line context. |
test/dotnet-watch.Tests/HotReload/CompilationHandlerTests.cs | Updated CompilationHandler constructor usage. |
test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs | Replaced direct Logger.WriteLine calls with contextual Log and expanded tests. |
test/TestAssets/TestProjects/WatchAspire/WatchAspire.Web/appsettings.json | Added default logging configuration. |
test/TestAssets/TestProjects/WatchAspire/WatchAspire.Web/appsettings.Development.json | Added development logging configuration. |
test/TestAssets/TestProjects/WatchAspire/WatchAspire.Web/WatchAspire.Web.csproj | Scaffolds new Web project. |
test/TestAssets/TestProjects/WatchAspire/WatchAspire.Web/Properties/launchSettings.json | Defines HTTP/HTTPS launch profiles. |
test/TestAssets/TestProjects/WatchAspire/WatchAspire.Web/Program.cs | Adds minimal top-level Program.cs for Web project. |
test/TestAssets/TestProjects/WatchAspire/WatchAspire.Web/Components/PhotoList.razor | Adds sample Razor component with C#12 array initializer. |
test/TestAssets/TestProjects/WatchAspire/WatchAspire.AppHost/WatchAspire.AppHost.csproj | References Web project in AppHost. |
test/TestAssets/TestProjects/WatchAspire/WatchAspire.AppHost/Program.cs | Updates DistributedApplication builder to include Web frontend. |
src/BuiltInTools/dotnet-watch/Properties/launchSettings.json | Configures non-interactive mode and Aspire env var for built-in tool. |
src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs | Refactors hot-reload callback logic and removes obsolete baseline calls. |
src/BuiltInTools/dotnet-watch/HotReload/IncrementalMSBuildWorkspace.cs | Updates GetSourceTextAsync to propagate encoding/checksum and uses C#12 syntax. |
src/BuiltInTools/dotnet-watch/HotReload/CompilationHandler.cs | Adds static commit requirement, removes legacy methods, and integrates commit/discard logic. |
Comments suppressed due to low confidence (3)
src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs:291
- The callback
FileChangedCallback
is declared asvoid
but now containsreturn
statements that return abool
. You should update its signature toasync Task<bool> FileChangedCallback(...)
(or match the expected delegate) to avoid compile errors.
question = "Do you want to restart these projects?";
src/BuiltInTools/dotnet-watch/HotReload/CompilationHandler.cs:267
- Using
return ([], []);
to return an emptyImmutableDictionary<ProjectId, string>
and an emptyImmutableArray<RunningProject>
is invalid syntax. Replace with(ImmutableDictionary<ProjectId, string>.Empty, ImmutableArray<RunningProject>.Empty)
.
return ([], []);
src/BuiltInTools/dotnet-watch/Properties/launchSettings.json:6
- [nitpick] The
workingDirectory
is an absolute path that is environment-specific. Consider using a relative path or parameterizing this value to avoid breaking on different developer machines.
"workingDirectory": "C:\\sdk1\\artifacts\\tmp\\Debug\\Aspire_BuildE---04F22750\\WatchAspire.AppHost",
@DustinCampbell ptal |
Switches to new Roslyn APIs for committing/discarding solution update and completes implementation of partial solution update: dotnet/roslyn#78244.
Fixes issue causing stale warnings being reported when updating document after rebuilding project. The updated document didn't get checksum algorithm set correctly, which caused mismatch.