1.1.0 - Bug with view component model type checking for View

[dazinator]

I have just updated my project from 1.0.0 to 1.1.0.
This seems to have broken one of my view components.

I am now seeing something that makes no sense...

Basically, it doesn't matter what type of model my VC passes to the View() an exception is always raised stating that the model is of a specific type, which is incompatible for the View.

Here you can see that my View component is passing a null IUser into the View() as the model. This results in the exception stating that the model item being passed into the view is of type MenuViewModel which is incompatible. No matter what I pass it, it always says its a MenuViewModel and that's not compatible.

What on earth is going on.

Here is the Razor View where the VC is being invoked in case it helps - the invocation itself works absolutely fine.

And here is the View for the VC as you can see the model type is IUser

@model Reach.GCv3.Business.Contracts.IUser

@if (Model != null && Model.IsImpersonated)
{
    <li>
        <a class="label label-warning">Impersonating</a>
    </li>
}

A few special thing to note about this VC is that its invoked within another VC's view.

For example, the top level VC renders a menu, by looping through its model of menu items. If the item is also a VC, it invokes that VC.

<ul data-toggle="collapse" data-target="#navbar.in" class="nav navbar-nav navbar-right">

                @foreach (var menuItem in Model.MenuItems)
                {
                    if (menuItem.IsViewComponent())
                    {
                        <!-- ko stopBinding: true -->
                         @await Component.InvokeAsync(menuItem.ViewComponentName, new { menuItem })
                        <!-- /ko -->
                    }
                    else
                    {
                        <li><a href="@menuItem.Path">@menuItem.Name</a> </li>
                    }
                }

            </ul>

The other thing is that if I don't atttempt to pass a model to the View at all then it works:

public IViewComponentResult GetView(MenuItemViewModel menuItem)
        {
            try
            {
                 return View(); // this works
               //  return View(someModel as IUser); // this doesn't
            }
            catch (Exception e)
            {

                throw;
            }
           
        }
    }

Lastly at the point my VC is invoked - this looks a bit odd - I tried to dig around and this strikes me as suspicious: The ViewContext.ViewData has a Model matching the type reported in the exception:

Here is the full exception message:

The model item passed into the ViewDataDictionary is of type 'Gluon.Module.NavMenu.ViewModels.MenuViewModel', but this ViewDataDictionary instance requires a model item of type 'Reach.GCv3.Business.Contracts.IUser'.

Here is the stack trace:

   at Microsoft.AspNetCore.Mvc.ViewFeatures.ViewDataDictionary.EnsureCompatible(Object value)
   at Microsoft.AspNetCore.Mvc.ViewFeatures.ViewDataDictionary..ctor(ViewDataDictionary source, Object model, Type declaredModelType)
   at Microsoft.AspNetCore.Mvc.ViewFeatures.ViewDataDictionary`1..ctor(ViewDataDictionary source)
   at Microsoft.AspNetCore.Mvc.ViewComponent.View[TModel](String viewName, TModel model)
   at Reach.GCv3.Mvc.Module.Authentication.ViewComponents.ImpersonationNavMenuItemViewCompoent.GetView(MenuItemViewModel menuItem) in D:\Repos\gc3-website\src\Reach.GCv3.Mvc.Module.Authentication\ViewComponents\ImpersonationNavMenuItemViewCompoent.cs:line 35

[dougbu]

The difference between View() and View(someModel as IUser) is the first creates a ViewDataDictionary<object> while the second creates a ViewDataDictionary<IUser>. The first is able to hold any model from the outer context. The second also reuses the outer model when someModel is null or not an IUser but that dictionary blows up if the outer model is another type.

[dazinator]

This sounds like the problem:

The second also reuses the outer model when someModel is null

This must be the breaking change as this code worked in 1.0.0. Also I can't see how that change in logic makes sense... If you pass a null model to a View, you would expect that model instance to make it to that View, is the one you passed.. you wouldn't expect MVC to ignore it and go grab some irrelevant instance from somewhere (what you referred to as OuterModel)

[frankabbruzzese]

@dougbu , @dazinator ,
Is what I said also in #5561. This new behavior is counter-intuitive, and definetively not useful, since it is very unlikely a developer re-use the Model of the View that calls the ViewComponent. A similar behavior violate ViewComponent encapsulation (as compared to the various JavaScript frameworks "components"). It is very likely each ViewModel uses its own specific Models.

Re-using the calling View ViewModel might be useful in partial views since they do not have any model processing subpart, not for ViewComponents that have their controllers to process models.

As I predicted other people are falling in the same mistake. I repeat here my conclusion in the other thread:

Maybe, a better approach would be just passing Caller ModelState and ViewData.TemplateInfo.HtmlPrefix to the ViewComponent instead of passing the whole ViewDataDictionary. This is what is needed to make errors are dispatched properly within the ViewComponent View and to make input fields are named properly for the ModelBinder to match them. This data are handled automatically by the system so there is no reason to bother the developer by forcing him to set them manually, but model and ModelMetadata processing is just what the ViewComponent code is supposed to process, its focus. Data to be processed by developers code should be passed as arguments in the ViewComponent call without assuming the default behavior of using the caller View Model...Otherwise what is the reason for using a ViewComponent if one has no model processing? A partial view would be more appropriate

[Eilon]

@ajaybhargavb I'm assigning this to you because it seems related to #5561.

[ajaybhargavb]

#5561 (comment)

[ajaybhargavb]

Closing as this is a dup of #5561