ReactiveUI icon indicating copy to clipboard operation
ReactiveUI copied to clipboard

[FEATURE][WPF] Optimization of the ViewModelViewHost behavior

Open tongbong opened this issue 2 years ago • 1 comments

First of all, thank you for providing this super nice library.

Is your feature request related to a problem? Please describe.

Actual behavior : a view is instantiated whenever the view model property changes

Problem : when using it for navigating inside the app, it causes high memory consumption + lots of garbage collection activity + slow down

Describe the solution you'd like

Proposal : make it behave like the WPF content presenter

If the new view model is of the same type than previous, then there is no need to create a new view. The actual view can be reused.

Describe suggestions on how to achieve the feature

    private IViewFor? _view;
    
    private void ResolveViewForViewModel(object? viewModel, string? contract)
    {
        if (viewModel is null)
        {
            Content = DefaultContent;
            return;
        }

        bool mustCreateView = _view == null 
                              || ViewModel == null 
                              || _view.ViewModel != null && _view.ViewModel.GetType() != viewModel.GetType();

        if (mustCreateView)
        {
            var viewLocator = ViewLocator ?? ReactiveUI.ViewLocator.Current;
            var viewInstance = viewLocator.ResolveView(viewModel, contract) ?? viewLocator.ResolveView(viewModel);

            if (viewInstance is null)
            {
                Content = DefaultContent;
                this.Log().Warn($"The {nameof(ViewModelViewHost)} could not find a valid view for the view model of type {viewModel.GetType()} and value {viewModel}.");
                return;
            }

            _view = viewInstance;
        }

        if (_view != null)
        {
            _view.ViewModel = viewModel;
            Content = _view;
        }
    }

tongbong avatar Apr 27 '22 11:04 tongbong

Hello, I have spent some time wrapping my head around that, but there is issue that you can have some kind of state in your view, that is not explicitly tied to your ViewModel and which you expect to be reset. That would be an issue and breaking change.

And I cannot think of way how to tackle that issue.

I would personally say, that the current state is the most flexible.

You can altought register your custom view locator, which you have provided in the description if that fits your scenario: https://www.reactiveui.net/docs/handbook/view-location/#overriding-viewlocator

tomasfil avatar Jul 19 '22 07:07 tomasfil

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

github-actions[bot] avatar Oct 20 '22 00:10 github-actions[bot]