-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[workload] setup C# implicit global usings from MSBuild #3267
[workload] setup C# implicit global usings from MSBuild #3267
Conversation
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.
I think this change is reasonable as-is and clearly solved a problem that we have right now that otherwise prevents using implicit using
's.
Has there been any discussion with the relevant SDK/C# teams about this broad problem of SDK clashes with implicit using's and that we might eventually want to solve this more broadly? There are countless problems that occur when you have more than one SDK (e.g. Android+iOS, or iOS+Razor) and each one "knows best," except in cases like this where they know what's best very differently from each other 😁
Last I heard there might be upcoming design for .NET 7. And the thought is right now NuGet packages shouldn't add implicit usings. (but someone probably will!) It looks like I have a merge conflict here to fix, too... |
@jonathanpeppers sounds great! |
Context: dotnet#3018 PR dotnet#3108 was a good start in enabling implicit C# global usings. However, there were a couple of issues to work through: 1. There is a `GlobalUsings.cs` file. Can we do this in MSBuild, so it's just there by default? I added: <ItemGroup Condition=" '$(UseMaui)' == 'true' and ('$(ImplicitUsings)' == 'true' or '$(ImplicitUsings)' == 'enable') "> <!-- %(Sdk) is only here if something later needs to identify these --> <Using Include="Microsoft.Extensions.DependencyInjection" Sdk="Maui" /> <Using Include="Microsoft.Maui" Sdk="Maui" /> <Using Include="Microsoft.Maui.Controls" Sdk="Maui" /> <Using Include="Microsoft.Maui.Controls.Hosting" Sdk="Maui" /> <Using Include="Microsoft.Maui.Controls.Xaml" Sdk="Maui" /> <Using Include="Microsoft.Maui.Graphics" Sdk="Maui" /> <Using Include="Microsoft.Maui.Essentials" Sdk="Maui" /> <Using Include="Microsoft.Maui.Hosting" Sdk="Maui" /> </ItemGroup> 2. The platform-specific usings were not cleared. So types like `Microsoft.Maui.Controls.Application` could conflict with `Android.App.Application`. To solve this, I added a `%(Platform)` metadata in the Android, iOS, macOS, MacCatalyst, and tvOS workloads: <Using Include="Android.App" Platform="Android" /> <Using Include="Android.Widget" Platform="Android" /> <Using Include="Android.OS.Bundle" Alias="Bundle" Platform="Android" /> Then in .NET MAUI's MSBuild targets we can do: <ItemGroup Condition=" '$(UseMaui)' == 'true' and '$(MauiEnablePlatformUsings)' != 'true' and ('$(ImplicitUsings)' == 'true' or '$(ImplicitUsings)' == 'enable') "> <Using Remove="@(Using->HasMetadata('Platform'))" /> </ItemGroup> I created `$(MauiEnablePlatformUsings)`, just in case someone ever needs to turn this off. I updated the templates to specify `ImplicitUsings=enable`. I updated `Controls.Sample.Tests.csproj`, as it was a .NET 6 project. We probably can't update the other samples yet, because there are Xamarin projects that would not support C# 10 language features.
41cd22b
to
c29655b
Compare
Description of Change
Context: #3018
Context: xamarin/xamarin-macios#13196
Context: dotnet/android#6447
PR #3018 was a good start in enabling implicit C# global usings.
However, there were a couple of issues to work through:
GlobalUsings.cs
file. Can we do this in MSBuild, soit's just there by default? I added:
Microsoft.Maui.Controls.Application
could conflict withAndroid.App.Application
.To solve this, I added a
%(Platform)
metadata in the Android, iOS,macOS, MacCatalyst, and tvOS workloads:
Then in .NET MAUI's MSBuild targets we can do:
I created
$(MauiEnablePlatformUsings)
, just in case someone everneeds to turn this off.
I updated the templates to specify
ImplicitUsings=enable
.I updated
Controls.Sample.Tests.csproj
, as it was a .NET 6 project.We probably can't update the other samples yet, because there are
Xamarin projects that would not support C# 10 language features.
PR Checklist
Does this PR touch anything that might affect accessibility?
Nope