Make TService type param more permissive on Add*<TService, TImplementation>()

[Mardoxx]

Fixes dotnet/aspnetcore#2828

[Mardoxx]

Will add tests after feedback (if this is a genuine issue)

[davidfowl]

Is this a breaking change?

[Mardoxx]

Not entirely sure. Superficially I don't think so!

I wouldn't have thought -- seeing as this is making it more permissive on the constraints -- but there may be some way you could write something where it would break something that worked previously using reflection/inline IL??

[Mardoxx]

Okay, it's been picked up as a breaking change - but is it really?

This is a strange one as it clearly works when not passing template params.

What's different between

this invocation

    public static IServiceCollection RegisterScopedTestService<TService, TImplementation>(this IServiceCollection services)
        where TImplementation : class, TService
    {
        return services.AddScoped<TService, TImplementation>();
    }

and this one

    public void SomeFunction(IServiceCollection services)
    {
        services.AddScoped<ITestService, TestService>();
    }

given

    public interface ITestService
    {
    }

    public class TestService : ITestService
    {
    }

The signature still expects TService to be a reference type, but the latter is working. What am I missing here?

[davidfowl]

You just need to add where TService : class so that you match the constraints of the method you're calling.

[Mardoxx]

Just realised that! 😄

Now I'm wondering if the constraint is actually required. Under what circumstances could you register something which fails these constraints

TService : class
TImplementation : TService, class

but passes with these

//TService : any
TImplementation : TService, class

[davidfowl]

Now I'm wondering if the constraint is actually required. Under what circumstances could you register something which fails these constraints

It may not be possible but I'm not sure it matters because we wouldn't make a breaking change like this regardless.

[Mardoxx]

interesting. I'll post on SO, see if anyone can out of curiosity.

Very understandable! Thanks 😄

[Mardoxx]

So after a little reading it would seem that the current definition is more technically correct, even if it's not necessary. I suspect the choice was taken to have both constraints there as it clearly shows that TService must be a reference type!

[Mardoxx]

https://stackoverflow.com/a/48471755/3515174 Brilliant insight here 😄