Skip to content

Conversation

@TheConstructor
Copy link
Contributor

Currently e.g.

public interface IServiceTag
{
}

[RegisterSingleton(Tags = "Client,FrontEnd")]
public class ServiceTag : IServiceTag
{
}

yields two independent registrations, so that provider.GetService<IServiceTag>() and provider.GetService<ServiceTag>() return two independent instances, that are individually tracked and disposed. Introducing a 'factory' that resolves ServiceTag (implementation-type), if it is registered, and non-open-generic, to resolve IServiceTag solves this. Now only a single instance is created.

Generated code for the registration above with this PR is

            if (tagSet.Count == 0 || tagSet.Intersect(new[] { "Client", "FrontEnd" }).Any())
            {
                global::Microsoft.Extensions.DependencyInjection.Extensions.ServiceCollectionDescriptorExtensions.TryAdd(
                    serviceCollection,
                    global::Microsoft.Extensions.DependencyInjection.ServiceDescriptor.Singleton<global::Injectio.Tests.Library.IServiceTag>((serviceProvider) => global::Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService<global::Injectio.Tests.Library.ServiceTag>(serviceProvider))
                );

                global::Microsoft.Extensions.DependencyInjection.Extensions.ServiceCollectionDescriptorExtensions.TryAdd(
                    serviceCollection,
                    global::Microsoft.Extensions.DependencyInjection.ServiceDescriptor.Singleton<global::Injectio.Tests.Library.ServiceTag, global::Injectio.Tests.Library.ServiceTag>()
                );

            }

@coveralls
Copy link

coveralls commented Apr 22, 2025

Pull Request Test Coverage Report for Build 14622968813

Details

  • 29 of 35 (82.86%) changed or added relevant lines in 2 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.3%) to 83.27%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/Injectio.Generators/ServiceRegistrationGenerator.cs 14 20 70.0%
Files with Coverage Reduction New Missed Lines %
src/Injectio.Generators/ServiceRegistrationGenerator.cs 1 83.81%
Totals Coverage Status
Change from base Build 14602062882: 0.3%
Covered Lines: 547
Relevant Lines: 627

💛 - Coveralls

@Cologler
Copy link

This was previously discussed in issue #138

@TheConstructor
Copy link
Contributor Author

@Cologler while the result is similar, I would argue, that you haven't really stated why this was desirable. I think people expect that one attribute yields one instance in their chosen lifetime, and so I created this PR. I am totally aware it isn't perfect, and may need changes or be rejected completely

@TheConstructor
Copy link
Contributor Author

Just realized, that I probably don't need to supress the second generic argument when registering a factory. Guess I will push a new version later.

@pwelter34
Copy link
Member

I've been considering this idea as well. Its especially important for singleton registrations. I like the idea that its an option, though I'm concerned about switch the default for backward compatibility. Then again, we are working toward a new version 5 that it could be acceptable.

@pwelter34
Copy link
Member

This enhancement could address the default behavior issue.

#217

@TheConstructor TheConstructor force-pushed the feature/proxy-factory branch from f48ab0f to a93513d Compare April 23, 2025 16:13
@hdocsek
Copy link

hdocsek commented Apr 24, 2025

I need to support initialization of multiple components which have this interface:

interface IInitialize
{
    void Initialize();
}

I tried to use
[RegisterSingleton(Duplicate = DuplicateStrategy.Append, Registration = RegistrationStrategy.SelfWithInterfaces)]
and expected that only one instance exists for both class and all of its interfaces.
However, this didn't work because it produced individual registrations.

I tried it it out with RegistrationStrategy.SelfWithProxyFactory from this PR instead and it worked like expected.

I am looking forward to getting this merged.

Special thanks to @TheConstructor

@pwelter34 pwelter34 merged commit 60191a8 into loresoft:main Apr 25, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants