OrchardCMS / Orchard

Orchard is a free, open source, community-focused Content Management System built on the ASP.NET MVC platform.
https://orchardproject.net
BSD 3-Clause "New" or "Revised" License
2.37k stars 1.12k forks source link

8776: Upgrade Microsoft.CodeDom.Providers.DotNetCompilerPlatform to latest version #8777

Closed BenedekFarkas closed 3 months ago

BenedekFarkas commented 3 months ago

Fixes #8776

Why do we need this package at all:

  1. Dynamic Compilation, as in modify C# code and the module will be recompiled and loaded into memory when reloading the page - it's a useful developer feature.
  2. VS Razor IntelliSense: The ASP.NET Compiler that ships with .NET Framework, even in the latest versions is an old one that doesn't support C# language features above version 5. The projects that have Razor files need this DLL to be loaded in memory (but AFAIR not necessarily strict about having it as a reference, just need to have some web.config entries).
  3. Static Razor compilation: When building the solution with MvcBuildViews=true, Razor files are compiled in-memory as an extra validation step (but they remain as .cshtml files, so they aren't compiled into a DLL like in ASP.NET Core). This is the pickiest out of these 3 features and every project that has Razor files needs this DLL reference (and some configuration).

Further changes and details besides package update:

MatteoPiovanelli-Laser commented 3 months ago

I have two questions: Why is this pinning the LangVersion? Do we need this package in so many projects?

BenedekFarkas commented 3 months ago

I have two questions: Why is this pinning the LangVersion? Do we need this package in so many projects?

Version pinning: The officially supported C# language version since .NET Framework 4.8 is 7.3 (and it probably won't go higher than that due to netstandard2.1). It is possible to build a project using a higher version (because it depends on which version your VS supports), but according to Microsoft it's not recommended to go higher than that due to potential runtime issues.


Package reference: This whole saga of adding Microsoft.CodeDom.Providers.DotNetCompilerPlatform.CSharpCodeProvider to the solution/projects serves 3 purposes - the necessary configuration has some overlap between them.

  1. Dynamic Compilation, as in modify C# code and the module will be recompiled and loaded into memory when reloading the page - it's a useful developer feature.
  2. VS Razor IntelliSense: The ASP.NET Compiler that ships with .NET Framework, even in the latest versions is an old one that doesn't support C# language features above version 5. The projects that have Razor files need this DLL to be loaded in memory (but AFAIR not necessarily strict about having it as a reference, just need to have some web.config entries).
  3. Static Razor compilation: When building the solution with MvcBuildViews=true, Razor files are compiled in-memory as an extra validation step (but they remain as .cshtml files, so they aren't compiled into a DLL like in ASP.NET Core). This is the pickiest out of these 3 features and every project that has Razor files needs this DLL reference (and some configuration).
BenedekFarkas commented 3 months ago

Made further changes and explained them in the PR description + added my previous answers there too, so they are all in one place.