Skip to content

Conversation

filzrev
Copy link

@filzrev filzrev commented Aug 21, 2025

This PR modify existing GetOutputDevice/ GetClientInfo extension method APIs to use build-in GetRequiredService instead of GetRequiredServiceInternal ,

Background
I want to wrap following two providers to single IServiceProvider.

  1. MTP's IServiceProvider
  2. Custom IServiceProvider that created by Microsoft.Extensions.Dependency.

But currently GetOutputDevice/ GetClientInfo extension method assume IServiceProvider is ServiceProvider instance.
And throw exception when using these API via wrapped IServiceProvider.

As far as I've confirmed.
GetRequiredService/GetRequiredServiceInternal APIs difference is skipInternalOnlyExtensions flag only.
And it's not affect result when resolving GetOutputDevice/ GetClientInfo.

Copy link
Member

@Youssef1313 Youssef1313 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's unclear to me what the benefit is.

GetOutputDevice should only ever be called on MTP's service provider. It seems to me you might be calling the extension method on a different service provider implementation (the one that combines MTP and Microsoft.Extensions.Dependency). But I don't think mixing stuff that way is a good idea.

@filzrev
Copy link
Author

filzrev commented Sep 1, 2025

I don't think mixing stuff that way is a good idea.

I agree that it's not a good design to mix IServiceProviders.

But these extension methods are exposed as public.
So, I though it should not impose restriction unless it's not strictly required.
(Existing other public methods are using GetRequiresServide except for IOutputDevice/IClientInfo)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants