Skip to content

Support view components in Go To Def#12222

Merged
davidwengier merged 5 commits intodotnet:mainfrom
davidwengier:GoToDefViewComponent
Sep 18, 2025
Merged

Support view components in Go To Def#12222
davidwengier merged 5 commits intodotnet:mainfrom
davidwengier:GoToDefViewComponent

Conversation

@davidwengier
Copy link
Copy Markdown
Member

Fixes #12215

Got nerd sniped since this should have been fixed by my previous PR, but it wasn't. Turns out the Razor compiler is weird, so I added some more metadata to un-weird it. I deliberately decided not to serialize and deserialize that metadata, because it's only used by this feature which only works in cohosting. If anyone disagrees with that, please let me know. If the future is cohosting then I believe that all goes away anyway.

@davidwengier davidwengier requested a review from a team as a code owner September 12, 2025 01:47
Copy link
Copy Markdown
Member

@DustinCampbell DustinCampbell left a comment

Choose a reason for hiding this comment

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

I deliberately decided not to serialize and deserialize that metadata, because it's only used by this feature which only works in cohosting. If anyone disagrees with that, please let me know. If the future is cohosting then I believe that all goes away anyway.

I disagree completely with this notion. This assumption will almost certainly result in significant bugs if the plan to include serialized tag helpers in the .NET SDK is ever realized. The current serialization work done around tag helpers is intended to support that effort later, and deliberately leaving holes in serialization now will result in a lot of headaches later.

Comment thread src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/RemoteTagHelperSearchEngine.cs Outdated
@davidwengier
Copy link
Copy Markdown
Member Author

if the plan to include serialized tag helpers in the .NET SDK is ever realized. The current serialization work done around tag helpers is intended to support that effort later

That is definitely something I had not considered/forgotten about. I was thinking of that as an optimization around serializing the same things on every project load, to communicate back to the Razor server. If that is still on the cards, then it definitely breaks my assumption.

@davidwengier
Copy link
Copy Markdown
Member Author

davidwengier commented Sep 17, 2025

Added the serialization. I'm doing a test insertion out of curiosity, but now that cohosting is on by default there aren't many tests that will be affected :)

Val build: https://dev.azure.com/dnceng/internal/_build/results?buildId=2795340
Test insertion: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/671028

Copy link
Copy Markdown
Member

@DustinCampbell DustinCampbell left a comment

Choose a reason for hiding this comment

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

Thanks!

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.

Go to Definition: possible to enable it for MVC/Razor Pages ViewComponents?

3 participants