Fix module template Startup namespaces (Lombiq Technologies: OCORE-111)

[Piedone]

Without these namespaces, a freshly created solution will have these build errors:

[hishamco]

Do we have implicit using turn on by default? /cc @agriffard

[Piedone]

We have but that only generates this:

global using global::System;
global using global::System.Collections.Generic;
global using global::System.IO;
global using global::System.Linq;
global using global::System.Net.Http;
global using global::System.Threading;
global using global::System.Threading.Tasks;

[Skrypt]

It worked for me when I tested them. Of course it can maybe fail to compile based on your solution settings.

[ns8482e]

I don’t think we need this, if the project is web Microsoft.AspNetCore.* will be added

When I tested web/module template it was working, can you confirm if you have Web sdk in csproj?

[ns8482e]

Do we have implicit using turn on by default? /cc @agriffard

Yes implicit using is enabled by default in project templates

[Piedone]

This is with a web project in the root if this is what you mean (as well as the module, but that comes from the template). But see for yourself, the resulting solution won't build:

dotnet new occms -o "src/Demo.Web"
dotnet new sln -n "Demo"
dotnet sln "Demo.sln" add "src/Demo.Web/Demo.Web.csproj"
dotnet new ocmodulecms -n "Demo.Module" -o "src/Modules/Demo.Module"
dotnet add "src/Demo.Web/Demo.Web.csproj" reference "src/Modules/Demo.Module/Demo.Module.csproj"
dotnet sln "Demo.sln" add "src/Modules/Demo.Module/Demo.Module.csproj"

[ns8482e]

@Piedone Sorry my bad - What I meant by I don’t think we need this, is that this is not correct fix as suggested in the PR.

With implicit using enabled, using is automatically added for web project sdk, Correct fix would be, in module use <Project Sdk="Microsoft.NET.Sdk.Web"> instead of <Project Sdk="Microsoft.NET.Sdk.Razor"> in template OrchardCore.Templates.Cms.Module.csproj and also in following templates

OrchardCore.Templates.Theme.csproj
OrchardCore.Templates.Mvc.Module.csproj

[Piedone]

The modules are not web projects per se, though. But semantics aside, the Razor SDK needs to be used in modules and themes, otherwise, Razor templates won't work when you publish the app. So, I still see importing the namespaces as the solution.

The MVC module needs the same fix indeed, so I've added that.

[ns8482e]

Orchard Modules are not Web, However it's also not same as RazorLibs. It's kind somewhere in between as It may have Startup.cs and Controllers and wwwroot like Web.

So IMHO with implicit using enabled and keeping Microsoft.NET.Sdk.Razor is not helping at all - as you are adding an using statements even if it's enabled.

If you prefer to keep Microsoft.NET.Sdk.Razor in modules then we should be adding all implicit using required by OC module into module target's prop file so that those gets added when Module Target is referenced.

[ns8482e]

Related #12676

[Piedone]

Using the targets props for implicit usings is an interesting idea. I've added an example of this (not complete, only for modules), please check.

However, I don't really like it. To begin, Microsoft.AspNetCore.Builder is, looking at the code of Orchard, for the overwhelming majority only used in Startup classes. Microsoft.AspNetCore.Routing is a bit more popular (160 instances vs 90), and Microsoft.Extensions.DependencyInjection more (540). But even that is just below 3 instances per project on average. I wouldn't want to needlessly pollute the local namespace by importing rarely used namespaces, and these cases just look like we should add the imports in the tempalte and that's the end of it.

What does everyone think?

[hishamco]

No need to introduce another #if block while it's already there in line 4

[hishamco]

Suggested change

#if (AddPart)

[Piedone]

Yeah, with the new central imports indeed not, though I'm awaiting feedback on that.

[hishamco]

Suggested change

#endif

[Piedone]

Since apparently nobody has a strong opinion on the props imports I mentioned in my last comment, I'm merging this in the state Antoine approved.