Skip to content
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

Watch project.assets.json files for nuget restore #70602

Merged
merged 2 commits into from
Oct 27, 2023

Conversation

dibarbet
Copy link
Member

Previously, we were not reloading the project when a restore happened, until you actually modified a file. This adds a file watcher for the project.assets.json file to tell us if a restore has happened and that the server needs to reload the project.

After:

[Trace - 11:15:07 AM] Sending notification 'workspace/didChangeWatchedFiles'.
Params: {
    "changes": [
        {
            "uri": "file:///c:/Users/dabarbet/source/repos/trash/MultipleTestProjects/XunitProject/obj/project.assets.json",
            "type": 1
        }
    ]
}

@dibarbet dibarbet requested a review from a team as a code owner October 27, 2023 18:18
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Oct 27, 2023
@@ -216,6 +219,27 @@ void UpdateProjectSystemProjectCollection<T>(IEnumerable<T> loadedCollection, IE
if (newItems.Count != oldItemsCount)
logger.LogTrace(logMessage, projectFullPathWithTargetFramework, newItems.Count);
}

void WatchProjectAssetsFile(ProjectFileInfo currentProjectInfo, IFileChangeContext fileChangeContext)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to only watch the actual msbuild property for ProjectAssetsFile here - I took a look at a bit of the O# and CPS code, and it looked they were only looking at that specific property (not for any project.assets.json file in obj/).

Copy link
Member

@jasonmalinowski jasonmalinowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functionally fine, but you don't need the record since you can already grab the previous data to compare against.

@dibarbet dibarbet merged commit c9bab46 into dotnet:main Oct 27, 2023
22 of 24 checks passed
@dibarbet dibarbet deleted the watch_project_assets_json branch October 27, 2023 22:59
@ghost ghost added this to the Next milestone Oct 27, 2023
@jjonescz jjonescz modified the milestones: Next, 17.9 P1 Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants