Call Dispose in RegionNavigationService if View/View Model implements IDisposable

[Jalalx]

I have checked source code of RegionNavigationService.cs, and It seems there is no such mechanism. I think it would be a good idea to dispose view/view model when IRegionMemberLifetime.KeepAlive is false. This is a good practice to put all clean ups in Dispose method such as unregistering static CompositeCommands or disposing DbContext instance in view model (yes, I'm not a fan of Repository pattern :P).

[brianlagunas]

Thanks for the suggestion. We will have to research the best way to handle that scenario as it is a common need. We may provide a mechanism which allows you to hook into the process and clean up however you like which won't require the use of IDispose. This can be achieve with a simple RegionBehavior if you need this functionality now.

[Jalalx]

Thank you for suggesting RegionBehavior. Here is my DisposableRegionBehavior:

    public class DisposableRegionBehavior : RegionBehavior
    {
        public const string BehaviorKey = "DisposableRegion";

        protected override void OnAttach()
        {
            this.Region.Views.CollectionChanged += OnActiveViewsChanged;
        }

        private void OnActiveViewsChanged(object sender, NotifyCollectionChangedEventArgs e)
        {
            if (e.Action == NotifyCollectionChangedAction.Remove)
            {
                DisposeViewsOrViewModels(e.OldItems);
            }
        }

        private static void DisposeViewsOrViewModels(IList views)
        {
            foreach (var view in views)
            {
                var frameworkElement = view as FrameworkElement;
                if (frameworkElement != null)
                {
                    var disposableDataContext = frameworkElement.DataContext as IDisposable;
                    if (disposableDataContext != null)
                        disposableDataContext.Dispose();

                    if (view is IDisposable)
                    {
                        var disposableView = view as IDisposable;
                        disposableView.Dispose();
                    }
                }
            }
        }
    }

[briannoyes]

I think the RegionBehavior is probably the best way with current code base because I don't think the Region/RegionManager currently track whether a contained view was provided as an instance (via Region.Add or RegionManager.AddToRegion or RegisterViewWtihRegion (factory method version) or whether RegionManager created it (through RequestNavigate or RegisterViewWithRegion - type variant). If we added tracking of that, then it seems it would make sense to check for IDisposable and call it if the View/ViewModel had IRegionMemberLifetime.KeepAlive=false.

Possibly a simpler and more straightforward way would be to add another property to IRegionMemberLifetime.DisposeOnDeactivate and do it based on that?

I do think this would be a very useful extension for us to add.

[Mike-E-angelo]

Yikes to a behavior-based boolean, IMO. It is simpler/straight-forward but leads to booleanitis. If we can get away with checking for IDisposable when !ShouldKeepAlive(), that would be my preference/vote if it is possible.

[Jalalx]

@Michael-DST Do you mean something like this? https://compositewpf.codeplex.com/SourceControl/network/forks/jalalx/LightPrism/changeset/aa8902f73326aa3fde7b0375cefe6ce14bda3a60

[Mike-E-angelo]

Yes! :)

[Jalalx]

I forget to call Dispose() for view model, so here is final:
https://compositewpf.codeplex.com/SourceControl/network/forks/jalalx/LightPrism/changeset/5ef43a2f6ced9ccb73d37afe02d7407842d4de3e

[briannoyes]

The problem as I mentioned is that if the view was provided to the region by the module code, then the module may still hold a reference to the view itself and is therefore the lifetime owner of that object and it would be inappropriate for the region manager to dispose it.

[Mike-E-angelo]

Ahhhh... Team Brian has better perspective on this matter, of course. :) I just hate seeing booleans used where something stronger can be utilized, but that's me. If that's the best answer here, then that's the best answer. 👍

[brianlagunas]

I'm sure once we dig into this feature and get more information about the internals, we will find a solution that will work for everyone.

[brianlagunas]

We have decided that using a custom region behavior is the solution for this scenario.

[lock]

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.