-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Switch to just using paths as keys for file watching #74877
Switch to just using paths as keys for file watching #74877
Conversation
@@ -31,7 +31,7 @@ internal sealed class FileWatchedReferenceFactory<TReference> | |||
/// are only created once we are actually applying a batch because we don't determine until the batch is applied if | |||
/// the file reference will actually be a file reference or it'll be a converted project reference. | |||
/// </summary> | |||
private readonly Dictionary<TReference, (IWatchedFile Token, int RefCount)> _referenceFileWatchingTokens = []; | |||
private readonly Dictionary<string, (IWatchedFile Token, int RefCount)> _referenceFileWatchingTokens = []; |
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.
this is the crux of the change. it keeps the token/ref-count assocaited with the path, not the reerence identity anymore.
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.
Dumb question: Could _previousDisposalLocations key on the path too?
@jasonmalinowski @ToddGrun @dibarbet ptal :) |
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.
This simplifies work happening in #74780
In that PR we want to be able to make things like AnalyzerFileReferences temporarily, but switch out with IsolatedReferences to get dedicated ALCs. If we use the analyzer ref itself as a key, this becomes much more complicated. However, if all we're using is the path anyways (for both listening to notifications, and for reporting them externally), then it's simpler to just base the API on that.