-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Autofac ContainerBuilder.Update Obsolete #969
Comments
I don't use Autofac so I rely on the community to support it. If you would like, you can submit a PR with your suggested improvements. If you decide to submit a PR, please fix it for all platforms. |
Conversely, I don't use Prism, so I'm not really sure I can do much to help you. I'm one of the Autofac maintainers and figured you should at least be aware. Perhaps someone in the community who uses both products can help out. |
Thanks for the heads up. I will leave this open and give it the "Help Wanted" tag and hope an experienced Autofac dev picks this up. |
I will try it. |
Hi, |
@perssondan Thank you. I do not have a lot of time now. |
@NewBLife @perssondan Hello all, for the last few days I have been working on a fix to get rid of the use of ContainerBuilder.Update() in Prism.Autofac. If you want to monitor my progress, the branch I am working in is here. My idea is that the updated version of Prism.Autofac will have two modes for the container - Mutable and Immutable:
You set Prism to use the new mode early in your application initialization with: At some point in the future, there will be a breaking change where Immutable becomes the default, and you will have to set the PrismApplication.ContainerType to AutofacContainerType.Mutable to keep using ContainerBuilder.Update(). Please understand that my update effort is a work in progress and I am still changing things around as I run into issues related to when Prism wants to register types in the IOC container. My only reason for adding this comment at this time is just to let people know that I am working on this, so other people don't feel like they need to (but they are welcome to come up with a better solution, of course). |
@ellisnet are you on the Slack Channel ? |
@dansiegel I am not. Should I be? Do I need a Slack subscription? |
@ellisnet not a requirement, but I usually suggest it since it's where you can quickly and easily get help if you're not sure about an implementation or want someone to take a look at something. |
Honestly I would prefer a breaking change to implement it the right way. This break would be published in our 7.0 update when we move to .NET Standard. Autofac users should understand that their DI container has deprecated the Update method so they will know this pain is coming. |
@dansiegel got me on Slack... @brianlagunas Understood. I am moving quickly, so at this exact moment I would like to keep implementing/testing it the way I am; it will be easy to rip out the parts that keep it backwards compatible. |
I also noticed the Xamarin integration appears to reference |
@tillig I believe once we've adopted NetCore support we can adopt Autofac 4.X and update the implementation then. |
I added a response to @tillig in the Autofac issue discussion - to explain that ContainerBuilder.Update() is being removed from Prism.Autofac.Forms. |
All - One question: I can't change the definition of IModule, and not all modules will need to register anything with the IoC container. So I created a new So - this:
Becomes this:
(The above Important note: Not all modules will need to register types, so Rules for modules that implement
So, I guess my question is: Are we okay with that? Does someone have a better suggestion for the name of the Thoughts? |
No chance that immutable container will work for WPF modules: they may be loaded on demand any time after the application has started and running. Example: https://www.infragistics.com/community/blogs/blagunas/archive/2013/08/06/prism-dynamically-discover-and-load-modules-at-runtime.aspx |
@dvorn I agree with you that a single immutable container - as is expected to be used by Prism.Autofac.Forms - doesn't look like it can work for WPF and dynamically-loading-modules scenarios. I should be clear and say that I am trying to use a single dedicated Autofac IoC container for Prism.Forms' needs and for common things like INavigationService, etc. There is nothing to stop an app that consumes Prism.Autofac.Forms from creating/using/disposing/re-creating its own Autofac containers. Autofac is a flexible and powerful DI library, and by pulling in Prism.Autofac.Forms library into your app project, you already have the Autofac NuGet. So, you could use that to create any containers that you need. And from what I read in the blog you referenced, it sounds like WPF might need a multi-container solution - with some containers not being created at app initialization. But [it is my understanding that] each container should be immutable, which is why they are eliminating the ContainerBuilder.Update() method. |
@ellisnet Mission: impossible. The whole idea of dynamic module loading is the following: you have an app which has menu items/button which, when triggered, are causing some navigation to a new view. The navigation system is based on resolving the new view via the container. After we loaded new module, we get new menu items/buttons which can navigate to new views, which previously did not exist in the container, but now the newly loaded module has registered them. It is true that we may change the implementation of Prism/applications to use multiple immutable containers and create a system where the navigation system operates not on a single container, but on a chain of dynamically created immutable containers. One is created initially, another when module A is loaded, etc. But then we will have in Prism/applications all kind of problems you describe in autofac/Autofac#811. Essentially, you avoid those problems by shifting them to Prism. Do we want that? I work mostly with Prism for WPF and know little about Forms. However, I guess that the original intention of Prism.Forms Modularity was the same - to allow module loading after the application is up and running, though currently it may not be implemented. |
I might interject here that if there is demand for a container, like Autofac or SimpleInjector that has an immutability issue, that perhaps we simply make note of the limitation allowing a number of applications built without modules or dynamically loading modules to operate successfully with developers using the container of their choice. However if someone is developing an application that requires this type of dynamic loading then they will simply need to use a different container. I just see a whole can of worms being opened up that perhaps Prism shouldn't be getting into when we try making a container work in a way that goes against the way it was designed to work. |
I'm not a Prism user but I have seen different ways different app types handle plugins. We try to describe some ideas - from lazy loading to assembly scanning to lambda registrations that get evaluated only at resolve in autofac/Autofac#811. That said, I don't know that I can help too much in solving a problem where the issue is "we need to drop new assemblies into the running application and have them loaded without restart," which is what that article earlier seems to allude to. That's actually sort of a bad thing from a DI perspective since one dependency may, for example, ask to enumerate the list of available plugins and then hold a reference to that list; followed by a new plugin getting loaded, causing the list to be stale/incorrect/inconsistent with the rest of the app. That's actually one of the primary reasons we're trying to get to the immutable container - that app inconsistency causes really challenging issues that are hard to troubleshoot. Again, you can read all about that in autofac/Autofac#811 if you're interested. The closest thing to the "load plugins from assemblies" that Autofac has is the dynamic assembly scanning feature which may help if you can at least do the location of the plugins at app startup rather than during runtime. You also may be interested in implementing a custom |
@tillig - thank you for continuing to monitor and participate in this issue discussion. Here is what I understand:
So that is where I am at. I believe Dan S. that there could be a situation where some features of Prism are supported with Unity, but not with Autofac. The "load modules from a directory" functionality might be one of those - it is not a feature that I anticipate using. (Since it is not possible to have a tone of voice in discussions like this, I hope it is clear that I am not angry or trying to shut down any discussion. I am just trying to acknowledge and re-state the reality that has led to the situation discussed in this issue.) |
We are pretty well into the point where we're planning on removing I would agree that the code needs to be adapted. I don't have much insight as to exactly how that should happen, but a good starting point for folks working on that is to check out the discussion issue and actually read through it. It's long, but it has a lot of information as well as other peoples' use cases that we've tried to address as they come up. |
And still looking for feedback on the [proposed] changes to module handling in Prism.Autofac.Forms... |
So basically it looks like Autofac for WPF will be dropped when Update support is dropped. Prism for WPF relies heavily on Modules which can be loaded in any number of ways. If Autofac cannot work with modules that are lazy loaded, then I have no choice but to drop support for Autofac on Prism for WPF. Now, this just means that I will not be updating the Prism.Autofac version to the version that drops the Update feature. |
@ellisnet I agree with @dansiegel - if Autofac wants to go immutable, we should not try to make it support things that it can't. Which means that Modularity functionality should be broken into two parts: modules that are available during application startup, and another part supporting modules which are not immediately available (e.g. downloaded) and modules loaded on-demand. Autofac may be used only for the first part. Modularity in Prism.Forms is relatively small and probably this can be done. As for WPF... I doubt anybody will ever want to do that. |
I won't try to convince you one way or the other (though it'd be cool if Autofac support continued) but from an IoC container perspective, immutability is actually pretty common and getting more common - because of that whole "inconsistent app behavior" thing. SimpleInjector was already mentioned in this thread, but LightInject and the new Microsoft.Extensions.DependencyInjection container are also immutable. Containers that appear immutable are sometimes trying behind-the-scenes to rebuild the whole container for you... which actually can have even more challenging side effects, like when singletons have to be disposed and rebuilt. My point is that it may be interesting to take a look at the notion of modules as they stand and consider a different design around them and how they interact with IoC containers. For example, maybe module loading doesn't happen inside the container and instead modules only support property injection or a manual constructor injection. That's kind of how ASP.NET Core middleware works - it appears stuff is going through DI, but in reality there's some reflection going on to locate the parameter types that need to be resolved and those are manually poked in by the integration. Again, if you can't/won't/don't want to get it to work, I totally understand. Just figured there may be a way to change the problem being solved and, at the same time, open up wider support for other IoC containers. |
For anyone still following this issue, I believe that I have completed my work. Based on important points made by @dvorn above, and after a discussion with @brianlagunas, I only worked on the implementation of Autofac for Prism for Xamarin.Forms (Prism.Autofac.Forms) - and not on anything for WPF or for UWP (without Xamarin.Forms). I submitted Pull Request #1017 this afternoon. @perssondan You had offered to test the updated implementation a few weeks ago, so if you are still willing to do that, it would be helpful. In the course of working on changes and testing them on iOS, Android and UWP - I spent quite a bit of time on several Prism.Forms app samples (from PrismLibrary/Prism-Samples-Forms and also from @dansiegel) - which you can view here. Please note that all of the Autofac samples are duplicated in three different folders - see the readme.md file for more information about the differences between the samples in different Autofac folders. To take a Prism.Autofac.Forms 6.3.0 application and convert it for use with the updated library, you would want to do the following in your Prism.Forms application code:
So this code:
Becomes simply:
And type registration code like the example above MUST be placed in the overridden
Please take a look at the samples mentioned above - they demonstrate a lot of what can be done with Prism.Forms and I have tested them all quite thoroughly. One additional note: At this time, it is easiest to continue using Autofac 3.5.2 with your Prism.Autofac.Forms-consuming app. Autofac 4.x has been converted to .NetStandard - so pulling in that version from NuGet pulls all of the .NetStandard package references into your project. It is my understanding that Prism will be updating to .NetStandard in the coming months, so at that time it may make more sense to move to Autofac 4.x. However, the updated version of Prism.Autofac.Forms should work fine with Autofax 3.5.2 and any version 4.x. But let me know if there are problems or questions. |
I suggest to simplify the approach taken by @ellisnet and make it compatible with other immutable DI container technologies. Namely,
public MyModule(ContainerBuilder builder, Container container) Here the exact types ContainerBuilder and Container are specific to particular DI container technology. The module should perform all type registrations in constructor using the supplied builder. The module may save the container instance for later use in the Initialize() method. The container should not be used to resolve anything in the constructor. For SimpleInjector the builder and the container will be the same and the constructor may have a single argument.
Note that this approach is based on convention rather than enforced by explicit interfaces, but this is true also for the approach of @ellisnet. Traditionally instances of modules were resolved via the container. Here the reflection is used. But the same is true for the approach of @ellisnet. |
@ellisnet You created AutofacContainer which combines Container and ContainerBuilder. But did you considered the possibility of making ContainerBuilder a property in PrismApplication so that ConfigureContainer() works directly with ContainerBuilder? The implementation will be streamlined and simplified. Of course this will be a breaking change. But maybe the fans of Autofac container will be happy to pay the cost of upgrading to immutable container? |
Hello @dvorn, thank you for your comments. Yes. My first attempt at using an immutable Autofac container was to just add a ContainerBuilder property to the PrismApplication class and expose it for type registration. This approach was suggested by @tillig in one of the discussions that he linked to at the top of this issue. Over the course of dealing with these problems - and some others that are not coming to mind right now - I came to the conclusion that for Prism.Autofac.Forms' purposes, having one AutofacContainer object which handles both registration and resolution duties was a better design, easier for users of the library to consume, and more in keeping with how Prism.Forms works (e.g. with Unity). |
@ellisnet But you may change AutofacExtensions to make it work with ContainerBuilder instead of Container. Note that Prism.Autofac.Forms is not supported by @brianlagunas, it is supported by Autofac community. As long as Prism.Forms is not changed, nobody cares what happens in Prism.Autofac.Forms but Autofac community. ContainerBuilder is for registering types and Container is for resolving types. Attempting to fooling the user to believe otherwise is a bad approach IMO. If you use things for what they are designed for, you get a simple straightforward implementation. But it is up to you and other users of Autofac with Prism.Forms. I personally do not use them, those were just my thoughts... |
@dvorn Thank you again. I think you have made points that are worthy of discussion; and maybe people who have used Autofac with Prism.Forms - or plan to use it - will have more thoughts on these points. |
@ellisnet Has there been any progress on making Autofac work with Prism.Forms? |
@stevejbrother we haven't had time to focus on this yet. We are still trying to wrap up our .NET Standard and automated build processes, as well as focus on the navigation bugs in XF. |
I would rather not see support for Autofac dropped, and I think it is possible to come up with a solution. I've only ever used Prism with WPF, but I'm sure something similar can be done for other platforms. The "Update" functionality is still present in Autofac's BeginLifetimeScope, which has an overload that takes an These new lifetime scopes need to live for the duration of the application. The Prism Autofac integration should provide a class/interface to help with the configurations above. @dvorn, @ellisnet, @brianlagunas thoughts? |
I went ahead and implemented a suggestion for how it can be solved, here. There are breaking changes (for example, There is a new interface
Modules can get it injected, register the types it needs by calling This change allows on-demand modules and will work even after |
This is being addressed by PR #1288. Essentially, modules are not supported with Autofac. only mutable containers will support modules. |
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. |
The Autofac
ContainerBuilder.Update
method has been marked obsolete in recent versions. There is a discussion issue explaining why and various workarounds for that.It looks like the Autofac integration is using Update. You'll need to switch it to work more like the SimpleInjector app type since SimpleInjector also doesn't allow changing the container post-build.
The text was updated successfully, but these errors were encountered: