Skip to content

Conversation

@davidwengier
Copy link
Member

Fixes #10306

This change takes a different approach than #10308 and #10330 and #10332 because life was meant to be interesting.

The original problem we were trying to solve is that when a Reset then Add came in, we would migrate all of the documents out of the project, then migrate them all back in again. The various solutions in the other PRs you can read about at your leisure, this one takes a rather simple approach:

  1. Ignore everything but the most recent update for a specific project
  2. If we need to update a project, but it doesn't exist, add it

In the long run we should remove IRazorProjectService.AddProjectAsync entirely, and rename IRazorProjectService.UpdateProjectAsync to ResetProjectAsync, but doing that now would just conflict with other open PRs.

@davidwengier davidwengier requested a review from a team as a code owner May 2, 2024 01:08

if (newProject.TryGetDocument(documentFilePath, out var document))
{
// We don't enqueue the current document because added documents are initially closed.
Copy link
Member Author

Choose a reason for hiding this comment

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

The change is because we have too many moving pieces at the moment. When I changed things so that newly added documents stopped guessing which was the right project to be added to, it meant we didn't start generating when a document moved from the misc files project to the right one. Since the EnqueueIfNecessary already had a check for only generating open documents, this explicit lack of work made no sense to me, and seems to me like it was only valid because the "guess which project to go in" was guessing correctly most of the time.

Copy link
Member Author

Choose a reason for hiding this comment

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

"Why isn't it broken in main then?" I hear you ask. Good question! It works in main for precisely the inefficiency this PR (and all of the others 😁) is trying to solve! In main, rather than migrating one document, we fully reset the whole project, which means going to 0 tag helpers, then in a future update back to the real number of tag helpers. That change in tag helpers (among other changes) triggers a ProjectChanged change type, which is handled elsewhere in this switch, and triggers the generation of the document.

_logger.LogInformation($"Failed to update untracked project '{projectKey}'.");
return;
}
updater =>
Copy link
Member

Choose a reason for hiding this comment

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

nit: indentation

@DustinCampbell
Copy link
Member

Fourth time's the charm!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ProjectConfigurationStateSynchronizer is doing too much work

5 participants