Add IComponentActivator by stsrki · Pull Request #19642 · dotnet/aspnetcore

stsrki · 2020-03-06T13:49:24Z

Currently components are instantiated with Activator.CreateInstance that is located in the internal class ComponentFactory. There is no way for authors of third-party components to change how components will be created.

With this feature I introduce the IComponentActivator to allow components to be instantiated by custom implementation.

I think this PR will address these issues.

dnfclas · 2020-03-06T13:49:40Z

All CLA requirements met.

stsrki · 2020-03-06T21:22:41Z

A lot of test were falling because IComponentActivator was not specified. I'm not sure where to register IComponentActivator with ServiceCollection since there is a LOT of tests. So instead I have made some changes to my original idea and if IComponentActivator is not registered within DI it will fallback to Activator.CreateInstance(componentType).

stsrki

@SteveSandersonMS After the last commit I think the commented lines can be safely removed but I wan't to see what you think before. tnx

stsrki · 2020-03-06T21:25:49Z

src/Components/Server/src/DependencyInjection/ComponentServiceCollectionExtensions.cs

            services.AddScoped<IJSRuntime, RemoteJSRuntime>();
            services.AddScoped<INavigationInterception, RemoteNavigationInterception>();
            services.AddScoped<AuthenticationStateProvider, ServerAuthenticationStateProvider>();
+            services.AddScoped<IComponentActivator, ComponentActivator>();


I think this can be now removed since activator will fallback to default implementation anyway.

stsrki · 2020-03-06T21:26:03Z

src/Mvc/Mvc.ViewFeatures/src/DependencyInjection/MvcViewFeaturesMvcCoreBuilderExtensions.cs

            services.TryAddScoped<NavigationManager, HttpNavigationManager>();
            services.TryAddScoped<IJSRuntime, UnsupportedJavaScriptRuntime>();
            services.TryAddScoped<INavigationInterception, UnsupportedNavigationInterception>();
+            services.TryAddScoped<IComponentActivator, ComponentActivator>();


Also can be removed.

Couldn't the default activator be a singleton? It doesn't hold any state.

mkArtakMSFT · 2020-03-11T16:36:17Z

Thanks for your PR, @stsrki.
We plan to take a closer look at this after we're done with the Blazor WASM release in May.

mkArtakMSFT · 2020-04-09T02:56:37Z

Update:
We're still busy wrapping up our upcoming Blazor WASM release, which is planned to go out in May. That forces us to delay some community PR reviews which target ASP.NET Core 5.0, so please expect some delays here. We will get to this as soon as we wrap up the Blazor work.

krynium · 2020-04-28T14:52:33Z

@dotnetjunkie

egil · 2020-06-15T15:20:53Z

src/Components/Components/src/IComponentActivator.cs

+        /// </summary>
+        /// <param name="componentType">The type of component to create.</param>
+        /// <returns>A reference to the newly created component.</returns>
+        object CreateInstance(Type componentType);


Suggested change

object CreateInstance(Type componentType);

IComponent CreateInstance(Type componentType);

Should this return an IComponent instead of just object? It seems that all components are IComponents, and having this returned here might save a cast higher up in the call stack.

You're probably right, but I just wanted to make it behave the same as Activator.CreateInstance(componentType) which is returning an object.

Since we require component instances to implement IComponent, I think it only makes sense if the activator returns that type.

Merge with aspnetcore repo

stsrki · 2020-06-30T10:28:54Z

I have updated this PR with latest changes from master branch!

@danroth27 I asked you couple of months ago when this PR will be given green light. The answer was probably for preview6 of .NET 5. Preview 6 is released but no info is given for this feature. Are there any plans soon for this?

@SteveSandersonMS You have been silent so far, but I would really like your feedback on this feature and how to proceed with it.

Sorry to bother you guys, I know you're already too busy! In case you need more info we can communicate over other channels, or we can proceed here. Just let me know what suits you best. Thanks!

SteveSandersonMS · 2020-06-30T11:42:34Z

Thanks @stsrki, I appreciate your continued follow-up on this!

By now, we're all pretty convinced this is a good feature and should be included. I'm scheduling time for it later this week. If you don't hear any confirmation by the end of the week please feel free to ping again, but I do expect to be getting to it :)

This means we expect it to be included in the preview 8 release, so I'm tracking it on that milestone too.

src/Components/Components/src/ComponentActivator.cs

SteveSandersonMS · 2020-06-30T11:49:31Z

src/Components/Components/src/ComponentFactory.cs

+            var activator = serviceProvider.GetService<IComponentActivator>();
+
+            var instance = activator != null
+                ? activator.CreateInstance(componentType)
+                : Activator.CreateInstance(componentType);


Instead of asking the service provider for IComponentActivator on every call, we'll need a way of caching the activator instance. This might force us to eliminate the static ComponentFactory.Instance and instead have the renderer get its own separate component factory instance during its own construction.

If you're able to look into doing that then great, otherwise I'll see what I can do to fix this up.

It would be great if you would look into this after I merge the changes. You have a lot more insight of what needs to be changed and where.

SteveSandersonMS · 2020-06-30T11:51:05Z

src/Components/Components/src/ComponentActivator.cs

+    /// <summary>
+    /// Default implementation of component activator.
+    /// </summary>
+    public class ComponentActivator : IComponentActivator


For consistency with other parts of the framework, can you rename this to DefaultComponentActivator?

SteveSandersonMS · 2020-06-30T11:53:05Z

@stsrki For us to be able to consider this, can you please also add tests?

stsrki · 2020-06-30T12:04:31Z

@stsrki For us to be able to consider this, can you please also add tests?

Sure, I will try to add them. There are some issue with the Components solution when I open it in VS, as I don't have all the references so the build is failing. Hopefully I will solve it.

Co-authored-by: Steve Sanderson <SteveSandersonMS@users.noreply.github.com>

…aspnetcore into feat-component-activator

stsrki · 2020-06-30T14:05:40Z

@SteveSandersonMS I have made changes based on your input and also added some basic tests. If you need anything else please let me know.

SteveSandersonMS · 2020-07-02T11:54:21Z

src/Components/Components/src/ComponentActivator.cs

@@ -0,0 +1,19 @@
+// Copyright (c) .NET Foundation. All rights reserved.


Note to self: rename file

Sorry about that, I will rename the file now.

No worries! I'm working on this PR now so if possible it would be good not to make further changes in case we conflict with each other. Is that OK?

Already renamed a file, didn't think you would be that fast to start working :)
OK, I will stop with changes from now on so you can continue. Please let me know if you need anything else.

PS. I really appreciate all your work. Thank you!

SteveSandersonMS · 2020-07-02T11:55:08Z

src/Components/Components/src/ComponentActivator.cs

