Prism: Standardize on framework-provided Dependency Injection abstractions, patterns

[ericwj]

With the upcoming .NET Standard 2.0 release of Prism, there is an opportunity to simplify Prism and bring it to a new standard of patterns and practices and clean up a number of issues from the code.

The problems

Let me list a number of things that I consider issues with Prism today. I've been looking at Prism.Windows exclusively so far.

On the highest level, or as a gist, my opinion is

• PrismApplication is needlessly complex - confusing I might say, especially for new users - and can be vastly simplified
• Dependency injection appears to be brought in as an afterthought, rather than as a pattern that is consistently applied
• ViewModels must be different design-time and run-time, which means e.g. the sample app goes about declaring their 'shape' just to allow them to work. But this defeats x:Bind, is brittle and just one more file to have open to the side next to the XAML and the 'real' ViewModel. This, most of all, should be fixed for me to 'love' Prism.

On the lowest level, looking through the code, the above is reflected in the following issues:

• CommonServiceLocator is broken (#1039) and represents an anti-pattern
• New is glue. There's too much of it.
• Most virtual methods in the Application classes are factories that are there twice. They can be removed, in phases, to maintain compatibility for a while.
• A lot of code is duplicated - or at least repetitive - amongst the DI containers that Prism supports.
• Most properties on Application classes are in fact offering references to services. Accessing them is through the static App.Current, which is a code smell.
• InitializeShell indirectly creates the dependency injection container (through InitializeFrameAsync). Why?
• Prism uses IContainer.Update which is rightfully deprecated by Autofac, requires the dependency injection container is mutable, registers instances after they are created instead of factories with DI.
• There are a number of implicit 'services' which are brought in as some Func'N. An explicitly declared interface, with proper documentation tags would be preferable.
• Application classes are extended for each distinct dependency injection provider to offer custom means to register services. These can be standardized.

Proposal

My proposal to fix these issues is as follows.

• Adopt dependency injection completely and thoroughly throughout the whole Prism code base.
  
  
  
  • Standardize on the system-provided System.IServiceProvider.
  
  
  • Make the dependency injection container immutable
  
  
  • Create it as early as possible during app startup. In a code path that does just that and has names to reflect that intention.
  
  
  • Standardize on Microsoft.Extensions.DependencyInjection and it's implementation of IServiceCollection during creation of the DI container.
  
  
  • Since this already requires Microsoft.Extensions.DependencyInjection as a dependency - as opposed to the .Abstractions package - this implementation should be the default DI container just as it is in ASP.NET.
  
  
• Remove methods on Application classes meant to be overridden by DI providers. They aren't needed.
• Keep the methods meant to be overridden by app devs - just for compatibility - and register them as factories with DI by default, but mark them obsolete.
• Replace uses of Func'N with properly named interfaces and default implementations, which optionally take a Func'N constructor argument. This can be made transparent to app devs.
• Plan for a major version bump that practically empties Application classes, apart from stuff that deals with launch and suspension, etc. These are Application concerns.
• Redesign view model resolution, to be compatible with x:Bind, destroy the practice demonstrated in the sample app to 'mimic' view model shapes in separate design-time implementations, allows mocks to be resolved from DI in design time such that real view models are creatable (the constructor can run)

Benefits

• Very easy to switch DI containers
• You don't need to figure out which DI container before even referencing Prism
• The developer can now opt into another DI container, e.g. by adding Prism.Autofac after writing half an application with no code changes whatsoever
• Adding say Autofac merely adds features (e.g. property injection), instead of requiring changes to even work
• Two files per view are too much - well, enough certainly. The third can be utterly destroyed, please.
• View models can be injected with design-time mocks of services (or even nulls), such that they are creatable.

Example: INavigationService

PrismApplication defines CreateNavigationService and OnCreateNavigationService. The latter returns null and both are virtual. Which one is used is determined with ??.

A better design would be to consider INavigationService a service like any other. In fact it is, apart from the requirement that it is created only after there is in fact a Frame to use. So it should be created during application initialization and PrismApplication should force that behavior.

InitializeFrameAsync should resolve INavigationService from the dependency injection provider instead of hard-wiring the call to the CreateNavigationService factory. This today isn't possible, because there is no common dependency injection abstraction. Well there is IServiceProvider, but it isn't used. It should've been used as such imho, but today InitializeFrameAsync is written on the assumption it would require a reference to IContainer (for Autofac). This is backwards.

Adding Microsoft.Extensions.DependencyInjection, PrismApplication is now capable both to register services (through IServiceCollection) in a standard manner, as well as resolve them.

To maintain compatibility with existing code using Prism, preferably already in the PrismApplication constructor, user code should be called to offer an opportunity to register an INavigationService implementation and a means to create it. These are the responsibilities of dependency injection.

After that, still preferably in the constructor, PrismApplication could then register IServiceCollection.TryAddSingleton(provider => OnCreateNavigationService(...)) such that

• It is guaranteed that the service is registered
• If OnCreateNavigationService is overriden by the app developer, it is being used
• If INavigationService is replaced by some other implementation and means to create it, e.g. IServiceCollection.AddSingleton<INavigationService, MyFancyNav>() it just works bypassing PrismApplication methods completely.

Then just [Obsolete] both CreateNavigationService and OnCreateNavigationService, preferably. They are factories and not Application concerns.

Subsequently, after bumping the major version,

• change the FrameNavigationService implementation to take a constructor parameter to receive IFrameFacade or an IFrameFacadeProvider of sorts such that DI can be made responsible for creating the service
• the registration of INavigationService can then be replaced with IServiceCollection.TryAddSingleton<INavigationService, FrameNavigationService>().

Having CreateNavigationService on PrismApplication and hardwiring the call makes modifying the dependencies of FrameNavigationSerice a breaking change or a bad experience or even a hack to write. And this is true for all those services: ISessionStateService, IDeviceGestureService, IEventAggregator, ...

[ericwj]

985

[brianlagunas]

Thanks for this detailed write-up. In fact, DI was an after thought or Prism.Windows :)

