razor icon indicating copy to clipboard operation
razor copied to clipboard

FUSE generating C# compile errors when non-FUSE doesn't

Open davidwengier opened this issue 1 year ago • 3 comments

I was trying out FUSE with a project of mine, and granted some of the packages are very old, but even given that it seems there might be a bug with FUSE and its detection of the AddComponentParameter method.

image

Turning off FUSE and restarting VS stops the errors appearing. I thought perhaps FUSE was doing a better job of identifying bad code, because I do have a very old version of QuickGrid, but a real build of the project in VS, or a command line build, works fine. The app also runs fine. I can see in the logs that we get 32 diagnostics from Roslyn, so I don't think it's on the tooling side.

Repro steps:

  1. Clone https://github.com/davidwengier/CGTCalculator/commit/4d4d21838bed12a3250a794031b35f3bdc417fea
  2. Turn on FUSE
  3. Open Transactions.razor in VS (or any other page with a component attribute)

davidwengier avatar Aug 14 '24 04:08 davidwengier

Ok, I understand what is going on here at least. In the source generator we detect if the AddComponentParameter method is available by actually looking at the reference and seeing if it's there: https://github.com/dotnet/razor/blob/bc0b702171653667ce651cdeb3cf844e1b63602f/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/SourceGenerators/RazorSourceGenerator.cs#L238-L249

We use that to set the value of https://github.com/dotnet/razor/blob/bc0b702171653667ce651cdeb3cf844e1b63602f/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/RazorCodeGenerationOptionsBuilder.cs#L96 which determines if we should use it or not.

We're not doing that on the tooling side, so we're always saying its safe to use AddComponentParameter even when it's not.

It looks like these options are set here in tooling: https://github.com/dotnet/razor/blob/bc0b702171653667ce651cdeb3cf844e1b63602f/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/LspProjectEngineFactoryProvider.cs#L52-L60

I took a quick look to see if we would be able to plumb anything through at that point, but it looks like these are set by listening to the client side. At this point I don't know enough about the tooling side to know how to proceed with fixing it.

@ryzngard Do you think you could take a look?

Note: one simple option is to just disable this always, the fallback is always valid, but that does mean we're going to be differing what code we generate in tooling vs runtime, which even though its small does affect things like hot reload. Once co-hosting comes online it won't matter, as we won't be generating code via this path, but that's going to be a while, so I think we'll want to fix this properly unless its super-duper hard to do so.

chsienki avatar Aug 16 '24 22:08 chsienki

We'll have to put something in RazorProjectInfo (formally known as project.razor.bin) if the info has to come from the compilation. Tag helper discovery is the only place we have access to a compilation. I probably should have thought about it more, and realised that was the issue 🤦‍♂️

Serialization format bump, yay!

davidwengier avatar Aug 16 '24 22:08 davidwengier

Reopening, this sadly still repros

davidwengier avatar Oct 25 '24 04:10 davidwengier

And reopening again...

davidwengier avatar Nov 06 '24 01:11 davidwengier

@davidwengier @ryzngard This is tagged as being compiler, but I don't think it is, right? It's tooling not setting the correct options?

chsienki avatar Mar 12 '25 22:03 chsienki

Yeah, its tooling. We don't plumb SuppressAddComponentParameter through to the compiler.

davidwengier avatar Mar 12 '25 23:03 davidwengier