Skip to content
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

[API Proposal]: Bring back extensions API to ease adoption of extensions libraries from older frameworks #87480

Closed
ericstj opened this issue Jun 13, 2023 · 3 comments · Fixed by #85846
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-Extensions-Primitives blocking Marks issues that we want to fast track in order to unblock other important work
Milestone

Comments

@ericstj
Copy link
Member

ericstj commented Jun 13, 2023

Background and motivation

Over time types have been removed from the Microsoft.Extensions.* libraries that have made it difficult for customers to use newer versions of those packages on older frameworks.

All of these APIs were obsoleted in a predictable way - first marking as obsolete for one release, then removal in a following release - however that removal has proven to be more breaking for the ecosystem in some cases.

The largest problem that has come up a lot is the removal of types that were used by ASP.NET Core in 2.x. Many of the libraries from this release are still the latest in use by .NETFramework customers - since that's the last version of the product that supported .NETFramework.

Bringing back these APIs has a small cost to maintenance, but will unblock folks from taking newer extensions packages and will allow other packages built on top of these extensions to safely take on the latest version without regressing people. It will also simplify our story for the Microsoft.Extesions packages in terms of best practice reccomendations. We can tell folks it's safe to reference the latest version that is supported and has the API they wish to use (same as we recommend for System.Text.Json) and not require folks to specialize and cross-target for each TFM to select a different version.

We've chosen to bring back API as

[EditorBrowsable(EditorBrowsableState.Never)]
[Obsolete("This property is retained only for compatibility. ...", error: true)]

To discourage use. This will fix binary compatibility, reduce discoverability, source compatibility is still broken by default but has the workaround of annotating consuming code with obsolete.

API Proposal

See https://github.com/dotnet/runtime/pull/85846/files?file-filters%5B%5D=.cs&show-viewed-files=true for added API

API added back is not subject to review since it must be identical to what was previously exposed.

I'm seeking approval / acknowledgement for adding back this API for the sake of binary compatibility.

We looked at all the API breaking changes between 2.1 and today and choose to add back API based on the following:

  1. Entry point API - extension methods, public properties on types we expect users to interact with.
  2. High usage

API was not brought back based on the following:

  1. Bringing back would break a runtime scenario like DI
  2. Pub-ternals which we never expect users to interact with
  3. Concrete types where we expect users to interact with interfaces

We double checked any case where we were excluding API and it's usage showed significant numbers to understand if there was a significant use case we were missing. One case of this was NullScope where we found all the usage was from tool-like packages redistributing the Microsoft.Extensions binaries themselves as an app layout - and is this not subject to break through package upgrade.

Usage analysis: xls md

API Usage

N/A

Alternative Designs

Only do this in implementation assemblies and not reference assemblies. This isn't helpful since we don't ship the reference assemblies in nuget packages.

Risks

We may not have brought back enough API - we can always bring back more based on customer feedback. We have tested some scenarios (like 2.1 ASP.NET Core App on NetFx with the latest packages - fixed).

Normal set of risk with adding API - name collisions, reflection ambiguity, etc. We've tried to anticipate some of this - like the DI consumption of constructors and reduced the API exposed to mitigate risk.

@ericstj ericstj added blocking Marks issues that we want to fast track in order to unblock other important work area-Extensions-Primitives api-ready-for-review API is ready for review, it is NOT ready for implementation labels Jun 13, 2023
@ericstj ericstj added this to the 8.0.0 milestone Jun 13, 2023
@ericstj ericstj self-assigned this Jun 13, 2023
@ghost
Copy link

ghost commented Jun 13, 2023

Tagging subscribers to this area: @dotnet/area-extensions-primitives
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

Over time types have been removed from the Microsoft.Extensions.* libraries that have made it difficult for customers to use newer versions of those packages on older frameworks.

All of these APIs were obsoleted in a predictable way - first marking as obsolete for one release, then removal in a following release - however that removal has proven to be more breaking for the ecosystem in some cases.

The largest problem that has come up a lot is the removal of types that were used by ASP.NET Core in 2.x. Many of the libraries from this release are still the latest in use by .NETFramework customers - since that's the last version of the product that supported .NETFramework.

Bringing back these APIs has a small cost to maintenance, but will unblock folks from taking newer extensions packages and will allow other packages built on top of these extensions to safely take on the latest version without regressing people. It will also simplify our story for the Microsoft.Extesions packages in terms of best practice reccomendations. We can tell folks it's safe to reference the latest version that is supported and has the API they wish to use (same as we recommend for System.Text.Json) and not require folks to specialize and cross-target for each TFM to select a different version.

We've chosen to bring back API as

[EditorBrowsable(EditorBrowsableState.Never)]
[Obsolete("This property is retained only for compatibility. ...", error: true)]

To discourage use. This will fix binary compatibility, reduce discoverability, source compatibility is still broken by default but has the workaround of annotating consuming code with obsolete.

API Proposal

See https://github.com/dotnet/runtime/pull/85846/files?file-filters%5B%5D=.cs&show-viewed-files=true for added API

API added back is not subject to review since it must be identical to what was previously exposed.

I'm seeking approval / acknowledgement for adding back this API for the sake of binary compatibility.

We looked at all the API breaking changes between 2.1 and today and choose to add back API based on the following:

  1. Entry point API - extension methods, public properties on types we expect users to interact with.
  2. High usage

API was not brought back based on the following:

  1. Bringing back would break a runtime scenario like DI
  2. Pub-ternals which we never expect users to interact with
  3. Concrete types where we expect users to interact with interfaces

We double checked any case where we were excluding API and it's usage showed significant numbers to understand if there was a significant use case we were missing. One case of this was NullScope where we found all the usage was from tool-like packages redistributing the Microsoft.Extensions binaries themselves as an app layout - and is this not subject to break through package upgrade.

Usage analysis: xls md

API Usage

N/A

Alternative Designs

No response

Risks

No response

Author: ericstj
Assignees: ericstj
Labels:

blocking, area-Extensions-Primitives, api-ready-for-review

Milestone: 8.0.0

@terrajobst
Copy link
Member

terrajobst commented Jun 13, 2023

Video

This plan makes sense as proposed. I don't think it makes sense for us to walk the entire list of APIs so we'll just approve this plan and the stated principles.

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Jun 13, 2023
@ericstj
Copy link
Member Author

ericstj commented Jun 14, 2023

I updated the XLS link above to point to a file that's at a more permanent internal location. The MD link is the same content available publicly but it is harder to view.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 14, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jun 16, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jul 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-Extensions-Primitives blocking Marks issues that we want to fast track in order to unblock other important work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants