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

Create a consistent and reliable API between Razor IDE and compiler #11182

Open
davidwengier opened this issue Nov 8, 2024 · 5 comments
Open
Assignees
Labels
area-compiler Umbrella for all compiler issues

Comments

@davidwengier
Copy link
Contributor

Right now the IDE configures the compiler by:

This has all generally been a bug farm in the past, makes it easy for things to be missed, and means there is no good way for someone working in the compiler to know if they will need IDE support for their work, nor someone working in the IDE know what they need to provide the compiler in order to get a coherent experience.

If we could centralise on a single strongly typed API that contained everything the Razor compiler needs from the IDE, it should make these issues go away. The simplest idea, from an IDE perspective, would be to add more parameters to the RazorConfiguration constructor, and get rid of the configure lambda entirely, and have that be the contract. Open to all thoughts.

/cc @DustinCampbell @333fred @chsienki

@chsienki chsienki self-assigned this Nov 8, 2024
@chsienki chsienki added this to the 17.13 Planning milestone Nov 8, 2024
@chsienki chsienki added the area-compiler Umbrella for all compiler issues label Nov 8, 2024
@jjonescz
Copy link
Member

jjonescz commented Nov 8, 2024

Didn't we want to use the razor source generator as the source of all compiler truth, and tooling just reading its outputs (#6919)? Then those outputs would be the only API between tooling and compiler, I presume.

@davidwengier
Copy link
Contributor Author

Yes, thats what cohosting will deliver, and will make this issue moot, but thats also not a short term thing.

@ady109
Copy link
Contributor

ady109 commented Nov 8, 2024

@davidwengier out of interest when is co-hosting expected to be delivered? Secondly what benefit(s) will us Visual Studio and/or VS Code users see in our day to day development loop using it?

@davidwengier
Copy link
Contributor Author

When is a very tough question to answer. We're about 90% of the way through porting each individual Razor feature over (#10364) but there is one particularly big rock that needs to move (turning on the source generator in the IDE) and that will probably uncover a lot of issues in Roslyn (eg dotnet/roslyn#72136, #10856, #10864, #10865 and a lot of as-yet-discovered issues I'm sure).

As for benefits, the main things to start with are performance and reliability. Performance because right now Razor runs its own project system, and has hooks into the .NET project system, and Roslyn, that get data and respond to events, and send it into Razor so it can be absorbed into it's project system etc. With cohosting Razor will essentially exist inside Roslyn, so all of that work will have already been done and we're just re-using the existing Roslyn project system. Reliability, and some performance, comes from Razor not having to compile the generated C# files and keep them synchronized with Roslyn, and make sure when we ask Roslyn about them we've send the right version across etc. because, again, everything will be in the same world.

In future we can then build on it to get more features available in Razor too. eg, because our current communication method with Roslyn is LSP, we essentially have a string based API which limits what we can do. When we have cohosting we can upgrade some of those APIs to talk in terms of Roslyn data structures, like Documents or SyntaxTrees and it will be possible to reason more about what is happening, and provide features based on it.

@ady109
Copy link
Contributor

ady109 commented Nov 10, 2024

@davidwengier thanks for the detailed explanation.
I look forward to the future reliability and performance benefits.

davidwengier added a commit that referenced this issue Nov 12, 2024
Preparation for ongoing work to hook up the Roslyn tokenizer and
#11182 I suppose.

There were three places that `UpdateProjectWorkspaceState` was called:

1. In `RazorProjectService`, just before calling
`UpdateProjectConfiguration`
2. In `ProjectWorkspaceStateGenerator`, where we will need to add a call
to `UpdateProjectConfiguration` in future, to wire up the tokenizer
3. In our LiveShare bits, in response to events from the above.

Previous attempts to plumb through more things for `RazorConfiguration`
resulted in RPS failures, that appeared to be simply more compilations
of closed files. This makes sense because we were adding another update,
which would have triggered another set of `ProjectChanged` events. I
thought it would make more sense to combine these two updates together,
so no matter which part of the project was being updated, there could be
a single `ProjectChanged` notification. This is that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-compiler Umbrella for all compiler issues
Projects
None yet
Development

No branches or pull requests

4 participants