aspnet / Mvc

[Archived] ASP.NET Core MVC is a model view controller framework for building dynamic web sites with clean separation of concerns, including the merged MVC, Web API, and Web Pages w/ Razor. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
5.62k stars 2.14k forks source link

Make ExpressionHelper.GetExpressionText, ExpressionMetadataProvider.From* public #8724

Closed San4es closed 5 years ago

San4es commented 5 years ago

We are using the following public APIs from the Microsoft.AspNetCore.Mvc.ViewFeatures namespace: ExpressionHelper.GetExpressionText ExpressionMetadataProvider.FromLambdaExpression ExpressionMetadataProvider.FromStringExpression

in our custom HtmlHelper extensions to integrate them with a model and client-side validation:

Making these types internal in #8705 you break our integration with the model. Are there public replacements for these APIs?

mkArtakMSFT commented 5 years ago

Thanks for contacting us, @San4es. As you know, these APIs were never meant to be consumed by our customers. While that was the case, we understand that some of these changes would break some apps, as peopled do consume what they get access to after all. @pranavkm, what would be the way to go around this? Are there alternative APIs to be used here?

pranavkm commented 5 years ago

Are there alternative APIs to be used here?

Not that I'm aware of. The equivalent APIs were public in MVC5, so we these seem like good candidates to be made public. I'll bring this up in the API review meeting today.

pranavkm commented 5 years ago

We have an API review for these types and have a draft proposal for this. The plan is to make ExpressionHelper and ExpressionMetadataProvider services in the DI container with the followship shape:

public class ExpressionHelper
{
    string GetExpressionText(LambdaExpression expression);
    string GetCachedExpressionText(LambdaExpression expression);
}

public class ExpressionMetadataProvider
{ 
   ModelExplorer FromLambdaExpression<TModel, TResult>(
            Expression<Func<TModel, TResult>> expression,
            ViewDataDictionary<TModel> viewData);

   ModelExplorer FromStringExpression(
            string expression,
            ViewDataDictionary viewData);
}

Moving this issue to 3.0.0-preview1 milestone

San4es commented 5 years ago

@pranavkm @mkArtakMSFT

Thank you for your replies.

The plan is to make ExpressionHelper and ExpressionMetadataProvider services in the DI ExpressionMetadataProvider -> public and make it non-static (put it in DI)

Do I understand correctly that the code for getting an instance of this classes in my helpers will look like below?

var expressionMetadataProvider = ViewContext.HttpContext.RequestServices.GetService<ExpressionMetadataProvider>();
var expressionHelper = ViewContext.HttpContext.RequestServices.GetService<ExpressionHelper>();

How can I save backward compatibility with ASP.NET Core 2.x simultaneously with 3.x versions?

pranavkm commented 5 years ago

Do I understand correctly that the code for getting an instance of this classes in my helpers will look like below?

Most likely if these are static extension methods on IHtmlHelper. @dougbu had some design suggestions here that I did not note down.

How can I save backward compatibility with ASP.NET Core 2.x simultaneously with 3.x versions?

Even if we were to leave the signature as-is and make the types public, at minimum, you'll have to reconcile referencing the namespace differences between 2.0 and 3.0. 3.0 also adds additional dimensions to multi-targeting and I think @davidfowl or @natemcmaster might have more insights in to what package authors need to do.

natemcmaster commented 5 years ago

more insights in to what package authors need to do.

I don't have full context on what you are trying to do, but in general, if you want to write a library that supports multiple versions of ASP.NET Core, you need to multi-target. Your project file might look like this:

<PropertyGroup>
    <TargetFrameworks>netstandard2.0;netcoreapp3.0</TargetFrameworks>
</PropertyGroup>

<ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
    <PackageReference Include="Microsoft.AspNetCore.Mvc" Version="2.1.0" />
</ItemGroup>
<ItemGroup Condition="'$(TargetFramework)' == 'netcoreapp3.0'">
    <FrameworkReference Include="Microsoft.AspNetCore.App" />
</ItemGroup>

In C#, you change which API you compile for using #if's.

#if NETCOREAPP3_0

    NewApiFrom30.DoSomething()

#else

    Internal21Api.DoSomething()

#endif
davidfowl commented 5 years ago

"supports" is too broad. If you want to specifically target features in 3.0 then you need to multi target.