diff --git a/src/Components/Components/ref/Microsoft.AspNetCore.Components.netcoreapp.cs b/src/Components/Components/ref/Microsoft.AspNetCore.Components.netcoreapp.cs index 78334efc06..c2c58118bd 100644 --- a/src/Components/Components/ref/Microsoft.AspNetCore.Components.netcoreapp.cs +++ b/src/Components/Components/ref/Microsoft.AspNetCore.Components.netcoreapp.cs @@ -107,6 +107,11 @@ namespace Microsoft.AspNetCore.Components protected virtual bool ShouldRender() { throw null; } protected void StateHasChanged() { } } + public partial class DefaultComponentActivator : Microsoft.AspNetCore.Components.IComponentActivator + { + public DefaultComponentActivator() { } + public Microsoft.AspNetCore.Components.IComponent CreateInstance(System.Type componentType) { throw null; } + } public abstract partial class Dispatcher { protected Dispatcher() { } @@ -234,6 +239,10 @@ namespace Microsoft.AspNetCore.Components void Attach(Microsoft.AspNetCore.Components.RenderHandle renderHandle); System.Threading.Tasks.Task SetParametersAsync(Microsoft.AspNetCore.Components.ParameterView parameters); } + public partial interface IComponentActivator + { + Microsoft.AspNetCore.Components.IComponent CreateInstance(System.Type componentType); + } public partial interface IHandleAfterRender { System.Threading.Tasks.Task OnAfterRenderAsync(); diff --git a/src/Components/Components/src/ComponentFactory.cs b/src/Components/Components/src/ComponentFactory.cs index 3ba141834b..eddb39a937 100644 --- a/src/Components/Components/src/ComponentFactory.cs +++ b/src/Components/Components/src/ComponentFactory.cs @@ -6,14 +6,9 @@ using System.Collections.Concurrent; using System.Linq; using System.Reflection; using Microsoft.AspNetCore.Components.Reflection; -using Microsoft.Extensions.DependencyInjection; namespace Microsoft.AspNetCore.Components { - /// - /// The property on this type is used as a static global cache. Ensure any changes to this type - /// are thread safe and can be safely cached statically. - /// internal class ComponentFactory { private static readonly BindingFlags _injectablePropertyBindingFlags @@ -22,19 +17,20 @@ namespace Microsoft.AspNetCore.Components private readonly ConcurrentDictionary> _cachedInitializers = new ConcurrentDictionary>(); - public static readonly ComponentFactory Instance = new ComponentFactory(); + private readonly IComponentActivator _componentActivator; + + public ComponentFactory(IComponentActivator componentActivator) + { + _componentActivator = componentActivator ?? throw new ArgumentNullException(nameof(componentActivator)); + } public IComponent InstantiateComponent(IServiceProvider serviceProvider, Type componentType) { - var activator = serviceProvider.GetService(); - - var instance = activator != null - ? activator.CreateInstance(componentType) - : Activator.CreateInstance(componentType); - - if (!(instance is IComponent component)) + var component = _componentActivator.CreateInstance(componentType); + if (component is null) { - throw new ArgumentException($"The type {componentType.FullName} does not implement {nameof(IComponent)}.", nameof(componentType)); + // The default activator will never do this, but an externally-supplied one might + throw new InvalidOperationException($"The component activator returned a null value for a component of type {componentType.FullName}."); } PerformPropertyInjection(serviceProvider, component); diff --git a/src/Components/Components/src/DefaultComponentActivator.cs b/src/Components/Components/src/DefaultComponentActivator.cs index 4f93188f2f..ac5f5b2fb7 100644 --- a/src/Components/Components/src/DefaultComponentActivator.cs +++ b/src/Components/Components/src/DefaultComponentActivator.cs @@ -6,14 +6,25 @@ using System; namespace Microsoft.AspNetCore.Components { /// - /// Default implementation of component activator. + /// Default implementation of . /// public class DefaultComponentActivator : IComponentActivator { + // If no IComponentActivator is supplied by DI, the renderer uses this instance. + // It's internal because in the future, we might want to add per-scope state and then + // it would no longer be applicable to have a shared instance. + internal static IComponentActivator Instance { get; } = new DefaultComponentActivator(); + /// - public IComponent? CreateInstance(Type componentType) + public IComponent CreateInstance(Type componentType) { - return Activator.CreateInstance(componentType) as IComponent; + var instance = Activator.CreateInstance(componentType); + if (!(instance is IComponent component)) + { + throw new ArgumentException($"The type {componentType.FullName} does not implement {nameof(IComponent)}.", nameof(componentType)); + } + + return component; } } } diff --git a/src/Components/Components/src/IComponentActivator.cs b/src/Components/Components/src/IComponentActivator.cs index e98e111a05..9f97c38e9f 100644 --- a/src/Components/Components/src/IComponentActivator.cs +++ b/src/Components/Components/src/IComponentActivator.cs @@ -7,14 +7,16 @@ namespace Microsoft.AspNetCore.Components { /// /// Represents an activator that can be used to instantiate components. + /// The activator is not responsible for dependency injection, since the framework + /// performs dependency injection to the resulting instances separately. /// public interface IComponentActivator { /// - /// Creates an component of the specified type using that type's default constructor. + /// Creates a component of the specified type. /// /// The type of component to create. /// A reference to the newly created component. - IComponent? CreateInstance(Type componentType); + IComponent CreateInstance(Type componentType); } } diff --git a/src/Components/Components/src/RenderTree/Renderer.cs b/src/Components/Components/src/RenderTree/Renderer.cs index bb8bddcdc2..ca67d48f9b 100644 --- a/src/Components/Components/src/RenderTree/Renderer.cs +++ b/src/Components/Components/src/RenderTree/Renderer.cs @@ -10,6 +10,7 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Components.Profiling; using Microsoft.AspNetCore.Components.Rendering; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.Components.RenderTree @@ -29,6 +30,7 @@ namespace Microsoft.AspNetCore.Components.RenderTree private readonly Dictionary _eventBindings = new Dictionary(); private readonly Dictionary _eventHandlerIdReplacements = new Dictionary(); private readonly ILogger _logger; + private readonly ComponentFactory _componentFactory; private int _nextComponentId = 0; // TODO: change to 'long' when Mono .NET->JS interop supports it private bool _isBatchInProgress; @@ -70,6 +72,10 @@ namespace Microsoft.AspNetCore.Components.RenderTree _serviceProvider = serviceProvider; _logger = loggerFactory.CreateLogger(); + + var componentActivator = serviceProvider.GetService() + ?? DefaultComponentActivator.Instance; + _componentFactory = new ComponentFactory(componentActivator); } /// @@ -89,7 +95,7 @@ namespace Microsoft.AspNetCore.Components.RenderTree /// The type of the component to instantiate. /// The component instance. protected IComponent InstantiateComponent(Type componentType) - => ComponentFactory.Instance.InstantiateComponent(_serviceProvider, componentType); + => _componentFactory.InstantiateComponent(_serviceProvider, componentType); /// /// Associates the with the , assigning diff --git a/src/Components/Components/test/ComponentFactoryTest.cs b/src/Components/Components/test/ComponentFactoryTest.cs index 662351bd6f..4fb0a34385 100644 --- a/src/Components/Components/test/ComponentFactoryTest.cs +++ b/src/Components/Components/test/ComponentFactoryTest.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Text; using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; using Xunit; @@ -17,7 +16,7 @@ namespace Microsoft.AspNetCore.Components { // Arrange var componentType = typeof(EmptyComponent); - var factory = new ComponentFactory(); + var factory = new ComponentFactory(new DefaultComponentActivator()); // Act var instance = factory.InstantiateComponent(GetServiceProvider(), componentType); @@ -28,44 +27,31 @@ namespace Microsoft.AspNetCore.Components } [Fact] - public void InstantiateComponent_CreatesInstance_WithActivator() + public void InstantiateComponent_CreatesInstance_NonComponent() + { + // Arrange + var componentType = typeof(List); + var factory = new ComponentFactory(new DefaultComponentActivator()); + + // Assert + var ex = Assert.Throws(() => factory.InstantiateComponent(GetServiceProvider(), componentType)); + Assert.StartsWith($"The type {componentType.FullName} does not implement {nameof(IComponent)}.", ex.Message); + } + + [Fact] + public void InstantiateComponent_CreatesInstance_WithCustomActivator() { // Arrange var componentType = typeof(EmptyComponent); - var factory = new ComponentFactory(); - - // Act - var instance = factory.InstantiateComponent(GetServiceProviderWithActivator(), componentType); - - // Assert - Assert.NotNull(instance); - Assert.IsType(instance); - } - - [Fact] - public void InstantiateComponent_CreatesInstance_WithActivator_NonComponent() - { - // Arrange - var componentType = typeof(NonComponent); - var factory = new ComponentFactory(); - - // Assert - Assert.Throws(()=>factory.InstantiateComponent(GetServiceProviderWithActivator(), componentType)); - } - - [Fact] - public void InstantiateComponent_AssignsPropertiesWithInjectAttribute() - { - // Arrange - var componentType = typeof(ComponentWithInjectProperties); - var factory = new ComponentFactory(); + var factory = new ComponentFactory(new CustomComponentActivator()); // Act var instance = factory.InstantiateComponent(GetServiceProvider(), componentType); // Assert Assert.NotNull(instance); - var component = Assert.IsType(instance); + var component = Assert.IsType(instance); // Custom activator returns a different type + // Public, and non-public properties, and properties with non-public setters should get assigned Assert.NotNull(component.Property1); Assert.NotNull(component.GetProperty2()); @@ -73,12 +59,24 @@ namespace Microsoft.AspNetCore.Components Assert.NotNull(component.Property4); } + [Fact] + public void InstantiateComponent_ThrowsForNullInstance() + { + // Arrange + var componentType = typeof(EmptyComponent); + var factory = new ComponentFactory(new NullResultComponentActivator()); + + // Act + var ex = Assert.Throws(() => factory.InstantiateComponent(GetServiceProvider(), componentType)); + Assert.Equal($"The component activator returned a null value for a component of type {componentType.FullName}.", ex.Message); + } + [Fact] public void InstantiateComponent_AssignsPropertiesWithInjectAttributeOnBaseType() { // Arrange var componentType = typeof(DerivedComponent); - var factory = new ComponentFactory(); + var factory = new ComponentFactory(new CustomComponentActivator()); // Act var instance = factory.InstantiateComponent(GetServiceProvider(), componentType); @@ -101,7 +99,7 @@ namespace Microsoft.AspNetCore.Components { // Arrange var componentType = typeof(ComponentWithNonInjectableProperties); - var factory = new ComponentFactory(); + var factory = new ComponentFactory(new DefaultComponentActivator()); // Act var instance = factory.InstantiateComponent(GetServiceProvider(), componentType); @@ -122,15 +120,6 @@ namespace Microsoft.AspNetCore.Components .BuildServiceProvider(); } - private static IServiceProvider GetServiceProviderWithActivator() - { - return new ServiceCollection() - .AddTransient() - .AddTransient() - .AddSingleton() - .BuildServiceProvider(); - } - private class EmptyComponent : IComponent { public void Attach(RenderHandle renderHandle) @@ -197,9 +186,23 @@ namespace Microsoft.AspNetCore.Components public TestService2 Property5 { get; set; } } - private class NonComponent { } - public class TestService1 { } public class TestService2 { } + + private class CustomComponentActivator : IComponentActivator where TResult : IComponent, new() + { + public IComponent CreateInstance(Type componentType) + { + return new TResult(); + } + } + + private class NullResultComponentActivator : IComponentActivator + { + public IComponent CreateInstance(Type componentType) + { + return null; + } + } } } diff --git a/src/Components/Components/test/DependencyInjectionTest.cs b/src/Components/Components/test/DependencyInjectionTest.cs index 2a77c75325..54e5e948ad 100644 --- a/src/Components/Components/test/DependencyInjectionTest.cs +++ b/src/Components/Components/test/DependencyInjectionTest.cs @@ -133,7 +133,7 @@ namespace Microsoft.AspNetCore.Components.Test } private T InstantiateComponent() where T: IComponent - => _renderer.InstantiateComponent(); + => (T)_renderer.InstantiateComponent(); class HasPropertiesWithoutInjectAttribute : TestComponent { diff --git a/src/Components/Components/test/OwningComponentBaseTest.cs b/src/Components/Components/test/OwningComponentBaseTest.cs index 155231b40b..2ece32bf97 100644 --- a/src/Components/Components/test/OwningComponentBaseTest.cs +++ b/src/Components/Components/test/OwningComponentBaseTest.cs @@ -23,7 +23,7 @@ namespace Microsoft.AspNetCore.Components var counter = serviceProvider.GetRequiredService(); var renderer = new TestRenderer(serviceProvider); - var component1 = renderer.InstantiateComponent(); + var component1 = (MyOwningComponent)renderer.InstantiateComponent(); Assert.NotNull(component1.MyService); Assert.Equal(1, counter.CreatedCount); diff --git a/src/Components/Components/test/RendererTest.cs b/src/Components/Components/test/RendererTest.cs index 5ecddb3ed6..2afaf99650 100644 --- a/src/Components/Components/test/RendererTest.cs +++ b/src/Components/Components/test/RendererTest.cs @@ -3733,6 +3733,35 @@ namespace Microsoft.AspNetCore.Components.Test Assert.Equal($"The {nameof(ParameterView)} instance can no longer be read because it has expired. {nameof(ParameterView)} can only be read synchronously and must not be stored for later use.", ex.Message); } + [Fact] + public void CanUseCustomComponentActivator() + { + // Arrange + var serviceProvider = new TestServiceProvider(); + var componentActivator = new TestComponentActivator(); + serviceProvider.AddService(componentActivator); + var renderer = new TestRenderer(serviceProvider); + + // Act: Ask for TestComponent + var suppliedComponent = renderer.InstantiateComponent(); + + // Assert: We actually receive MessageComponent + Assert.IsType(suppliedComponent); + Assert.Collection(componentActivator.RequestedComponentTypes, + requestedType => Assert.Equal(typeof(TestComponent), requestedType)); + } + + private class TestComponentActivator : IComponentActivator where TResult: IComponent, new() + { + public List RequestedComponentTypes { get; } = new List(); + + public IComponent CreateInstance(Type componentType) + { + RequestedComponentTypes.Add(componentType); + return new TResult(); + } + } + private class NoOpRenderer : Renderer { public NoOpRenderer() : base(new TestServiceProvider(), NullLoggerFactory.Instance) diff --git a/src/Components/Server/src/DependencyInjection/ComponentServiceCollectionExtensions.cs b/src/Components/Server/src/DependencyInjection/ComponentServiceCollectionExtensions.cs index b8e66a4e5e..cd65b7fdc4 100644 --- a/src/Components/Server/src/DependencyInjection/ComponentServiceCollectionExtensions.cs +++ b/src/Components/Server/src/DependencyInjection/ComponentServiceCollectionExtensions.cs @@ -74,7 +74,6 @@ namespace Microsoft.Extensions.DependencyInjection services.AddScoped(); services.AddScoped(); services.AddScoped(); - services.AddScoped(); services.TryAddEnumerable(ServiceDescriptor.Singleton, CircuitOptionsJSInteropDetailedErrorsConfiguration>()); diff --git a/src/Components/Shared/test/TestRenderer.cs b/src/Components/Shared/test/TestRenderer.cs index b44cdf479b..346ae4a417 100644 --- a/src/Components/Shared/test/TestRenderer.cs +++ b/src/Components/Shared/test/TestRenderer.cs @@ -88,8 +88,8 @@ namespace Microsoft.AspNetCore.Components.Test.Helpers return task; } - public T InstantiateComponent() where T : IComponent - => (T)InstantiateComponent(typeof(T)); + public IComponent InstantiateComponent() + => InstantiateComponent(typeof(T)); protected override void HandleException(Exception exception) { diff --git a/src/Mvc/Mvc.ViewFeatures/src/DependencyInjection/MvcViewFeaturesMvcCoreBuilderExtensions.cs b/src/Mvc/Mvc.ViewFeatures/src/DependencyInjection/MvcViewFeaturesMvcCoreBuilderExtensions.cs index 34b57eb374..84ffe50ce1 100644 --- a/src/Mvc/Mvc.ViewFeatures/src/DependencyInjection/MvcViewFeaturesMvcCoreBuilderExtensions.cs +++ b/src/Mvc/Mvc.ViewFeatures/src/DependencyInjection/MvcViewFeaturesMvcCoreBuilderExtensions.cs @@ -212,13 +212,12 @@ namespace Microsoft.Extensions.DependencyInjection services.TryAddScoped(); services.TryAddScoped(); services.TryAddScoped(); - + services.TryAddTransient(); // This does caching so it should stay singleton services.TryAddSingleton(); services.TryAddSingleton(); - services.TryAddSingleton(); // // Antiforgery