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

Consider removing package references for inbox assemblies #5046

Closed
kevinchalet opened this issue Mar 14, 2024 · 3 comments · Fixed by #5058
Closed

Consider removing package references for inbox assemblies #5046

kevinchalet opened this issue Mar 14, 2024 · 3 comments · Fixed by #5058
Assignees
Labels
enhancement This issue represents an ask for new feature or an enhancement to an existing one

Comments

@kevinchalet
Copy link

Hey there 👋🏻

As part of OpenIddict 5.3.0, I introduced built-in support for Microsoft.Extensions.Http.Resilience (on .NET 8.0+) and it's working beautifully (thanks for making ResilienceHandler public BTW, it wouldn't have been possible otherwise!).

One of the rules I enforce in OpenIddict is to always optimize the dependencies graph as much as possible and avoid referencing (directly or transitively) packages for which inbox assemblies are already offered in .NET. Unfortunately, it seems a few projects maintained here reference NuGet packages for which an inbox version in .NET 8.0 exists (for instance, System.Text.Json).

Here's the dependencies I get for OpenIddict.Validation.SystemNetHttp (FWIW, with 6M+ downloads, it's currently the most downloaded NuGet.org package that depends on the new HTTP resilience stack):

On .NET 7.0, that doesn't reference Microsoft.Extensions.Http.Resilience:

   [net7.0]: 
   Package de niveau supérieur               Demandé                  Résolu            
   > Microsoft.DotNet.XliffTasks       (A)   [1.0.0-beta.23475.1, )   1.0.0-beta.23475.1
   > Microsoft.Extensions.Http.Polly         2.1.1                    7.0.15            
   > PolySharp                               1.13.1                   1.13.1            

   Package transitif                                            Résolu
   > Microsoft.Extensions.DependencyInjection                   7.0.0 
   > Microsoft.Extensions.DependencyInjection.Abstractions      7.0.0 
   > Microsoft.Extensions.Http                                  7.0.0 
   > Microsoft.Extensions.Logging                               7.0.0 
   > Microsoft.Extensions.Logging.Abstractions                  7.0.0 
   > Microsoft.Extensions.Options                               7.0.0 
   > Microsoft.Extensions.Primitives                            7.0.0 
   > Microsoft.IdentityModel.Abstractions                       7.4.0 
   > Microsoft.IdentityModel.JsonWebTokens                      7.4.0 
   > Microsoft.IdentityModel.Logging                            7.4.0 
   > Microsoft.IdentityModel.Protocols                          7.4.0 
   > Microsoft.IdentityModel.Tokens                             7.4.0 
   > Polly                                                      7.2.3 
   > Polly.Extensions.Http                                      3.0.0 

On .NET 8.0, that references Microsoft.Extensions.Http.Resilience:

   [net8.0]: 
   Package de niveau supérieur                    Demandé                  Résolu            
   > Microsoft.DotNet.XliffTasks            (A)   [1.0.0-beta.23475.1, )   1.0.0-beta.23475.1
   > Microsoft.Extensions.Http.Polly              2.1.1                    8.0.1             
   > Microsoft.Extensions.Http.Resilience         8.2.0                    8.2.0             
   > PolySharp                                    1.13.1                   1.13.1            

   Package transitif                                              Résolu
   > Microsoft.Bcl.TimeProvider                                   8.0.1 
   > Microsoft.Extensions.AmbientMetadata.Application             8.2.0 
   > Microsoft.Extensions.Compliance.Abstractions                 8.2.0 
   > Microsoft.Extensions.Configuration                           8.0.0 
   > Microsoft.Extensions.Configuration.Abstractions              8.0.0 
   > Microsoft.Extensions.Configuration.Binder                    8.0.1 
   > Microsoft.Extensions.DependencyInjection                     8.0.0 
   > Microsoft.Extensions.DependencyInjection.Abstractions        8.0.0 
   > Microsoft.Extensions.DependencyInjection.AutoActivation      8.2.0 
   > Microsoft.Extensions.DiagnosticAdapter                       3.1.32
   > Microsoft.Extensions.Diagnostics                             8.0.0 
   > Microsoft.Extensions.Diagnostics.Abstractions                8.0.0 
   > Microsoft.Extensions.Diagnostics.ExceptionSummarization      8.2.0 
   > Microsoft.Extensions.FileProviders.Abstractions              8.0.0 
   > Microsoft.Extensions.Hosting.Abstractions                    8.0.0 
   > Microsoft.Extensions.Http                                    8.0.0 
   > Microsoft.Extensions.Http.Diagnostics                        8.2.0 
   > Microsoft.Extensions.Logging                                 8.0.0 
   > Microsoft.Extensions.Logging.Abstractions                    8.0.0 
   > Microsoft.Extensions.Logging.Configuration                   8.0.0 
   > Microsoft.Extensions.ObjectPool                              8.0.2 
   > Microsoft.Extensions.Options                                 8.0.2 
   > Microsoft.Extensions.Options.ConfigurationExtensions         8.0.0 
   > Microsoft.Extensions.Primitives                              8.0.0 
   > Microsoft.Extensions.Resilience                              8.2.0 
   > Microsoft.Extensions.Telemetry                               8.2.0 
   > Microsoft.Extensions.Telemetry.Abstractions                  8.2.0 
   > Microsoft.IdentityModel.Abstractions                         7.4.0 
   > Microsoft.IdentityModel.JsonWebTokens                        7.4.0 
   > Microsoft.IdentityModel.Logging                              7.4.0 
   > Microsoft.IdentityModel.Protocols                            7.4.0 
   > Microsoft.IdentityModel.Tokens                               7.4.0 
   > Microsoft.IO.RecyclableMemoryStream                          2.3.2 
   > Polly                                                        7.2.4 
   > Polly.Core                                                   8.3.0 
   > Polly.Extensions                                             8.3.0 
   > Polly.Extensions.Http                                        3.0.0 
   > Polly.RateLimiting                                           8.3.0 
   > System.Diagnostics.DiagnosticSource                          8.0.0 
   > System.Text.Encodings.Web                                    8.0.0 
   > System.Text.Json                                             8.0.2 
   > System.Threading.RateLimiting                                8.0.0 

Some of them - like System.Threading.RateLimiting, not provided OOTB - are expected, but I think (at least) the following ones are not necessary and could be removed on .NET 8.0+:

  • Microsoft.Bcl.TimeProvider
  • System.Diagnostics.DiagnosticSource
  • System.Text.Encodings.Web
  • System.Text.Json

That would be nice if you could improve that in the next minor version 😃

Best regards.

@geeknoid geeknoid added enhancement This issue represents an ask for new feature or an enhancement to an existing one and removed untriaged labels Mar 16, 2024
@geeknoid geeknoid self-assigned this Mar 16, 2024
@lahma
Copy link

lahma commented Mar 19, 2024

Came here to create an issue about this problem and who was again one step ahead... Also wishing for this improvement in next minor if possible.

@kevinchalet
Copy link
Author

Haha, nice to see you here, @lahma! ❤️

Note: I mentioned .NET 8.0+, but the same logic should also be applied to .NET 6.0 as it seems most of the 8.0 packages here target both LTS versions.

@geeknoid
Copy link
Member

I'll get this done.

geeknoid pushed a commit that referenced this issue Mar 19, 2024
- Enable the ReferenceTrimmer analyzer to make sure
the dependencies stay clean.

Fixes #5046
geeknoid added a commit that referenced this issue Mar 20, 2024
* Trim dependencies

- Enable the ReferenceTrimmer analyzer to make sure
the dependencies stay clean.

Fixes #5046

---------

Co-authored-by: Martin Taillefer <[email protected]>
@github-actions github-actions bot locked and limited conversation to collaborators Apr 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement This issue represents an ask for new feature or an enhancement to an existing one
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants