@BrennanConroy please measure signalr impact

@BrennanConroy please measure signalr impact

Microsoft.AspNetCore.SignalR.Client ->
Microsoft.AspNetCore.SignalR.Client.Core ->
Microsoft.AspNetCore.SignalR.Common ->
Microsoft.AspNetCore.Connections.Abstractions ->
Microsoft.Extensions.Features

What other impact are you interested in?

New client on old asp.net core apps. Old client in new apps.

I expect both will work fine. If Microsoft.Extensions.Features is loaded, the type forward takes precedence over anything else. Otherwise, the client will continue to use the old assembly. Worth checking though. Edit: Not so sure of this anymore. Definitely worth checking 😄

SignalR will reference this new project. This is a breaking change for SignalR clients but it's ok since this is already broken

Why is changing dependencies breaking?

Are we keeping Http.Features as a package targeting netstandard?

Are we keeping Http.Features as a package targeting netstandard?

No, that's the main reason for this change, and the reason it's breaking.

No, that's the main reason for this change, and the reason it's breaking.

Lets consider this then:

App (.NET 6) -> Package1 -> Http.Features 3.1 (for IFeatureCollection)
App (.NET 6) -> Package2 -> Extensions.Features 6.0 (for IFeatureCollection)

Consider App is a console app and then App as an ASP.NET Core app.

Do you get a source time type conflict?

Keep in mind that Http.Features and Extensions.Features are also both in the shared framework and should unify to the latest version (needs verification). A standalone publish should also be able to resolve the conflict (needs verification). To really get into trouble I expect you'd need to not be using the shared framework. I'll do some experiments.

I just want to explore the breaks so we can document them and anticipate what and how packages will snap

Create new project Microsoft.Extensions.Features (netstandard2.0)
Move IFeatureCollection, FeatureCollection etc to this assembly. Add type-forwarding from Http.Features.
...
This allows us to use new language features like DIMs in Http.Features without cross compiling.

Is a layering issue for DIMs if want to do the same thing with the FeatureCollection itself to get the same benefits as Headers?

So would maybe need to introduce a derived type IHttpFeatureCollection

IHttpFeatureCollection : IFeatureCollection
{
    IRouteValuesFeature RouteValuesFeature => Get<IRouteValuesFeature>();
    // ...
}

Which would then go viral and IFeatureCollection couldn't be used for the Http side as it wouldn't be the derived type etc

As I assume you wouldn't want to add Http DIMs to a more general non-Http IFeatures collection in Extensions?

As I assume you wouldn't want to add Http DIMs to a more general non-Http IFeatures collection in Extensions?

That's correct.

As I assume you wouldn't want to add Http DIMs to a more general non-Http IFeatures collection in Extensions?

That's correct.

But an issue is if a IHttpFeaturesCollection then wouldn't be interopative with a IFeaturesCollection to get the benefits of DIMs without adding an is check; which would diminish the benefits of DIMs?

HttpFeatureCollection : IHttpFeatureCollection
{
    private IFeatureCollection _collection;

    public HttpFeatureCollection(IFeatureCollection collection)
    {
        _collection = collection;
    }

    IRouteValuesFeature RouteValuesFeature => 
        _collection is IHttpFeatureCollection httpCollection ? 
            httpCollection.RouteValuesFeature :  
            _collection.Get<IRouteValuesFeature>();

    // ...
}

Or do it in contructor and then null check

HttpFeatureCollection : IHttpFeatureCollection
{
    private IHttpFeatureCollection _httpCollection;
    private IFeatureCollection _collection;

    public HttpFeatureCollection(IFeatureCollection collection)
    {
        if (collection is IHttpFeatureCollection _httpCollection)
        {
            _httpCollection = collection;
        }
        else
        {
            _collection = collection;
        }
    }

    IRouteValuesFeature RouteValuesFeature => 
        _httpCollection is not null ? 
            _httpCollection.RouteValuesFeature :  
            _collection.Get<IRouteValuesFeature>();

    // ...
}

Ofc its more complicated as there are othertypes of IFeatureCollection in use; like the TransportConnection is one but it isn't part of the Http kinda stuff but is available via the Http collection, so if also wanted Transport DIMs to be usable to the Http then they would need to go on top

ITransportFeatureCollection : IFeatureCollection
{
    IConnectionLifetimeFeature ConnectionLifetimeFeature=> Get<IConnectionLifetimeFeature>();
    // ...
}

IHttpFeatureCollection : ITransportFeatureCollection
{
    IRouteValuesFeature RouteValuesFeature => Get<IRouteValuesFeature>();
    // ...
}

Then accept that Http was always over a connection (it probably is); but then that's quite a formal api heirachy using DIMs vs it being a bit more fluid currently (although is a guess if the featureCollection will resolve the type as its not guaranteeing types via contract; whereas the DIMs would be strongly suggesting it)

Just wondering if there is an opperturnity for DIMs over the IFeatureCollection itself; how that would work and what type DefaultHttpContext would be taking in is contructor (where it resolves IHttpRequestFeature, IHttpResponseFeature etc from); and then what would Kestrel's HttpProtocol be implementing?

Or would it not work?

Just wondering if there is an opperturnity for DIMs over the IFeatureCollection itself; how that would work and what type DefaultHttpContext would be taking in is contructor (where it resolves IHttpRequestFeature, IHttpResponseFeature etc from); and then what would Kestrel's HttpProtocol be implementing?

I don't think it would work with the layering we want. At least, not with the way that we want to push feature collection down into the stack. It feels bad to attempt to use the same trick here. We would need to replat everything on top of a more specific one and do the type check.

Lets consider this then:

App (.NET 6) -> Package1 -> Http.Features 3.1 (for IFeatureCollection)
App (.NET 6) -> Package2 -> Extensions.Features 6.0 (for IFeatureCollection)

Consider App is a console app and then App as an ASP.NET Core app.

Do you get a source time type conflict?

I was able to contrive a setup like this to produce the error.

Console App net6.0

• Project reference to library1 net5.0
  • Package reference to Microsoft.AspNetCore.Http.Features 5.0.0
• Project reference to library2 net6.0
  • Project reference (in repo) to Microsoft.Extensions.Features

Error	CS0433	The type 'FeatureCollection' exists in both 'Microsoft.AspNetCore.Http.Features, Version=5.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60' and 'Microsoft.Extensions.Features, Version=6.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60'

Interestingly I only get the error at compile time when I directly reference IFeatureCollection in the App Program.cs. If it's only referenced by type in the libraries everything compiles and runs fine with it treated as two distinct types. Then you only get compilation failures if you try to pass the IFeatureCollection from one library to the other.

Workaround: In the base App add a direct reference to Microsoft.AspNetCore.Http.Features 6.0 to lift the dependency and include the type forward. Note this works with in-repo references, we'll want to confirm it after we have a full build and real packages.

I get the same results testing a web app in-repo, but I think that's because it's not using the shared framework. I'll need to re-test after we have a full build.

I don't see anything surprising or blocking here. I'll plan to re-test after it's merged and we have a build.

@davidfowl is that enough coverage for us to proceed?

Workaround: In the base App add a direct reference to Microsoft.AspNetCore.Http.Features 6.0 to lift the dependency and include the type forward. Note this works with in-repo references, we'll want to confirm it after we have a full build and real packages.

This is why I wanted to ask if we should ship a package with type forwards for 6.0 as a way to bridge the gap until 7.

Draft Announcement:

Microsoft.AspNetCore.Http.Features split, no longer shipped as a package

Microsoft.AspNetCore.Http.Features is being split into two assemblies, Microsoft.AspNetCore.Http.Features and Microsoft.Extensions.Features.

Version introduced

6.0

Old behavior

Microsoft.AspNetCore.Http.Features 5.0 shipped both in the ASP.NET shared framework and as a nuget package.

Microsoft.AspNetCore.Http.Features 5.0 targeted .NET 4.6.1, .NET Standard 2.0, and .NET 5.0

New behavior

Microsoft.AspNetCore.Http.Features 6.0 will ship only in the ASP.NET shared framework, not as a nuget package.

Microsoft.AspNetCore.Http.Features 6.0 will target .NET 6.0 only.

Microsoft.Extensions.Features 6.0 will ship both in the ASP.NET shared framework and as a nuget package.

Microsoft.Extensions.Features 6.0 will target .NET 4.6.1, .NET Standard 2.0, and .NET 6.0

The following types will move to the new assembly. The namespaces will not be changed and type forwards will be added for compatibility.

namespace Microsoft.AspNetCore.Http.Features
IFeatureCollection
FeatureCollection
FeatureReference<T>
FeatureReferences<T>

Reason for change

This allows these core types to be shared more broadly across components, while also allowing the remaining Http specific components in Microsoft.AspNetCore.Http.Features to take advantage of new runtime and language features.

Recommended action

When upgrading to ASP.NET 6.0 remove any packages references for Microsoft.AspNetCore.Http.Features. Add a package reference for Microsoft.Extensions.Features only if required.

Class libraries that need to consume the types from Microsoft.AspNetCore.Http.Features should add a FrameworkReference instead:

  <ItemGroup>
    <FrameworkReference Include="Microsoft.AspNetCore.App" />
  </ItemGroup>

https://docs.microsoft.com/aspnet/core/fundamentals/target-aspnetcore?#use-the-aspnet-core-shared-framework

Libraries with out of date references may encounter a TypeLoadException or the following error:

Error	CS0433	The type 'IFeatureCollection' exists in both 'Microsoft.AspNetCore.Http.Features, Version=5.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60' and 'Microsoft.Extensions.Features, Version=6.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60'

This can be mitigated by adding the FrameworkReference shown above to any of the affected projects.

Category

ASP.NET

Affected APIs

IFeatureCollection
FeatureCollection
FeatureReference
FeatureReferences

Issue metadata

• Issue type: breaking-change

This is why I wanted to ask if we should ship a package with type forwards for 6.0 as a way to bridge the gap until 7.

I had to add a TypeForwards in the Headers (as it is currently) 9bc7c11 with a #pragma warning disable RS0016 as the shipped api thing decides its in two places and complains

Workaround: In the base App add a direct reference to Microsoft.AspNetCore.Http.Features 6.0 to lift the dependency and include the type forward. Note this works with in-repo references, we'll want to confirm it after we have a full build and real packages.

This is why I wanted to ask if we should ship a package with type forwards for 6.0 as a way to bridge the gap until 7.

Fair point. I wonder if the FrameworkReference will work just as well. I won't be able to test that until there's a real build.

I had to add a TypeForwards in the Headers (as it is currently) 9bc7c11 with a #pragma warning disable RS0016 as the shipped api thing decides its in two places and complains

@benaadams after this PR you should be able to reverse that dependency you added and avoid moving any types. I'll follow up with you on that PR.

Fair point. I wonder if the FrameworkReference will work just as well. I won't be able to test that until there's a real build.

The external Grpc tests were failing for me as they ref'd the previous framework (5.0) and the auto-update to 6.0 broke because the type moved, so a type forwards is probably recommended

For the announcement, technically it's every API in the current package, not just the few.

For the announcement, technically it's every API in the current package, not just the few.

@pranavkm true, but the template calls for specific links. Any suggestions for an abbreviated format to cover the others? The docs don't list APIs by assembly, only namespace, and it's not a one-to-one match.

Yeah, I'm not super sure there's a good way to present it. @JamesNK any suggestions?

Sorry, I'm not sure what the question for me is.

Fair point. I wonder if the FrameworkReference will work just as well. I won't be able to test that until there's a real build.

The external Grpc tests were failing for me as they ref'd the previous framework (5.0) and the auto-update to 6.0 broke because the type moved, so a type forwards is probably recommended

So with the change in this PR, the package Grpc.AspNetCore will break when used with .NET 6? If so that definitely needs to be improved because the package isn't doing anything special or hacky. If Grpc.AspNetCore is broken then tons of packages will be broken. This break would be too big.

So with the change in this PR, the package Grpc.AspNetCore will break when used with .NET 6? If so that definitely needs to be improved because the package isn't doing anything special or hacky. If Grpc.AspNetCore is broken then tons of packages will be broken. This break would be too big.

Shouldn't do; the TypeFowarders are there, was just pointing out they were necessary

Sorry, I'm not sure what the question for me is.

Think its about the announcement? #32043 (comment)

For the announcement, technically it's every API in the current package, not just the few.

@pranavkm true, but the template calls for specific links. Any suggestions for an abbreviated format to cover the others? The docs don't list APIs by assembly, only namespace, and it's not a one-to-one match.

@Tratcher that looks good. I think we should also mention the types of errors that can show up:

Error	CS0433	The type 'FeatureCollection' exists in both 'Microsoft.AspNetCore.Http.Features, Version=5.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60' and 'Microsoft.Extensions.Features, Version=6.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60'

And likely a TypeLoadException runtime error because the type moved and you're missing the reference?

And likely a TypeLoadException runtime error because the type moved and you're missing the reference?

Updated. Actually getting a TypeLoadException would be tough, I wasn't able to trigger one in testing. If you had a nuget dependency you would at least have the old type. You'd have to manually copy your old library dll to a project that only referenced the new Extensions.Features and not the shared framework.

Reviewers: This should be ready. We think the Code Check issue is a false positive that @JunTaoLuo will review and override as needed.

I imagine testing just how breaking this is will be easier after we merge it. We can adjust/revert later.

Right, we just need to document all the things.

the Code Check issue is a false positive

Not exactly a "false positive" but a case where the error should be overridden.

Now that I think about it, is / are the relevant PublicAPI.Shipped.txt file or files moving along with the existing assembly name❔ Should still have files containing exactly what we shipped in 5.0 even if the project moved. That is, use *REMOVED* if necessary rather than letting VS edit the shipped files.

/azp run

Now that I think about it, is / are the relevant PublicAPI.Shipped.txt file or files moving along with the existing assembly name❔ Should still have files containing exactly what we shipped in 5.0 even if the project moved. That is, use *REMOVED* if necessary rather than letting VS edit the shipped files.

These files:
error : src/Http/Http.Features/src/PublicAPI/net6.0/PublicAPI.Shipped.txt
error : src/Http/Http.Features/src/PublicAPI/netstandard2.0/PublicAPI.Shipped.txt

Were recently added in 6.0 by #31625. We're removing them now that we've found a better solution.

The 5.0 APIs are still covered by src/PublicAPI.Shipped.txt

@JunTaoLuo @dougbu this is ready to merge.