Suggestion: Loosen the coupling of the Shell with the Bootstrapper.

[karlfl]

I'm not sure the full reason behind the tight coupling of the Shell with the Bootstrapper, so I offer this as a suggestion.

I've found it useful at times to create the shell in it's own module rather than in the bootstrapper and then use module dependencies to ensure it's loaded first. This allows a more robust shell implementation without cluttering the main application startup project, especially when there are many supporting objects. Here's how I got it to work without using the bootstrapper to create the shell. (note this is a WPF application, but it may apply to other types)

First return null from the CreateShell method (it's abstract so I'm forced to implement this)

        protected override DependencyObject CreateShell()
        {
            return null;
        }

Then in the shell module init create the shell instance, and register the regions (required since the "return null" above prevents the bootstrapper from making these two calls)...

        public void Initialize()
        {
            Shell shell = new Shell();

            RegionManager.SetRegionManager(shell, _container.Resolve<IRegionManager>());
            RegionManager.UpdateRegions();

            Application.Current.MainWindow = (Window)shell;
            Application.Current.MainWindow.Show();
        }

Finally ensure that any module depending on the shell has the shell module in it's DependsOn. This is done to ensure that the shell module is loaded first, and the region manager is properly initialized.

This works great, however with some possibly unneeded overhead (i.e. the 'dummy' CreateShell method and the forced DependsOn for modules requiring the shell). It would be nice if there was a way of telling the bootstrapper that instead of a "Shell" I want to use a "Startup Module". The bootstrapper then would manage this differently, for example ensuring that the Startup Modue is loaded first.

Thoughts?

[Mike-E-angelo]

I believe you will find support behind this. I know I have discussed this very problem of the Bootstrapper rigidity with @JohnTasler and he might also provide some feedback here.

Additionally, in my fork you will find a Setup object that is designed to replace the Bootstrapper's tight-coupling (in a Xaml-described-friendly way) along with discrete SetupCommands (by way of implementing the ICommand interface) that perform the different activities of the bootstrapper. This gives you a fine-grained control over the initialization process and allows you to do it in a Xaml/designer-friendly way.

To your suggestion, here is a command that sets up the shell in Unity and here is an example of that command described in Xaml.

[brianlagunas]

Honestly this is not something that I think we will consider. The initialization of a Prism app now is very simple and extensible. I would probably challenge you on your approach, since you are now creating a tight coupling between the Prism app, and your Shell module. So I am not really seeing the advantage. If you just wanted to have the container of your regions to be dynamic, or in it's own module, then I would just have a single regioned Shell in the Prism app, and then inject your container (the view to act as your region container) using your Shell module to act as the region targets. This way the Prism app is really just a dumb container.

[karlfl]

If it's ok, i'd like to provide a bit more discussion on this topic. I guess the fundamental question(s) I'm asking is; Why is the bootstrapper forcing the developer to define a Shell? Isn't this creating a tight coupling when it's not necessary? As shown above, it can be worked around with a 'return null' method, so your right it is extensible, but, why does it force the method definition/override? If it was truly required then the bootstrapper's run method wouldn't allow for Shell == null. Digging into the code we see there are only two lines of code in the bootstrapper that depend on the shell reference:

    RegionManager.SetRegionManager(shell, _container.Resolve<IRegionManager>());
    RegionManager.UpdateRegions();

So RegionManager requires shell, but why does the bootstrapper require the reference?

You do bring up an interesting point, that I had not fully considered, when you mention the simple single-region shell approach. That approach will work in many cases. The only one I can think of where it gets a little more complex is when the window chrome needs to be different or changed based on other variables (i.e. admin users chrome is different than the standard user chrome).

So, I'm not sure of the exact implementation, but I think removing the bootstrapper dependency on shell would make the framework easier to extend. A good start might be replacing the abstract definition of CreateShell() with a virtual method which, by default, returns null. That would not break the model but allow simpler extensibility.

[karlfl]

Not sure if this is the right way to do it, but I changed the title of issue to simplify the suggestion. If needed we can re-introduce the 'startup module' in a separate issue/suggestion.

[briannoyes]

The bootstrapper's job is to get the Prism "environment" initialized. That means setting up the container, getting all the services that are part of Prism registered with the container and initialized before the app and modules start loading, and then kick off those two processes as well.

The way RegionManager works it needs access to the visual tree so it can detect the regions that are there during the initial parsing process or those dynamically added for the RegisterViewWithRegion and navigation functionality. So the RegionManager cannot be treated as ready to go until it has access to the shell, which is why shell creation and initialization are part of the bootstrapper workflow.

I'm not clear on what coupling you are really objecting to there though. The only coupling that is there is that the bootstrapper is aware of a UIElement that you treat as your shell. If you don't like having that construction happen in the bootstrapper itself, just make your bootstrapper have a constructor parameter and pass it in when you construct it in the App class. Then the only thing the bootstrapper has to know is that it was passed a UI element, it doesn't have to know the type, how it was constructed, etc. But it needs that UI element for the region functionality to work right, end of story. And in my experience many Shell's need to be constructed through the container so they can do DI themselves (even though most of it should be going on in the ViewModel), thus they need to wait for the Container to be ready, which is part of the workflow of the bootstrapper, thus Create/InitializeShell are there.

If you are not going to use region functionality, you can just return null from the CreateShell method. But if you want regions, you have to pay the tax of making the bootstrapper aware of the shell. Its been that way since the first version of Prism in 2008 and this is the first objection I've heard about that structure, and I can't think of a way to do that would not be coupled in some other way.

[karlfl]

Thanks for your thoughts and feedback guys. I enjoy the discussion. In fact this discussion has allowed me to refine my thinking a bit and force me to ask myself, 'what am I really asking for'. I'm a little surprised that I'm the first with this thought or challenge. But it is what it is.

