dotnet / sdk

Core functionality needed to create .NET Core projects, that is shared between Visual Studio and CLI
https://dot.net/core
MIT License
2.71k stars 1.07k forks source link

[dotnet-sdk-9.0.100-preview.7.24379.15] Fail to build CleanArchitecture2 app with an error "CS0619:'MudMenu.OffsetY' is obsolete: 'Use AnchorOrigin or TransformOrigin instead.'" #42509

Closed Junjun-zhao closed 3 weeks ago

Junjun-zhao commented 3 months ago

Application Name: CleanArchitecture2 OS: Windows 10 21H2 CPU: X64 .NET Build Number: dotnet-sdk-9.0.100-preview.7.24379.15 App Source checking at: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2183568 Github Link: https://github.com/blazorhero/CleanArchitecture

Verify Scenarios: 1). Windows 10 21H2 AMD64 + dotnet-sdk-6.0.424:Pass 2). Windows 10 21H2 AMD64 +dotnet-sdk-9.0.100-preview.6.24328.19: Pass 3). Windows 10 21H2 AMD64 +dotnet-sdk-9.0.100-preview.7.24379.15: Fail

Description : When build the 3rd party application with the latest .NET 9 build "dotnet-sdk-9.0.100-preview.7.24379.15", it throws obsolete exception such as "error CS0619: 'MudMenu.OffsetX' is obsolete: 'Use AnchorOrigin or TransformOrigin instead". After investigation, we found it is related to the MudBlazor Nuget package(Version : 6.0.2).

This does not reproduce on dotnet-sdk-9.0.100-preview.6.24328.19. Could you help confirm whether this is expected? If Yes, is this a breaking change?

Findings: The CS0619 error will be gone after upgrade the MudBlazor nuget package from V6.0.2 to the latest V7.5.0.

Minimal Repro Steps: The machine has dotnet-sdk-9.0.100-preview.7.24379.15 installed

  1. Create Blazor Server app with .NET 6.0
  2. Install MudBlazor nuget package (Version : 6.0.2)
  3. Add the following to _Imports.razor @using MudBlazor
  4. Add the following to _Host.cshtml
    
    <link href="https://fonts.googleapis.com/css?family=Roboto:300,400,500,700&display=swap" rel="stylesheet" />
    <link href="_content/MudBlazor/MudBlazor.min.css" rel="stylesheet" />
6. Add the following to MainLayout.razor

@inherits LayoutComponentBase

BlazorAppDemo
8. Build app with 9.0.100-preview.7.24379.15 SDK

**Expected Result:**
App build successfully.

**Actual Result:**
App build with error:

D:\ReproBugDeMO\BlazorAppDemo\BlazorAppDemo\obj\Debug\net6.0\Microsoft.CodeAnalysis.Razor.Compiler\Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator\Shared_MainLayout_razor.g.cs(114,51): error CS0619: 'MudMenu.Direction' is obsolete: 'Use AnchorOrigin or TransformOrigin instead.' D:\ReproBugDeMO\BlazorAppDemo\BlazorAppDemo\obj\Debug\net6.0\Microsoft.CodeAnalysis.Razor.Compiler\Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator\Shared_MainLayout_razor.g.cs(131,51): error CS0619: 'MudMenu.OffsetX' is obsolete: 'Use AnchorOrigin or TransformOrigin instead.' D:\ReproBugDeMO\BlazorAppDemo\BlazorAppDemo\obj\Debug\net6.0\Microsoft.CodeAnalysis.Razor.Compiler\Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator\Shared_MainLayout_razor.g.cs(148,52): error CS0619: 'MudMenu.OffsetY' is obsolete: 'Use AnchorOrigin or TransformOrigin instead.'


**App Repro steps:**
The machine has dotnet-sdk-9.0.100-preview.7.24379.15 installed.
1. Copy the source code to your locally machine.
2. cd to CleanArchitecture2\src\Server folder.
3. Build app with 9.0.100-preview.7.24379.15 SDK

**Expected Result:**
App build successfully.