We are actually in talks now about an update/rewrite of Pism.Windows to make it more consistent and improve the API. These notes will be very helpful during these discussions.

[ericwj]

What I didn't mention was that - if everything is going to be .NET Standard in the fall presumably, Prism would be able to provide common abstractions for all stuff useful in ViewModels, such that they can truly be written once for most apps across Xamarin, UWP, WPF.

I was basically seeing where I'd end up refactoring 6.3.0 like this. I have it mostly working, except for the ViewModelLocator stuff. But in my own app I in fact use a static view model factory which fills in nulls for constructor arguments except for a pair that are commonly used in ViewModel constructors, unless an IServiceProvider is set during application initialization.

This is most importantly about IDesignMode and ICommandFactory. And I find it very useful to wrap those into properties of IUIServices such that ViewModels can have a short list of constructor arguments and automatically get IDispatcher, IAlertService and things like that.

[brianlagunas]

Feel free to share what you have. The more information we have the better.

[ericwj]

Don't think the UWP label reflects the intention.

Going about this, it should really be about all platforms. One .NET, One Prism.

[brianlagunas]

Currently, Microsoft.Extensions.DependencyInjection is not compatible with Xamarin.Forms until XF gets support for .NET Standard.

[ericwj]

Well, this would build on .NET Standard 2.0 in the first place. To release in Prism 7.0 by fall I suppose.

Don't mind me just poccing this for private use - though I'm happy to share my fork building on UWP 5.3.3.

[bartlannoeye]

Keep us informed of your progress (maybe a link to the fork where you'll be working on) in case you start before we pick it up ourselves.

FYI: We still have to bring quite a few application lifecycle events (right now Prism is only supporting about half of them) and the session state is under work as well.

[ericwj]

It'll be at /ericwj/Prism in a branch - I'll shout something here when there is something interesting to see.

[brianlagunas]

Stage one will be to get .NET Standard support. Stage two will be to review Prism for UWP. Then these types of conversation will begin.

[CriggerMarg]

WPF code have same problems, so it isn't only UWP problem. I referenced similiar issue with RegionNavigationService class.

[brianlagunas]

Have you made any progress on your branch? I would be interested to see what you have so far.

[brianlagunas]

So I spent a couple of hours researching this and the needs of the Prism library as a whole (not just UWP), and the IServiceProvider will not meet our needs. We need access to the actual containers to use their more advanced registration and resolution features. We also need our containers to be mutable to properly support modules.

Unless I am overlooking something obvious, we will have to continue using the approach we have now, minus the CommonServiceLocator.

[ericwj]

Did you consider that there is the possibility to create and use subordinate service providers? These can be created after the root service provider has been created and they can resolve from the subordinate or the root provider, overriding/replacing and/or adding to what the root provider can resolve. I can't really tell whether this will be enough for modules to work.

I recall when I poc'ed this I needed a container specific virtual method to properly do the provider specific registration/creation and a namespace import to implement some provider specific extension methods for resolution, but I ended up with a single base Prism application class and the virtual method would be in the user application class. I did not get to look into modules, however.

[dazinator]

We need access to the actual containers to use their more advanced registration and resolution features.

Could you elaborate on the features that are lacking. I know one limitation is that you can't create Child containers via IServiceProvider (You can create a "scoped" container - which is an immutable nested container usually used for Per Request scoping in web apps). And I know you can't mutate the registrations for an existing IServiceProvider. Is there any others?

I was also in this kind of scenario recently with my own framework and in the end I decided to extend IServiceProvider with my own requirements:

 public interface ITenantContainerAdaptor : IServiceProvider, IDisposable
    {
        ITenantContainerAdaptor CreateNestedContainer();
        ITenantContainerAdaptor CreateChildContainer();

        /// <summary>
        /// Used to add services to a container AFTER its initialised.
        /// </summary>
        /// <param name="configure"></param>
        void Configure(Action<IServiceCollection> configure);

        string ContainerName { get; }
        Guid ContainerId { get; }
        ContainerRole Role { get; }
    }

So my framework requires an ITenantContainerAdaptor implementation to be configured on startup. This allows me to create child containers, and mutate the container, in the places necessary, but it has the downside that I had to create the actual concrete container implementations as support packages - i.e an StructureMap implementation which the user configured on startup like so:

https://github.com/dazinator/Dotnettency/blob/develop/src/Sample.Mvc/Startup.cs#L50

And I will need to create additional implementations for the other containers as needed, such as Ninject etc.

But it does have the benefit that it allows me to unify around IServiceProvider wherever I need to, and also allows me to expose an IServiceCollection registration API experience that is agnostic of any underlying container. This is useful when you want to dynamically load a module - and the module may be a third party module and doesn't want to be tied to any specific container when it registers its services. It means the host can change the container without impacting any modules. I have also found that the IServiceCollection registration API is usually sufficient for most needs. Especially given that there are many libraries now on the netstandard stack (from asp.net core) which offer a fluent registration API for IServiceCollection - like EF Core as one minor example.

[brianlagunas]

Another issue is that we need to stick to .NET Standard 1.0. So I may just create my own IServiceProvider implementation that accomplishes what I need.

[brianlagunas]

I can't rely on IServiceProvider, but that doesn't mean we can't standardize on our own façade API around an actual DI container. I was thinking of creating an interface that would act as a façade and provide the functionality required by Prism. For more advanced usage, the developer could gain access to the actual DI container if necessary. Maybe something like

 public interface IContainer
    {
        object Instance { get; } //the actual DI container instance

        object GetService(Type type);

        object GetService(Type type, string name);

        T GetService<T>();

        T GetService<T>(string name);

        void RegisterInstance<TInterface>(TInterface instance);

        void RegisterSingleton<TFrom, TTo>() where TTo : TFrom;

        void RegisterType(Type from, Type to);

        void RegisterType(Type from, Type to, string name);

        void RegisterType<TFrom, TTo>(string name) where TTo : TFrom;
    }

Then the Prism code base would always rely on these methods. As long as the DI container extension projects implemented these, then it would be a seamless integration for any container.

Thoughts?

[dazinator]

My personal preference was to have my façade extend IServiceProvider to get the GetService methods for free (including the generic ones, which Microsoft expose as Extensions methods).

For the Register methods, I chose to do this on my façade:

  void Configure(Action<IServiceCollection> configure);

Because then you get to leverage the existing / framework way for type registration including the various overloads and extensions. It means new ways to register types can be added for consumers, without breaking changes to the facade interface.

Most containers are now providing integration with IServiceProvider - and IServiceCollection - so this means more code is actually maintained by others (IServiceProvider stuff maintained by Microsoft, and IServiceProvider / IServiceCollection integrations usually published as a support package by container authors wanting to be compatible with the asp.net core / netstandard stack.

However if you really don't want to go down that approach, defining it all in your own façade is another approach with different tradeoffs I guess.

Here is what this looked like for structuremap

[brianlagunas]

As I mentioned, we can't use IServiceProvider as we are in netstandard 1.0. The ServiceProvider and IServiceCollection is in netstandard 2.0. Also, there are a number of features that Prism needs that are to limited in the IServiceCollection implementation. Such as named instances and registrations, as well as a mutable container.

[brianlagunas]

By the way I am separating these methods into separate interfaces

    public interface IContainerExtension: IContainer, IContainerRegistry
    {

    }

    public interface IContainer
    {
        object Instance { get; }

        object Resolve(Type type);

        object Resolve(Type type, string name);

        T Resolve<T>();

        T Resolve<T>(string name);
    }



md5-594d1061cce3016a4c35d3541200e84c



    public interface IContainerRegistry
    {
        void RegisterInstance<TInterface>(TInterface instance);

        void RegisterSingleton<T>(); //maybe

        void RegisterSingleton<TFrom, TTo>() where TTo : TFrom;

        void RegisterType(Type from, Type to);

        void RegisterType(Type from, Type to, string name);

        void RegisterType<T>(); //maybe

        void RegisterType<T>(string name); //maybe

        void RegisterType<TFrom, TTo>() where TTo : TFrom;

        void RegisterType<TFrom, TTo>(string name) where TTo : TFrom;
    }

[dazinator]

The ServiceProvider and IServiceCollection is in netstandard 2.

Fyi these abstractions are available since netstandard1.1, not sure of that makes any difference.

https://www.nuget.org/packages/Microsoft.Extensions.DependencyInjection/1.0.0

Such as named instances and registrations, as well as a mutable container.

Yeah I understand, My own abstraction derives from IServiceProvider to add mutability support (as well as child and nested container support but at the end of the day its still an IServiceProvider which suited me for asp.net core scenario - obviously your own goals and scenarios may be different.

Named registrations is interesting. There are already support packages out there to provide named registration extension methods for IServiceCollection, so that shouldnt be a blocker. In fact if you adopted IServiceCollection abstraction you would get this stuff for free via the existing packages that people publish around these abstractions. That was important for me but perhaps not for you.

[brianlagunas]

netstandard 1.1 is not netstandard 1.0.

Also, we keep our dependencies minimal and only on the absolutely required packages such as Xamarin.Forms and a container. It becomes dangerous to take dependencies on other packages, especially those maintained by individuals as they can abandon the project at any time.

IServiceProvider is not common in desktop or Xamarin development. It's market really is in the asp.net space. Adding support for IServiceProvdier doesn't buy Prism anything. There is no real benefit to explicitly supporting IServiceProvider in Prism.

I think the intent of this post was to create a more standardized DI API that was more plug & play without all the ceremony. This new façade API accomplishes this. While IServiceProvider might have been nice to use, it does not exist in the target framework.

[dazinator]

netstandard 1.1 is not netstandard 1.0.

My point was these API's are available in netstandard1.1 which is notnetstandard2.0 as you had asserted. I have looked at the existing projects that are targeting netstandard1.0 as I wanted to check if incrementing them to netstandard1.1 would be viable. The only difference I can see between netstandard1.0 and netstandard1.1 is that the former supports Windows Phone Silverlight where as netstandard1.1 is not implemented by Windows Phone Silverlight. My understanding (and I am sure you can correct me if I am wrong) is that you aren't interested in supporting windows phone silverlight - in which case - incrementing from netstandard1.0 to netstandard1.1 might be a viable option for you.

It becomes dangerous to take dependencies on other packages, especially those maintained by individuals as they can abandon the project at any time.

I completely agree. The dependency we are taking about here is Microsoft.Extensions.DependencyInjection which is not maintained by an individual developer and is extremely widely used, in real world apps and is a central to the net core stack. You would also be able to drop CommonServiceLocator as a dependency.

IServiceProvider is not common in desktop or Xamarin development.

IServiceProvider has been in the framework since .NET 1.1 and I grant that it may not have been commonly used in desktop applications (even though System.ComponentModel makes use of it) because developers want an actual Container with decent features. IServiceProvider is an abstraction for a container, and not a replacement for one..

I think the intent of this post was to create a more standardized DI API that was more plug & play without all the ceremony.

Read all the bullet points, and I think the intent of the post is clearly more opinionated than that, and I agree with many of those bullet points listed! I will leave it at that.

P.s creating your own abstraction as you have suggested would still be an improvement over what is in place today so kudos if you do go ahead with that.

[dvorn]

@brianlagunas I suggest to have in the interfaces only basic methods making other extension methods:

public interface IContainer
{
        object Instance { get; }
        object Resolve(Type type, string name);
}

public interface IContainerRegistry
{
        void RegisterInstance<TInterface>(TInterface instance);
        void RegisterSingleton<TFrom, TTo>() where TTo : TFrom;
        void RegisterType(Type from, Type to, string name);
    }

In the future, in C# 8.0, extension methods will become interface default methods

[brianlagunas]

@dazinator IServiceProvider is not in netstandard1.1. Regardless, as far as Prism is concerned, there is no benefit to using IServiceProvider over using our own façade interfaces. Also, only WPF uses CommonServiceLocator. No other platform requires it to function. UWP added it as a courtesy, but it will be removed in the next release. Also, we were talking about more that just one dependency, as you recommend other packages that support the named registrations., That, or I would have to write those anyways on top of the newly added dependencies. I feel more comfortable not having to change target platforms, and instead controlling what's required for Prism to function with as few dependencies as possible. Instead of using IServiceProvider as the abstraction, I am using my own interfaces which will accomplish the same thing. My biggest hang up is that we require named registrations and resolutions, as well as access to the actual container for more advanced scenarios.

@dvorn Good idea. I'll take a look at which methods can be extension methods, and which ones would be required. This will cut down a little on the core required to create a Prism extension.

[dazinator]

IServiceProvider is not in netstandard1.1.

It is:-

there is no benefit to using IServiceProvider over using our own façade interfaces.

There is a benefit to using IServiceCollection for registration of types. There is a large ecosystem of libraries that offer fluent api's for configuring them on startup with IServiceCollection becuase thats how they operate in an asp.net core scenario, or when targeting netstandard.

Consider Xamarin apps - you can now use EF Core (Microsofts premier data access library.). How do you configure EF Core in the container? The answer is.. EF Core comes with a fluent API for IServiceCollection so you can configure it like this:-

services.AddDbContext<BloggingContext>(options => options.UseSqlite("Data Source=blog.db"));

May I ask how we would register EF Core with a new abstraction? My assumption is we would have to examine Microsoft's code, then work out what services there fluent API registers, then perform the equivalent registrations with the new prism abstraction. If Prism exposed IServiceCollection for registration then pretty much any dependency used in netcore (i,e targeting netstandard) becomes compatible.

Anyway, I feel I have said enough on this topic, ultimately the decision is yours to make, and if you don't see any benefit that's fine.

[dansiegel]

@dazinator bottom line is we do not directly support 1.1... Prism.Autofac.Forms does because Autofac itself only goes as low as 1.1. We support either 1.0 or 2.0 for Prism Forms. I understand your issues with wanting to register EntityFramework using the Fluent API.

@brianlagunas and I have spent a lot of time discussing this issue, and ultimately we cannot directly rely on IServiceProvider and IServiceCollection for a variety of reasons that extend beyond whether or not they are available. That said once we have have worked out how our interfaces will work we can consider extending it to work with libraries like EntityFramework provided you are not using netstandard1.0

[brianlagunas]

Interesting. When I was trying it out, it didn't show up for me. I must have fat fingered something. If IServiceCollection can do everything we need it to then I would be open to using it, but it is limited in it's current capacity, For example, we need to be able to register and resolve named instances. Unless I am missing something obvious, it doesn't have an API for that, and since it doesn't give you access to the container, you can't write an extension method to do it. Unless you have some ideas you can share? We also have to considers things such as modularity which requires a mutable container and API.

Remember, we have to consider all platforms and we aren't talking just about UWP.

[dazinator]

@brianlagunas - re: named registrations, see: https://github.com/yuriy-nelipovich/DependencyInjection.Extensions - I haven't looked at it extensively but it looked simple enough at first glance. This package consists of about 4 classes.

[brianlagunas]

Thanks for the link, I'll take a look, but at first glance that API is not going to work.

[dvorn]

@brianlagunas I think you can move more methods to extensions. Take a look at how e.g. Unity is really implemented: http://unity.codeplex.com/SourceControl/latest#source/Unity/Src/UnityContainerExtensions.cs

Note that if a particular extension method is not suitable for some container (other than Unity), it can be overridden as an instance method which takes precedence over extension methods.

[brianlagunas]

@dvorn Yeah, I moved a lot into extensions. You can check out the branch

https://github.com/PrismLibrary/Prism/tree/Container-Abstractions/Source/Xamarin/Prism.Forms/Ioc

[dvorn]

@brianlagunas I mean you can move even more. E.g., void RegisterType(Type type); is in fact void RegisterType(Type from, Type to); where from and to are type.

[brianlagunas]

Sweet. Didn't catch that! I'll take another look.

[sharpninja]

         object GetService(Type type);

         object GetService(Type type, string name);

         T GetService<T>();

         T GetService<T>(string name);

Please name these methods something else. They are standardized in Microsoft.Extensions.DependencyInjection and as such the naming collisions confuse Intellisense and you wind up with the wrong using.

[brianlagunas]

@sharpninja Where are you seeing those method names? Also, just because Microsoft names something one way doesn't mean it's the standard.

[sharpninja]

@sharpninja Where are you seeing those method names? Also, just because Microsoft names something one way doesn't mean it's the standard.

I was replying to this. But so many things in Microsoft.Extensions are horribly name abused and you wind up having to do all kinds of namespace hysterics when there are collisions.

[brianlagunas]

@sharpninja the code you are referring to is not how they are named in Prism. That was used as an example when this idea was first proposed back in Nov of 2017

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.