I understand the purpose of the bootstrapper and I'm not suggesting that we make major changes to it or re-design its purpose. I agree that UIElement(s) (including a Shell) are important in application startup. I understand the need to register the regions on the shell using the RegionManager (the 'tax' you refer to, although Prism makes that a pretty small, two line, tax :-) ). I'm just not convinced that the average startup UI is as simple as a single component stored as a member on the Bootstrapper. Especially when you introduce things like a splash screen or custom window chrome. However, I realize that there may be some simple applications that do only need a single UIElement or Shell in their startup, so I was looking more for increased flexibility than re-design. Personally, I like keeping my Prism startup project (aka shell project) fairly small and let the individual modules do the heavy lifting, including loading a splash screen, caching data, and eventually bringing up the main shell. If I get some time I'll throw up a sample app on Github to show how I do it.

In regards to this specific suggestion. I already have a work around for my needs and I know it's possible to ignore the Shell in the bootstrapper and implement my own more robust shell module. I have done that using several versions of Prism, even in a way that still uses regions via the RegionManager with only a few lines of code (see my first comment). I was not trying to suggest the removal of "CreateShell" or the UIElement reference to Shell in the bootstrapper. Not at all. I was only suggesting a change to allow developers to more easily ignore the bootstrapper's shell implementation if they so choose. If I was required to boil this suggestion down to a single line it would be this statement, which I eluded to at the end of my last comment.

Replace the abstract definition of CreateShell() with a virtual method which, 
    by default, returns null.

So that was all. Prism still works great as it is and I'll continue to support it and use it on current and future applications I develop and I am delighted that it's now community developed.

[briannoyes]

Ah, ok. Appears I was reading too much into your request. That would be an easy and harmless change and I can remember at least one app I worked on where I wished it was that way myself. I'll discuss with BrianL to make sure I am not missing something, but I think we could slip that in to the next release.

[brianlagunas]

There are actually some other changes I would like to make to the Bootstrapper regarding the Shell, but would be braking changes. None the less, I would like your thoughts.

Since we have dropped support for Silverlight, there is no need for CreateShell to return a DependencyObject. Instead it can be a Window, since we know we are in WPF now. Another change would be to modify InitializeShell to call Application.Current.MainWindow.Show by default. So devs wouldn't have to worry about that method override at all. Unless they are doing something custom.

The only thing is, this would break current apps. I don't think it is a horrible break, as you are just changing the return type from DP to Window, and as along as the overridden InitializeShell didn't call base.InitializeShell, then there wouldn't be an issue there. I'm not sure it's worth it or not. Maybe not for the data type change, but possibly the change for InitializeShell would be worth it?

[briannoyes]

What do you gain by doing that. When designing APIs against objects with an inheritance hierarchy, I've always stuck to the principal of only depend on the minimum class in the hierarchy that support the API you are going to work with. So switching to Window, while it may make it more clear what kind of dependency object you are supposed to pass, introduces unnecessary coupling to a more derived type that the framework doesn't even care about. And it could block scenarios that you haven't anticipated.

I don't think there is any technical reason that someone couldn't treat a inner view within an app the "shell" of a Prism app - as in they are only doing Prism from that element down. Can't think of any great reason you might want to do that, but no good reason to block it either.

So my vote is NO.

[brianlagunas]

Okay, I can see not changing the DataType of the return value for CreateShell, but how about at least providing a default InitializeShell so that devs don't have to worry adding code just to show the Shell if it exists and it's a Window? Something like:

protected virtual void InitializeShell()
{
     if (Shell != null && Shell is Window)
     {
          Application.Current.MainWindow = (Window)Shell;
          Application.Current.MainWindow.Show();
     }
}

This change will only impact devs who called base.InitializeShell in their bootstrapper, and is easily fixed by removing that line of code in their override.

Thoughts?

[briannoyes]

Actually don't like that either for a couple reasons:

• People who have been with Prism since Prism 1 often don't even implement InitializeShell at all, they just construct and show their shell from CreateShell because that is all there was in Prism 1. It was introduced in Prism 2 for some obscure reason related to Silverlight that I forget, and also because there might be specialized scenarios where you need to separate construction and initialization. This would mean people doing that would have show called on their main window twice, which would be bad.
• You don't need to explicitly set Application.Current.MainWindow, it gets set automatically in WPF to the first Window that Show is called on. One could argue that it is better to have it explicitly coded like this, but others could argue that it is an unnecessary line of code so it shouldn't be there.

[brianlagunas]

Yes, I realize that this would break those who are either showing their shell in the CreateShell method, or that are calling base.InitializeShell in their overridden InitializeShell method call. The fix is easy for both of those scenarios, and would eliminate code for the majority of users than it would impact those that are showing the shell in the CreateShell method (which goes against recommendations anyways). The bottom line is calling Application.Current.MainWindow.Show is repetitive monkey code, that we can remove the need for, and still provide the flexibility needed for more advanced scenarios.

I guess we can always wait for more demand, since this hasn't been brought up before now. I just think we can make this part a little easier for our users, instead of keeping it the way it is simply for legacy support reasons.

[tstephansen]

This is just my opinion but I wouldn't like the bootstrapper to automatically call Application.Current.MainWindow.Show(). I use a separate bootstrapper that is identical to my application's bootstrapper with the exception of this method for unit testing and I don't want it to create a window. If I'm misguided here (this is probably not the best practice) please ignore my comment.

[brianlagunas]

Please review PR #184 to make sure this meets your needs.

[brianlagunas]

CreateShell is now virtual.

[tstephansen]

Works great. Thank you!

[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.