-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add global usings for .NET projects (.NET SDK, Web and Worker) #18459
Conversation
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
src/WebSdk/ProjectSystem/Targets/Microsoft.NET.Sdk.Web.ProjectSystem.targets
Outdated
Show resolved
Hide resolved
src/WebSdk/ProjectSystem/Targets/Microsoft.NET.Sdk.Web.ProjectSystem.targets
Outdated
Show resolved
Hide resolved
src/WebSdk/ProjectSystem/Targets/Microsoft.NET.Sdk.Web.ProjectSystem.targets
Outdated
Show resolved
Hide resolved
src/WebSdk/ProjectSystem/content/Microsoft.NET.Sdk.Web.ProjectSystem.DefaultGlobalUsing.cs
Outdated
Show resolved
Hide resolved
src/WebSdk/ProjectSystem/content/Microsoft.NET.Sdk.Web.ProjectSystem.DefaultGlobalUsing.cs
Outdated
Show resolved
Hide resolved
src/WebSdk/ProjectSystem/Targets/Microsoft.NET.Sdk.Web.ProjectSystem.targets
Outdated
Show resolved
Hide resolved
src/WebSdk/ProjectSystem/Targets/Microsoft.NET.Sdk.Web.ProjectSystem.targets
Outdated
Show resolved
Hide resolved
src/WebSdk/ProjectSystem/Targets/Microsoft.NET.Sdk.Web.ProjectSystem.targets
Outdated
Show resolved
Hide resolved
src/WebSdk/ProjectSystem/Targets/Microsoft.NET.Sdk.Web.ProjectSystem.targets
Outdated
Show resolved
Hide resolved
src/WebSdk/ProjectSystem/content/Microsoft.NET.Sdk.Web.ProjectSystem.DefaultGlobalUsing.cs
Outdated
Show resolved
Hide resolved
src/WebSdk/ProjectSystem/Targets/Microsoft.NET.Sdk.Web.ProjectSystem.targets
Outdated
Show resolved
Hide resolved
src/WebSdk/ProjectSystem/Targets/Microsoft.NET.Sdk.Web.ProjectSystem.DefaultGlobalUsing.cs
Outdated
Show resolved
Hide resolved
src/WebSdk/ProjectSystem/Targets/Microsoft.NET.Sdk.Web.ProjectSystem.DefaultGlobalUsing.cs
Outdated
Show resolved
Hide resolved
src/WebSdk/ProjectSystem/Targets/Microsoft.NET.Sdk.Web.ProjectSystem.targets
Outdated
Show resolved
Hide resolved
src/WebSdk/ProjectSystem/Targets/Microsoft.NET.Sdk.Web.ProjectSystem.DefaultGlobalUsing.cs
Outdated
Show resolved
Hide resolved
src/WebSdk/ProjectSystem/Targets/Microsoft.NET.Sdk.Web.ProjectSystem.targets
Outdated
Show resolved
Hide resolved
src/WebSdk/ProjectSystem/Targets/Microsoft.NET.Sdk.Web.ProjectSystem.DefaultGlobalUsing.cs
Outdated
Show resolved
Hide resolved
src/WebSdk/ProjectSystem/Targets/Microsoft.NET.Sdk.Web.ProjectSystem.DefaultGlobalUsing.cs
Outdated
Show resolved
Hide resolved
2 more considerations for global usings:
|
Hmm, what are the common use cases for these two? I'd assume MVC stuff for S.CM.DA but for M.E.C.M is it really used that frequently? I feel I rarely create one, but sometimes use other things that internally rely on it. |
Ah I found some justification for the two new usings in offline conversations, colour me somewhat convinced. |
src/WebSdk/ProjectSystem/Targets/Microsoft.NET.Sdk.Web.ProjectSystem.DefaultGlobalUsing.cs
Outdated
Show resolved
Hide resolved
src/WebSdk/ProjectSystem/Targets/Microsoft.NET.Sdk.Web.ProjectSystem.targets
Outdated
Show resolved
Hide resolved
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.GenerateGlobalUsings.targets
Outdated
Show resolved
Hide resolved
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.GenerateGlobalUsings.targets
Outdated
Show resolved
Hide resolved
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.GenerateGlobalUsings.targets
Outdated
Show resolved
Hide resolved
-Depends on dotnet/sdk#18459 flowing to dotnet/aspnetcore repo before we can merge this - There's a couple of places we could remove usings from .razor files but need to verify the global usings flow into Razor compiler (#34217) - Reordered usings in some places to ensure they get emitted in alphabetical order (modulo System.* & Microsoft.* coming before any others)
Enables dotnet/winforms#5183 Relates to dotnet/sdk#18459
Enables dotnet/winforms#5183 Relates to dotnet/sdk#18459
To be honest I'm finding the global usings feature very intrusive. I'm on the 3rd dotnet/* repo that updating to 6.0.100-rc.1.*, and I have to deal with
|
A couple of questions:
|
dotnet/winforms, dotnet/winforms-designer, dotnet/wpf |
I don't think those repositories are representative of what the average developer will experience. Have you used the global usings in a real project that doesn't use arcade based repository? Also:
|
The point I'm trying to make - if a developer tries to migrate an existing project to .NET 6.0 right now the developer will be likely be "greeted" with the error above. |
Right, specifically because they will be trying to use .NET 6 with VS 2019 where C# 10 support is dodgy.
That's why the option exists to turn it off. |
I'm a bit confused here. The issue this PR references only discusses In general this feature IMHO causes more issues than it solves. The errors it can introduce in your build are very unhelpful to discover what the root cause is. There is no where in your code or project settings where those global namespaces are defined, since it is all implicit. I'm already seeing a slew of PRs in the .net repos opting out of this behavior, which to me indicates the problem with this. I can also see that WinForms and WPF are planning to add more implicit namespaces via their SDKs, Xamarin iOS and Android already did it, and I wouldn't be surprised if more nuget packages will start doing the same thing. This means that as you're adding dependencies, your code could immediately break, and it will quickly get completely out of hand. (oh and currently - temporarily - setting UseWpf to true will actually disable the feature, so again referencing a dependency might break you the other way around as well). The issue actually points out that this can go wrong if it's done too broadly which is exactly what is happening with all the SDKs starting to pile on to it:
While I do think the idea of global namespaces is great, I'm rather concerned about implicitly defining them. I'd rather this was done at a per-project level instead, either via an |
I'd like to point out that the issues aren't just for old code, they're for new code too. Adding a global using to a project template might already be breaking, but removing one from a project template is now a massive consideration that will break third party docs that reference the template. And if you can't use them in templates for above reason then what's the point? You're still showing new users all of the namespaces anyway then, realistically. |
I'm not convinced this is a bad idea yet, I think we have to make a few tweaks to deal with the set of issues we've seen so far but I haven't seen enough to make me want to pull the plug:
Some common problems that I’ve seen in the wild so far:
Yes that's a known issue. This is why we're being conservative with what we add to them. They version nicely with TFMs so an explicit upgrade is required to see new implicit usings. I think there are a few tactical things we can do here:
|
Fair enough, but like, I still don't understand the benefit. Really feels like the goal here is to smooth the very beginning first lines of C#, but like, this doesn't make the mid-level of learning C# (and to be clear this seems very specific to C#) any more straightforward. I also don't like the way that it creates side effects across files. There was a point where the mono project was trying to do single-file projects for example, with the idea being you could just import via copy/paste an entire library. Some projects still aim for this. Could we get something like "using Globals" to opt into using globals, or something stupid like it? I don't know, anything is better than this. |
That's merely a temporary problem/solution.
I'd discourage that. It's nice you can use newer and older VS (depending on user) and still work and compile the same way.
Browsing the recent commits in the various .NET repos, 'conservative' isn't what I would call adding more and more globals with each dependency (xamarin, wpf, winforms etc). And with it currently just being an item in an itemgroup, we'll start seeing nuget packages starting to abuse this as well. |
Based on what I've seen that's the bulk of the problem that I've seen so far. With a smattering of ambiguous types (MSBuild Task vs Threading Task)
Maybe, "compile the same way" is impossible when the older version of VS doesn't support the language features.
We're still in "wait and see" mode. |
Because you downgraded the language version to 9 and we currently don't gate the feature on that (though it's being discussed). |
@davidfowl Where is the best place to provide feedback on this? Commenting/voting on a closed PR seems likely to be ineffective. |
Here is a good place for now. |
Fixes dotnet/aspnetcore#32451.
TODOs:
Followups: