Skip to content

Changes to Microsoft.AspNetCore.Mvc.Internal #8678

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

Closed
Stamo-Gochev opened this issue Mar 20, 2019 · 13 comments
Closed

Changes to Microsoft.AspNetCore.Mvc.Internal #8678

Stamo-Gochev opened this issue Mar 20, 2019 · 13 comments
Assignees
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates question

Comments

@Stamo-Gochev
Copy link

Stamo-Gochev commented Mar 20, 2019

Third-party libraries that relied on the (previously) public classes from Microsoft.AspNetCore.Mvc.Internal are now broken after the update in 8d66f10

This prevents us from supporting .NET Core 3.0.

The discussion regarding this change is in #4932

Changes for similar pubinternal types also affect us: b18526c

@pranavkm Can you suggest an alternative to using the now internal classes? Copying these files as public in the third-party library code raises both license and maintenance issues.

@mkArtakMSFT mkArtakMSFT added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Mar 20, 2019
@mkArtakMSFT
Copy link
Contributor

Thanks for contacting us, @Stamo-Gochev.
What were the specific types you relied on and in which ways?

@Stamo-Gochev
Copy link
Author

Here is a list of the main classes:

They are used for building custom expressions for sorting, grouping, filtering, etc. Some of the code from the pubinternal classes are complex enough and we relied on leveraging them to build these expressions.

What I can suggest as a better approach for handling the main point from the discussion:

In ASP.NET Core, pubinternal types are types that are declared as public but put in an .Internal namespace. While these types are public they have no support policy and are subject to breaking changes. Unfortunately accidental use of these types has been common, resulting in breaking changes to these projects and limiting our ability to maintain the framework.

is to extract these internal namespaces in separate nuget packages as they were before making them internal. This will allow third-party libraries to reference the package with a specific version - if an update is made (as they are used by other parts of ASP.NET internally) that introduces a breaking change, then the third-party lib can stick to the previous version or update and see if this breaking change affects it at all.

@pranavkm
Copy link
Contributor

We addressed this particular gap as part of aspnet/Mvc#8724. 3.0 offers a ModelExpressionProvider type that's available from DI and combines the 3 APIs that were previously available:

  • ModelExpressionProvider.GetExpressionText<TModel, TValue>(Expression<Func<TModel, TValue>>)
  • ModelExpressionProvider.CreateModelExpression<TModel, TValue>(ViewDataDictionary<TModel>, string expression)
  • ModelExpressionProvider.CreateModelExpression<TModel, TValue>(ViewDataDictionary<TModel>, Expression<Func<TModel, TValue>> expression)

The latter two return an instance of ModelExpression which is a superset for ModelExplorer that ExpressionMetadataProvider returns. aspnet/Mvc#8724 (comment) has a suggestion for libraries that need to support different versions of AspNetCore.

is to extract these internal namespaces in separate nuget packages as they were before making them internal.

We specifically wanted to reduce the volume of API that MVC exposed that was unsupported. Extracting the feature to a separate package would essentially require for us to support it in some format. If you absolutely must use an unsupported API, copying the source code works just as well.

@Stamo-Gochev
Copy link
Author

Stamo-Gochev commented Mar 21, 2019

The ModelExpressionProvider class wraps the ExpressionHelper.GetExpressionText method, but we are using it in a static class that provides extension methods, so DI is not possible (I am ignoring ideas like property/method injection in these extension methods).

@Stamo-Gochev
Copy link
Author

One more thing about the changes to Microsoft.AspNetCore.Mvc.ViewFeatures.Internal from the issue you linked - what is the equivalent for ExpressionTextCache in 3.0? Is this just a ConcurrentDictionary<LambdaExpression, string>?

@pranavkm
Copy link
Contributor

If you have access to HttpContext in your extension method (typically true for IHtmlHelper extensions), you should be able to access ModelExpressionProvider using HttpContext.RequestServcies. ModelExpressionProvider manages caching, so there isn't a equivalent API for ExpressionTextCache.

@pranavkm
Copy link
Contributor

@Stamo-Gochev feel free to reopen if you have further questions

@Stamo-Gochev
Copy link
Author

Stamo-Gochev commented Mar 29, 2019

@pranavkm
Thanks for the clarifications.

Can you provide some details whether Microsoft.AspNetCore.Mvc.ViewFeature will be upgraded to 3.0 as a standalone package? Is it supposed to remain locked to 2.x for the LTS and the way to get version 3.0 is by installing Microsoft.AspNetCore.App 3.0? This seems like the only way to get the new version of the APIs - the suggestion from aspnet/Mvc#8724 (comment) is what works.

@pranavkm
Copy link
Contributor

Can you provide some details whether Microsoft.AspNetCore.Mvc.ViewFeature will be upgraded to 3.0 as a standalone package?

It will be available as part of the shared framework in 3.0. Here's a couple of issues that detail what this entails:

@dodyg
Copy link

dodyg commented Apr 3, 2019

I need the following from Microsoft.AspNetCore.Mvc.Internal;

  • MvcRouteHandler
  • AttributeRouting

https://github.com/OrchardCMS/OrchardCore/blob/d729a2048a4390380d99e7be9f7968ff8f8e9271/src/OrchardCore/OrchardCore.Mvc.Core/ModularTenantRouteBuilder.cs

I am trying to port Orchard Core Framework to ASP.NET Core 3.0

Also from Microsoft.AspNetCore.Mvc.Razor.Internal

  • IRazorViewEngineFileProviderAccessor ,
  • IViewCompilationMemoryCacheProvider

https://github.com/OrchardCMS/OrchardCore/blob/13a1fd2b1edbe5278d3572db76e1b96ddf3ba93a/src/OrchardCore/OrchardCore.Mvc.Core/Startup.cs

@pranavkm
Copy link
Contributor

pranavkm commented Apr 3, 2019

@dodyg could you please file a new issue?

@joetherod
Copy link

This is terrible. How do you rip out features like this without easily being able to make it work? Upgrading to 3.0 is practicality a rewrite. Microsoft was a lot better with this. Seems like you do these things without caring with the amount of code already out there.

@pranavkm
Copy link
Contributor

@joetherod could you file a separate issue if there are other APIs that affect your particular scenarios?

@dotnet dotnet locked as resolved and limited conversation to collaborators Sep 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates question
Projects
None yet
Development

No branches or pull requests

5 participants