**Actual Result:**
Fail to build the app with error: 
```D:\CleanArchitecture2\src\Client\obj\Debug\net6.0\Microsoft.CodeAnalysis.Razor.Compiler\Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator\Shared_MainBody_razor.g.cs(702,56): error CS0619: 'MudMenu.Direction' is obsolete: 'Use AnchorOrigin or TransformOrigin instead.' 
D:\CleanArchitecture2\src\Client\obj\Debug\net6.0\Microsoft.CodeAnalysis.Razor.Compiler\Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator\Shared_MainBody_razor.g.cs(719,56): error CS0619: 'MudMenu.OffsetY' is obsolete: 'Use AnchorOrigin or TransformOrigin instead.'
D:\CleanArchitecture2\src\Client\obj\Debug\net6.0\Microsoft.CodeAnalysis.Razor.Compiler\Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator\Shared_MainBody_razor.g.cs(1657,53): error CS0619: 'MudMenu.OffsetX' is obsolete: 'Use AnchorOrigin or TransformOrigin instead.'

Dotnet Info:

.NET SDK:
 Version:           9.0.100-preview.7.24379.15
 Commit:            812e813ae9
 Workload version:  9.0.100-manifests.28aae763
 MSBuild version:   17.12.0-preview-24374-02+48e81c6f1

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.19045
 OS Platform: Windows
 RID:         win-x64
 Base Path:   C:\Program Files\dotnet\sdk\9.0.100-preview.7.24379.15\

Host:
  Version:      9.0.0-preview.7.24376.15
  Architecture: x64
  Commit:       static

.NET SDKs installed:
  9.0.100-preview.7.24379.15 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 9.0.0-preview.7.24379.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 9.0.0-preview.7.24376.15 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 9.0.0-preview.7.24379.3 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

@dotnet-actwx-bot @dotnet/compat

Junjun-zhao commented 3 months ago

@marcpopMSFT This issue was found during our source compat testing for the 3rd party application. Could you please help check and triage it? In the meantime, could you confirm if this is a blocker for .NET 9 Preview 7?

Please help transfer to the correct team if this is not correct. Thanks very much.

javiercn commented 3 months ago

@chsienki @jaredpar this seems to be Razor compiler related. Can you take a look?

chsienki commented 3 months ago

Looking.

chsienki commented 3 months ago

Interesting, this is related to the FUSE work. Where before we we're just emitting strings, we're now using nameof() so have a real reference to the backing property, leading to the C# compiler (correctly) issuing a diagnostic about it being obsolete.

What's more interesting is that the design time code was correctly issuing the diagnostics image

But you were still able to build due to the difference in runtime and design time code generation strategies.

mkArtakMSFT commented 3 months ago

My opinion on this is that this is not a bug, in fact it's the correct / desired behavior. So the expectations from the customers should be adjusted accordingly. So perhaps the compatibility testing app validation scenarios should be adjusted to account for this change.

PriyaPurkayastha commented 3 months ago

Is the change introduced as part of the FUSE work considered as a breaking change or is it considered to be fixing a bug that was previously either being ignored or handled differently? Compat lab did not create this app. So we will need to notify the app owner and let them know that they need to make changes in their app to unblock adoption of .NET 9. The question is how common such usage could be in the real world and how many apps in the real world could be impacted similarly. Would you have documentation to make them aware of this change and what they need to do or reset customer expectations? cc @marklio

marklio commented 3 months ago

Another question... Are these our APIs that are obsoleted? Or are these obsoletions in the MudBlazor library? If they are ours, were they obsoleted following the appropriate deprecation policy? (it seems odd that they are "throw error" deprecated. I don't see warnings as errors in this app)

chsienki commented 3 months ago

These are obsoletions in MudBlazor itself. The tested app is using properties that have been deprecated, and previously on the command line we wern't correctly surfacing the message (note that we were in the IDE).

I would argue that this is a case of fixing a bug, not introducing a breaking change: the library was correctly telling the application not to use these properties and we were incorrectly not informing the user.

Further, at the top the issue it's mentioned that these go away if you upgrade to MudBlazor 7.5.0. Looking at the source for MudBlazor these properties have been removed (hence the deprecation notice presumably). With the backing properties removed, the attributes just become HTML attributes which means the app is now broken at runtime, with no explanation to the user why.

marklio commented 3 months ago

So, we are now properly surfacing errors as the library has already declared. That makes sense. I assume that these "worked" at runtime?

With the backing properties removed, the attributes just become HTML attributes which means the app is now broken at runtime, with no explanation to the user why.

I realize it is orthogonal, but that seems like unfortunate behavior, particularly since we've been hiding the opportunity for obsoletion. Again, I don't think this really matters if adding "plan ol HTML attributes" to things is common. Might be nice to have an analyzer type thing that let you know the attribute isn't bound to anything. Again, I have no idea how useful/common that is.

chsienki commented 3 months ago

I realize it is orthogonal, but that seems like unfortunate behavior, particularly since we've been hiding the opportunity for obsoletion. Again, I don't think this really matters if adding "plan ol HTML attributes" to things is common. Might be nice to have an analyzer type thing that let you know the attribute isn't bound to anything. Again, I have no idea how useful/common that is.

Yeah, this is interesting. I actually misspoke about these ending up as HTML attributes, it's up to the component in question what it does with them, but it's still valid to add arbitrary attributes to the component: https://learn.microsoft.com/en-us/aspnet/core/blazor/components/splat-attributes-and-arbitrary-parameters?view=aspnetcore-8.0#arbitrary-attributes

We might consider a warning when the component in question doesn't implement that feature, but that would definitely have to be behind a warning wave, which we haven't done yet. I'll make sure to open an issue to track that so we don't forget it.

chsienki commented 3 months ago

See: https://github.com/dotnet/Razor-Language-Design/discussions/9 and https://github.com/dotnet/razor/pull/9144

Junjun-zhao commented 3 weeks ago

Closing this issue as it is by design according to comment https://github.com/dotnet/sdk/issues/42509#issuecomment-2265937684 and a new complier warning filed in warning wave https://github.com/dotnet/sdk/issues/42509#issuecomment-2266049204