+        /// <inheritdoc />
+        public IComponent? CreateInstance(Type componentType)
+        {
+            return Activator.CreateInstance(componentType) as IComponent;


Note to self: throw if null

SteveSandersonMS · 2020-07-02T12:08:27Z

I'm following up on this in #23607, so will close this in favour of that.

@stsrki - your commits are over in #23607 now, so you will still get attribution for this contribution. Thanks very much!

GTmAster · 2020-07-27T11:40:45Z

Hi, any information when it will be released in asp.net Core 3.1?

stsrki · 2020-07-27T11:42:20Z

@GTmAster As fa as I know the plan was to make it part of .Net 5 preview 8.

GTmAster · 2020-07-27T11:44:25Z

@stsrki This is not good, we are really expecting this to be part of current asp.net version. So there is no way to use non-conforming DI libraries as Simple Injector now with blazor?

egil · 2020-07-27T12:03:57Z

@GTmAster its only coming to .net 5. @SteveSandersonMS confirmed this in the related issue. And no, you cannot do it with other DI libraries, as the current way components are created is hardcoded inside Blazor's core.

GTmAster · 2020-07-27T12:12:19Z

@egil Thanks for clarification.

Add IComponentActivator

b822efc

stsrki requested a review from SteveSandersonMS as a code owner March 6, 2020 13:49

ghost added the area-blazor Includes: Blazor, Razor Components label Mar 6, 2020

Mladen Macanović and others added 2 commits March 6, 2020 15:28

Added missing DI registrations for SSB

295a477

Use default Activator if IComponentActivator is not specified in DI

dba8a02

stsrki commented Mar 6, 2020

View reviewed changes

mkArtakMSFT assigned SteveSandersonMS Mar 11, 2020

stsrki mentioned this pull request Apr 14, 2020

Prevent users from injecting their own components Megabit/Blazorise#571

Closed

Siphonophora mentioned this pull request May 19, 2020

Enable shallow rendering mode bUnit-dev/bUnit#17

Closed

MitchellNZ mentioned this pull request Jun 6, 2020

Feature: Vertical Bar options Megabit/Blazorise#954

Merged

16 tasks

SteveSandersonMS mentioned this pull request Jun 15, 2020

Test Blazor topic dotnet/AspNetCore.Docs#10747

Closed

2 tasks

egil reviewed Jun 15, 2020

View reviewed changes

stsrki and others added 2 commits June 30, 2020 11:29

Merge pull request #1 from dotnet/master

6bffe40

Merge with aspnetcore repo

Merge with master

36feb0e

SteveSandersonMS added this to the 5.0.0-preview8 milestone Jun 30, 2020

SteveSandersonMS reviewed Jun 30, 2020

View reviewed changes

src/Components/Components/src/ComponentActivator.cs Outdated Show resolved Hide resolved

SteveSandersonMS reviewed Jun 30, 2020

View reviewed changes

src/Components/Components/src/ComponentActivator.cs Outdated Show resolved Hide resolved

SteveSandersonMS reviewed Jun 30, 2020

View reviewed changes

Mladen Macanovic and others added 2 commits June 30, 2020 14:07

Rename ComponentActivator to DefaultComponentActivator

e6f84fb

Update src/Components/Components/src/ComponentActivator.cs

2990453

Co-authored-by: Steve Sanderson <SteveSandersonMS@users.noreply.github.com>

stsrki and others added 5 commits June 30, 2020 14:10

Update src/Components/Components/src/ComponentActivator.cs

469091b

Co-authored-by: Steve Sanderson <SteveSandersonMS@users.noreply.github.com>

Merge branch 'feat-component-activator' of https://github.com/stsrki/…

a9d9656

…aspnetcore into feat-component-activator

IComponentActivator to return IComponent type

69cb2dc

Register IComponentActivator as singleton

23e78d3

Tests for default component activator

160f45c

SteveSandersonMS reviewed Jul 2, 2020

View reviewed changes

Rename ComponentActivator file to DefaultComponentActivator

fcd0ac0

SteveSandersonMS mentioned this pull request Jul 2, 2020

Add IComponentActivator #23607

Merged

SteveSandersonMS closed this Jul 2, 2020

SteveSandersonMS mentioned this pull request Oct 6, 2020

How to integrate non-conforming containers with Razor Components? #8886

Closed

dotnetjunkie mentioned this pull request Dec 31, 2020

Allow the interception of property injection with Blazor Components #28957

Closed

dotnetjunkie mentioned this pull request Feb 11, 2021

Intercepting Blazor page events. #30115

Closed

	object CreateInstance(Type componentType);
	IComponent CreateInstance(Type componentType);

		@@ -0,0 +1,19 @@
		// Copyright (c) .NET Foundation. All rights reserved.

Conversation

stsrki commented Mar 6, 2020

Uh oh!

dnfclas commented Mar 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stsrki commented Mar 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stsrki left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkArtakMSFT commented Mar 11, 2020

Uh oh!

mkArtakMSFT commented Apr 9, 2020

Uh oh!

krynium commented Apr 28, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stsrki commented Jun 30, 2020

Uh oh!

SteveSandersonMS commented Jun 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SteveSandersonMS commented Jun 30, 2020

Uh oh!

stsrki commented Jun 30, 2020

Uh oh!

stsrki commented Jun 30, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SteveSandersonMS commented Jul 2, 2020

Uh oh!

GTmAster commented Jul 27, 2020

Uh oh!

stsrki commented Jul 27, 2020

Uh oh!

GTmAster commented Jul 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

egil commented Jul 27, 2020

Uh oh!

GTmAster commented Jul 27, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

dnfclas commented Mar 6, 2020 •

edited

Loading

stsrki commented Mar 6, 2020 •

edited

Loading

SteveSandersonMS commented Jun 30, 2020 •

edited

Loading

GTmAster commented Jul 27, 2020 •

edited

Loading