Skip to content

Commit

Permalink
Revert "Activate components by default" (#40526)
Browse files Browse the repository at this point in the history
Reverting type activation based on discussion in #40521

This reverts commit 197d527.
This reverts commit 0e7409e.
  • Loading branch information
pranavkm authored Mar 3, 2022
1 parent e9dddd6 commit 87f870d
Show file tree
Hide file tree
Showing 9 changed files with 71 additions and 128 deletions.
87 changes: 18 additions & 69 deletions src/Components/Components/src/ComponentFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using System.Diagnostics.CodeAnalysis;
using System.Reflection;
using Microsoft.AspNetCore.Components.Reflection;
using Microsoft.Extensions.DependencyInjection;
using static Microsoft.AspNetCore.Internal.LinkerFlags;

namespace Microsoft.AspNetCore.Components;
Expand All @@ -15,71 +14,46 @@ internal sealed class ComponentFactory
private const BindingFlags _injectablePropertyBindingFlags
= BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic;

private static readonly ConcurrentDictionary<Type, ComponentInitializer> _cachedInitializers = new();
private readonly IComponentActivator? _componentActivator;
private static readonly ConcurrentDictionary<Type, Action<IServiceProvider, IComponent>> _cachedInitializers = new();

public ComponentFactory(IComponentActivator? componentActivator)
private readonly IComponentActivator _componentActivator;

public ComponentFactory(IComponentActivator componentActivator)
{
_componentActivator = componentActivator;
_componentActivator = componentActivator ?? throw new ArgumentNullException(nameof(componentActivator));
}

public static void ClearCache() => _cachedInitializers.Clear();

public IComponent InstantiateComponent(IServiceProvider serviceProvider, [DynamicallyAccessedMembers(Component)] Type componentType)
{
if (_componentActivator is not null)
{
return InstantiateWithActivator(_componentActivator, serviceProvider, componentType);
}

return InstantiateDefault(serviceProvider, componentType);
}

private static IComponent InstantiateDefault(IServiceProvider serviceProvider, [DynamicallyAccessedMembers(Component)] Type componentType)
{
// This is thread-safe because _cachedInitializers is a ConcurrentDictionary.
// We might generate the initializer more than once for a given type, but would
// still produce the correct result.
if (!_cachedInitializers.TryGetValue(componentType, out var initializer))
{
if (!typeof(IComponent).IsAssignableFrom(componentType))
{
throw new ArgumentException($"The type {componentType.FullName} does not implement {nameof(IComponent)}.", nameof(componentType));
}

initializer = new(CreatePropertyInitializer(componentType), ActivatorUtilities.CreateFactory(componentType, Type.EmptyTypes));
_cachedInitializers.TryAdd(componentType, initializer);
}

return initializer.CreateDefault(serviceProvider);
}

private static IComponent InstantiateWithActivator(IComponentActivator componentActivator, IServiceProvider serviceProvider, [DynamicallyAccessedMembers(Component)] Type componentType)
{
var component = componentActivator.CreateInstance(componentType);
var component = _componentActivator.CreateInstance(componentType);
if (component is null)
{
// A user implemented IComponentActivator might return null.
// 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}.");
}

// Use the activated type instead of specified type since the activator may return different/ derived instances.
componentType = component.GetType();
PerformPropertyInjection(serviceProvider, component);
return component;
}

private static void PerformPropertyInjection(IServiceProvider serviceProvider, IComponent instance)
{
// This is thread-safe because _cachedInitializers is a ConcurrentDictionary.
// We might generate the initializer more than once for a given type, but would
// still produce the correct result.
if (!_cachedInitializers.TryGetValue(componentType, out var initializer))
var instanceType = instance.GetType();
if (!_cachedInitializers.TryGetValue(instanceType, out var initializer))
{
initializer = new(CreatePropertyInitializer(componentType));
_cachedInitializers.TryAdd(componentType, initializer);
initializer = CreateInitializer(instanceType);
_cachedInitializers.TryAdd(instanceType, initializer);
}

initializer.ActivateProperties(serviceProvider, component);
return component;
initializer(serviceProvider, instance);
}

private static Action<IServiceProvider, IComponent> CreatePropertyInitializer([DynamicallyAccessedMembers(Component)] Type type)
private static Action<IServiceProvider, IComponent> CreateInitializer([DynamicallyAccessedMembers(Component)] Type type)
{
// Do all the reflection up front
List<(string name, Type propertyType, PropertySetter setter)>? injectables = null;
Expand Down Expand Up @@ -119,29 +93,4 @@ void Initialize(IServiceProvider serviceProvider, IComponent component)
}
}
}

private readonly struct ComponentInitializer
{
private readonly Action<IServiceProvider, IComponent> _propertyInitializer;

private readonly ObjectFactory? _componentFactory;

public ComponentInitializer(Action<IServiceProvider, IComponent> propertyInitializer, ObjectFactory? componentFactory = null)
{
_propertyInitializer = propertyInitializer;
_componentFactory = componentFactory;
}

public IComponent CreateDefault(IServiceProvider serviceProvider)
{
var component = (IComponent)_componentFactory!(serviceProvider, Array.Empty<object?>());
ActivateProperties(serviceProvider, component);
return component;
}

public void ActivateProperties(IServiceProvider serviceProvider, IComponent component)
{
_propertyInitializer(serviceProvider, component);
}
}
}
22 changes: 22 additions & 0 deletions src/Components/Components/src/DefaultComponentActivator.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics.CodeAnalysis;

namespace Microsoft.AspNetCore.Components;

internal class DefaultComponentActivator : IComponentActivator
{
public static IComponentActivator Instance { get; } = new DefaultComponentActivator();

/// <inheritdoc />
public IComponent CreateInstance([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type componentType)
{
if (!typeof(IComponent).IsAssignableFrom(componentType))
{
throw new ArgumentException($"The type {componentType.FullName} does not implement {nameof(IComponent)}.", nameof(componentType));
}

return (IComponent)Activator.CreateInstance(componentType)!;
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="utf-8"?>
<linker>
<assembly fullname="Microsoft.AspNetCore.Components, Version=7.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60">
<assembly fullname="Microsoft.AspNetCore.Components, Version=6.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60">
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
<argument>IL2026</argument>
Expand Down Expand Up @@ -41,7 +41,7 @@
<argument>ILLink</argument>
<argument>IL2072</argument>
<property name="Scope">member</property>
<property name="Target">M:Microsoft.AspNetCore.Components.ComponentFactory.InstantiateWithActivator(Microsoft.AspNetCore.Components.IComponentActivator,System.IServiceProvider,System.Type)</property>
<property name="Target">M:Microsoft.AspNetCore.Components.ComponentFactory.PerformPropertyInjection(System.IServiceProvider,Microsoft.AspNetCore.Components.IComponent)</property>
</attribute>
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
Expand All @@ -59,7 +59,7 @@
<argument>ILLink</argument>
<argument>IL2077</argument>
<property name="Scope">member</property>
<property name="Target">M:Microsoft.AspNetCore.Components.ComponentFactory.CreatePropertyInitializer(System.Type)</property>
<property name="Target">M:Microsoft.AspNetCore.Components.ComponentFactory.CreateInitializer(System.Type)</property>
</attribute>
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
Expand Down
13 changes: 8 additions & 5 deletions src/Components/Components/src/RenderTree/Renderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public event UnhandledExceptionEventHandler UnhandledSynchronizationException
/// <param name="serviceProvider">The <see cref="IServiceProvider"/> to be used when initializing components.</param>
/// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
public Renderer(IServiceProvider serviceProvider, ILoggerFactory loggerFactory)
: this(serviceProvider, loggerFactory, GetComponentActivatorOrDefault(serviceProvider)!)
: this(serviceProvider, loggerFactory, GetComponentActivatorOrDefault(serviceProvider))
{
// This overload is provided for back-compatibility
}
Expand All @@ -87,8 +87,10 @@ public Renderer(IServiceProvider serviceProvider, ILoggerFactory loggerFactory,
throw new ArgumentNullException(nameof(loggerFactory));
}

// Even though IComponentActivator is not marked as nullable, we do allow null because that's how the framework internally indicates
// that we should use default activation logic.
if (componentActivator is null)
{
throw new ArgumentNullException(nameof(componentActivator));
}

_serviceProvider = serviceProvider;
_logger = loggerFactory.CreateLogger<Renderer>();
Expand All @@ -97,9 +99,10 @@ public Renderer(IServiceProvider serviceProvider, ILoggerFactory loggerFactory,

internal HotReloadManager HotReloadManager { get; set; } = HotReloadManager.Default;

private static IComponentActivator? GetComponentActivatorOrDefault(IServiceProvider serviceProvider)
private static IComponentActivator GetComponentActivatorOrDefault(IServiceProvider serviceProvider)
{
return serviceProvider.GetService<IComponentActivator>();
return serviceProvider.GetService<IComponentActivator>()
?? DefaultComponentActivator.Instance;
}

/// <summary>
Expand Down
49 changes: 3 additions & 46 deletions src/Components/Components/test/ComponentFactoryTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public void InstantiateComponent_CreatesInstance()
{
// Arrange
var componentType = typeof(EmptyComponent);
var factory = new ComponentFactory(null);
var factory = new ComponentFactory(new DefaultComponentActivator());

// Act
var instance = factory.InstantiateComponent(GetServiceProvider(), componentType);
Expand All @@ -22,30 +22,12 @@ public void InstantiateComponent_CreatesInstance()
Assert.IsType<EmptyComponent>(instance);
}

[Fact]
public void InstantiateComponent_CreatesInstance_WithTypeActivation()
{
// Arrange
var componentType = typeof(ComponentWithConstructorInjection);
var factory = new ComponentFactory(null);

// Act
var instance = factory.InstantiateComponent(GetServiceProvider(), componentType);

// Assert
Assert.NotNull(instance);
var component = Assert.IsType<ComponentWithConstructorInjection>(instance);
Assert.NotNull(component.Property1);
Assert.NotNull(component.Property2);
Assert.NotNull(component.Property3); // Property injection should still work.
}

[Fact]
public void InstantiateComponent_CreatesInstance_NonComponent()
{
// Arrange
var componentType = typeof(List<string>);
var factory = new ComponentFactory(null);
var factory = new ComponentFactory(new DefaultComponentActivator());

// Assert
var ex = Assert.Throws<ArgumentException>(() => factory.InstantiateComponent(GetServiceProvider(), componentType));
Expand Down Expand Up @@ -113,7 +95,7 @@ public void InstantiateComponent_IgnoresPropertiesWithoutInjectAttribute()
{
// Arrange
var componentType = typeof(ComponentWithNonInjectableProperties);
var factory = new ComponentFactory(null);
var factory = new ComponentFactory(new DefaultComponentActivator());

// Act
var instance = factory.InstantiateComponent(GetServiceProvider(), componentType);
Expand Down Expand Up @@ -147,31 +129,6 @@ public Task SetParametersAsync(ParameterView parameters)
}
}

public class ComponentWithConstructorInjection : IComponent
{
public ComponentWithConstructorInjection(TestService1 property1, TestService2 property2)
{
Property1 = property1;
Property2 = property2;
}

public TestService1 Property1 { get; }
public TestService2 Property2 { get; }

[Inject]
public TestService2 Property3 { get; set; }

public void Attach(RenderHandle renderHandle)
{
throw new NotImplementedException();
}

public Task SetParametersAsync(ParameterView parameters)
{
throw new NotImplementedException();
}
}

private class ComponentWithInjectProperties : IComponent
{
[Inject]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="utf-8"?>
<linker>
<assembly fullname="Microsoft.AspNetCore.Components.Forms, Version=7.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60">
<assembly fullname="Microsoft.AspNetCore.Components.Forms, Version=6.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60">
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
<argument>IL2026</argument>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,18 @@
<?xml version="1.0" encoding="utf-8"?>
<linker>
<assembly fullname="Microsoft.AspNetCore.Components.Web, Version=7.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60">
<assembly fullname="Microsoft.AspNetCore.Components.Web, Version=6.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60">
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
<argument>IL2026</argument>
<property name="Scope">member</property>
<property name="Target">M:Microsoft.AspNetCore.Components.Web.Infrastructure.JSComponentInterop.SetRootComponentParameters(System.Int32,System.Int32,System.Text.Json.JsonElement,System.Text.Json.JsonSerializerOptions)</property>
</attribute>
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
<argument>IL2026</argument>
<property name="Scope">member</property>
<property name="Target">M:Microsoft.AspNetCore.Components.Web.WebEventData.ParseEventArgsJson(Microsoft.AspNetCore.Components.RenderTree.Renderer,System.Text.Json.JsonSerializerOptions,System.UInt64,System.String,System.Text.Json.JsonElement)</property>
</attribute>
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
<argument>IL2062</argument>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<linker>
<assembly fullname="Microsoft.AspNetCore.Components.WebAssembly.Authentication, Version=42.42.42.42, Culture=neutral, PublicKeyToken=adb9793829ddae60">
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
Expand All @@ -14,4 +14,4 @@
<property name="Target">M:Microsoft.Extensions.DependencyInjection.WebAssemblyAuthenticationServiceCollectionExtensions.&lt;&gt;c__1`1.&lt;AddAuthenticationStateProvider&gt;b__1_0(System.IServiceProvider)</property>
</attribute>
</assembly>
</linker>
</linker>
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="utf-8"?>
<linker>
<assembly fullname="Microsoft.JSInterop, Version=7.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60">
<assembly fullname="Microsoft.JSInterop, Version=6.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60">
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
<argument>IL2026</argument>
Expand Down

0 comments on commit 87f870d

Please sign in to comment.