Different working in ViewComponents in v.1.1.0

[simon25608]

Hello!

After upgrading from version 1.0.1 to 1.1.0 I have detected that the behavior of the viewcomponets has changed.
In the previous version it had the following code:
<div id="componente-documentos" class="formulario-seccion"> @await Component.InvokeAsync("Archivos", new { vista = Archivos.Listado, archivos = Model.Archivos, titulo = "Documentos adjuntos" }) </div>

Where archivos = Model.Archivos is a List. And in the component template I receive the model:
@model IList<COFC.ViewModels.ArchivoViewModel>

In this same view, I call the component again, with different parameters and the one who calls another different view
@await Component.InvokeAsync("Archivos", new { vista = Archivos.Nuevo })

that receives the model
model COFC.ViewModels.ArchivoViewModel

And this is the component code:

` public class Archivos : ViewComponent
{

    public const string Detalle = "Detalle";
    public const string Listado = "Listado";
    public const string Nuevo = "Nuevo";
    public const string Modificar = "Modificar";

    public IViewComponentResult Invoke()
    {
        object vista;
        object archivos;
        object titulo;
        object archivo;
        if (ViewComponentContext.Arguments.TryGetValue("vista", out vista) && ViewComponentContext.Arguments.TryGetValue("archivos", out archivos) && ViewComponentContext.Arguments.TryGetValue("titulo", out titulo))
        {
            ViewBag.TituloArchivos = (string)titulo;
            return View((string)vista, (ICollection<ArchivoViewModel>)archivos);
        }
        else if (ViewComponentContext.Arguments.TryGetValue("vista", out vista) && ViewComponentContext.Arguments.TryGetValue("archivos", out archivos))
        {
            return View((string)vista, (ICollection<ArchivoViewModel>)archivos);
        }
        else if (ViewComponentContext.Arguments.TryGetValue("vista", out vista) && ViewComponentContext.Arguments.TryGetValue("archivo", out archivo))
        {
            return View((string)vista, (ArchivoViewModel)archivo);
        }
        else if (ViewComponentContext.Arguments.TryGetValue("vista", out vista))
        {
            return View((string)vista);
        }
        else
        {
            throw new Exception("Los parámetros enviados no se corresponden con los definidos");
        }

    }
}`

The second call to the viewcomponent returns the view
else if (ViewComponentContext.Arguments.TryGetValue("vista", out vista)) { return View((string)vista); }

And this is where the different behavior occurs between version 1.0.1 and 1.1.0. The previous version was perfectly executed, creating an empty object of the viewmodel used in the view (in this case ArchivoViewModel), but in the new version it tries to use the viewmodel of the first call (in this case List ) causing the exception:

InvalidOperationException: The model item passed into the ViewDataDictionary

In this case, this is solved with
else if (ViewComponentContext.Arguments.TryGetValue("vista", out vista)) { return View((string)vista, new ArchivoViewModel()); }

But there are other cases in which it is more difficult to solve, is this change of normal operation? I find nothing about it.

I hope I explained well

Thanks.

[frankabbruzzese]

Can you please post the ViewComponent View, and the whole error message (you posted just the initial part). It appears to be a type mismatch with the VieDataDictionary generic type.

[simon25608]

The property passed in the call

public IList<ArchivoViewModel> Archivos { get { return _archivos; } set { _archivos = value; } }

Call:
<div id="componente-documentos" class="formulario-seccion"> @await Component.InvokeAsync("Archivos", new { vista = Archivos.Listado, archivos = Model.Archivos, titulo = "Documentos adjuntos" }) </div>
View:

Second View:

Exception:

InvalidOperationException: The model item passed into the ViewDataDictionary is of type 'System.Collections.Generic.List`1[COFC.ViewModels.ArchivoViewModel]', but this ViewDataDictionary instance requires a model item of type 'COFC.ViewModels.ArchivoViewModel'.
Microsoft.AspNetCore.Mvc.ViewFeatures.ViewDataDictionary.EnsureCompatible(object value)
Microsoft.AspNetCore.Mvc.ViewFeatures.ViewDataDictionary..ctor(ViewDataDictionary source, object model, Type declaredModelType)
lambda_method(Closure , ViewDataDictionary )
Microsoft.AspNetCore.Mvc.Razor.RazorPageActivator.Activate(IRazorPage page, ViewContext context)
Microsoft.AspNetCore.Mvc.Razor.RazorView.RenderPageCoreAsync(IRazorPage page, ViewContext context)
Microsoft.AspNetCore.Mvc.Razor.RazorView+d__14.MoveNext()
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
Microsoft.AspNetCore.Mvc.Razor.RazorView+d__13.MoveNext()
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
Microsoft.AspNetCore.Mvc.ViewComponents.ViewViewComponentResult+d__20.MoveNext()
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
Microsoft.AspNetCore.Mvc.ViewComponents.DefaultViewComponentInvoker+d__5.MoveNext()
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
Microsoft.AspNetCore.Mvc.ViewComponents.DefaultViewComponentHelper+d__12.MoveNext()
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
System.Runtime.CompilerServices.TaskAwaiter.GetResult()
AspNetCore._Views_Shared_Components_Archivos_Listado_cshtml+d__37.MoveNext() in Listado.cshtml

[frankabbruzzese]

It appears that, somehow, the View selected is not the right one for tha actual ViewModel:
you pass the IList model to the View designed for the single model...Exception messages say this.

Verify in the debugger wich if is followed before the exception, I can't belive this exception is thrown
in the branch with a null ViewModel (return View((string)vista)).

[simon25608]

If the exception says that the last model is of type IList and the second view expects a ArchivoViewModel.
As I said, the behavior in different to previous versions, so that the exception does not occur it is necessary to do return View((string)vista, new ArchivoViewModel());

Seeing other cases a little more complex than I have, what I see is that in previous versions not to pass the model in the call to the component created an empty object of the model that receives the view.
Now however the model of the first call to the component is passed to it.
Notice that in the cases that I expose, from a component is called another component

[frankabbruzzese]

You say, basically, that when the ViewModel is null, the ViewComponent try to infer its Type somehow, an this causes the problem. If you pass the actual model it has the actual run-time type an everything works properly.

Very strange, I'll try to reproduce.

[iscifoni]

@simon25608 in the second view try to use

@Html.Partial("Genericas/Formularios/boton", new ViewDataDictionary<COFC.ViewModels.ArchivoViewModel>(ViewData), .......

instead of

@Html.Partial("Genericas/Formularios/boton", new ViewDataDictionary(ViewData)

[simon25608]

@frankabbruzzese

Correct. If you want the files to play it, let me know.

[simon25608]

@iscifoni

The problem is with the view component, not with partial views, that is working correctly.

I do not understand the relationship: S

[frankabbruzzese]

@simon25608
What @iscifoni says is correct, When you call new ViewDataDictionary(ViewData) a ViewDataDictionary is created with the same type of the original ViewData. Often, this is the cause of View type mismatch bugs.

It is hard to belive that Views try to infer somehow the type of a null object, if you dont provide explicitely this information somehow.

[simon25608]

@frankabbruzzese

Yes, I know and I think so because I am interested in being created with the same type of the original.

But that does not have any relation with the described error, is more if I eliminate all the code of the second sight simply leaving
@model COFC.ViewModels.ArchivoViewModel @using COFC.ViewComponents

the error is the same, because the view no longer gets executed due to receiving an erroneous model.
The solution I found is:

return View((string)vista, new ArchivoViewModel());

or

@await Component.InvokeAsync("Archivos", new { vista = Archivos.Nuevo, archivo = new ArchivoViewModel() })

but this is different from other versions

[iscifoni]

@simon25608 I can't reproduce the error , please try to use this in your view component

return View<COFC.ViewModels.ArchivoViewModel>((string)vista, null)

instead of

return View((string)vista)

or set

ViewData.Model = null; return View((string)vista);

before returning view,

[simon25608]

@iscifoni

This return View<COFC.ViewModels.ArchivoViewModel>((string)vista, null) cause a follow exception:
The model item passed into the ViewDataDictionary is of type 'System.Collections.Generic.List`1[COFC.ViewModels.ArchivoViewModel]', but this ViewDataDictionary instance requires a model item of type 'COFC.ViewModels.ArchivoViewModel'.

and ViewData.Model = null; return View((string)vista); works correctly

[iscifoni]

@Eilon

(https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewComponent.cs#L271)

I am not sure because I can't reproduce this error but I think that on the second call to the viewcomponent the ViewData.Model is already set with the value of the previous call.

The second call doesn't have a model so ViewData.Model is not updated . When the second view is rendered, the binded model is the model of the previous view.

This is a bug ?

[simon25608]

@iscifoni

I am not sure because I can't reproduce this error but I think that on the second call to the viewcomponent the ViewData.Model is already set with the value of the previous call.

I think that's exactly what happens.

[frankabbruzzese]

@iscifoni ,
That code is "pretty standard" and you find it also in controllers. If the model != null extract type information from model otherwise use ViewComponent or controller ViewData. In fact developer may provide model information by attaching a ModelExplorer to the ViewDataDictionary.

However each ViewComponent call should have a new fresh copy of its ViewDataDictionary. If that is the problem, there is some bug in the caching logics...that might have several other consequences. For instance ModelState remains dirty ...and so on.

[frankabbruzzese]

@simon25608 ,
Can you please prepare a minimal project without nested calls, but with just 2 Views and 2 Viemodels that may be used bya the ViewComponent + some logi of which View to select (say with a link)...but really the simplest stuff that shows the problem, so we may be sure the broblem is not elsewhere.

Just to verify if the information on the type is remembered from previous calls when model is null.

[frankabbruzzese]

@simon25608 ,
If you send me your solution + detailed instructions to reproduce, I'll try to figure out what the problem is during the week end.
Frankly, speaking, I am not convinced there is a bug. Also a bug in Component caching cant cause that behavior, since component ViewData is not used as it is in the View but it is copied. This means also if ViewComponent ViewData is not properly cleared, nothing happens in you software since you dont modify it: it is just copied into a View.

Please upload solution in Google drive and send me the link through the contact form of my blog:

[simon25608]

@frankabbruzzese

https://github.com/simon25608/TestViewComponent

When the code is uploaded, when the project is started directly and the exception is released, if we modify the project.json to the previous version, the application executes correctly.

[iscifoni]

@frankabbruzzese @simon25608

The listado.cshtml view is rendered by Archivos (this is a ComponentView) but it contains a recoursive call to Archivos (the same ComponentView) ...
This could be the reason why on the second call ViewData.Model contains previous value.

[simon25608]

@iscifoni

Yes, it's what you said yesterday

[frankabbruzzese]

@iscifoni
O yes understood! When model is not null it is attached to the ViewComponent ViewData, not directly to the View ViewData, so ViewComponent ViewData is actually modified!

Since, ViewComponents are cached (at least during the same View processing or during the same request), and before re-using them jsust a ViewData.Clear() or similar "light operation" is performed...that doesnt clears the ModelExplorer automatically created when the not null ViewModel was inserted, so also if the model is reset to null the ModelExplorer contains wrong type information.

Fix should be either to add Model directly to newly created ViewData or creating a new ViewData when ViewComponent is re-used.

Anyway, one may verify what happens by inspecting ViewComponent ViewData just before the error.
Verifying 1) What is contained in its ModelExplores; 2) Verifying what is contained in its Model.

[Eilon]

@ajaybhargavb can you help investigate?

[frankabbruzzese]

I found exactly what the problem is. The problem is not due to recursion, or caching but to a subtle conceptual problem that prevents ViewComponents from passing a null ViewModel to their Views.
Not clear if the behavior I am going to describe is "by design", but if not the bug is severe.

I removed the model also from the first call ie when the ViewComponent is called the first time, and the error occurs also in this case. This means the problem always occur with null models.

By inspecting ViewComponent ViewDataDictionary in debugger I verified what follows:

ViewComponent initial ViewDataDictionary is taken from the calling View. In other terms it is either the same as the View ViewDataDictionary or a clone of it. This makes sense to pass main View error infos, etc. to the ViewComponent.

Now if ViewComponent passes a not null model to the View, the not null model is attached to its ViewDataDictionary, thus causing all Model Metadata to be recomputed to match the new model, so in this case everything works properly.

However, if the model is null, the ViewComponent ViewDataDictionary remains untouched, and is used to create its View ViedDataDictionary. This, in turn means, that the ViewComponent View receives a ViewDataDictionary cloned from the View that called the ViewComponent.

In other terms when a ViewComponent passes a null ViewModel to its View its View must have the same ViewModel as the View calling the ViewComponent.

Not sure this is bug, though, since also partial views when called with @Html.Partial with no new model use the same model metadata of the calling View. So this behavior might be by design.

However, for sure, in the case of ViewComponent that behaves much more like controllers, this behavior is counter-intuitive and bug-prone.

[dougbu]

@frankabbruzzese calling View<TModel>(model: null) in a view component means "create a new ViewDataDictionary<TModel> with the existing model". A similar call in a controller means "create a new (non-generic) ViewDataDictionary with the existing model". In either case, set ViewData.Model = null before calling View() to remove an existing Model.

[frankabbruzzese]

@dougbu ,
"A similar call in a controller means "create a new (non-generic) ViewDataDictionary with the existing model". This way of looking at what happens, is not very convincing for controllers, since when controllers are invoked their ViewData.Model are always null! Also when an action method receives a model as its unique parameter. ViewDataDictionary model is usually null and makes absolutely no sense "hiding" the model returned ny an action method by adding it l to ViewDataDictionary.Model in the middle of the code instead of passing it to View(...., model) at the end of the code (which result in more readible and maintenable code). Probably 99,99% of Asp.Net Mvc programmers never modified Controllers ViewData.Model directly

What I do sometimes is setting ViewDataDictionary.ModelExplorer when controller returns a null model, and the View has a generic "object" model. This way I pass metadata to this kind of "poorly typed " View also when model is null.

So I understnd (As I already said) this behavior for ViewComponent might be "by deisgn", but being very different from the way people is used to work with controllers it is counter-intuitive.

[dougbu]

when controllers are invoked their ViewData.Model are always null!

Just as in view components, the action may have set ViewData.Model.

being very different from the way people is used to work with controllers it is counter-intuitive.

It seems you're arguing view components work too much like controllers. What is the "counter-intuitive" difference you're seeing? Or, what change would you like to see in how View(...) works in view components?

[frankabbruzzese]

@dougbu ,
It is counter-intuitive because at certain point everyone in this thread was almost sure there was a bug. If the behavior were intuitive, someone would have solved the issue with no need to debug the code.

My previous argument had the purpose of understanding why.
I think, because since Mvc 2 people is used that when it set a null model in View(...) it passes a null model to the View. You may object that also in the early Mvc versions in this case the Controller ViewData is passed as its is...not the model is set to null. However, I think this is irrelevant, since controller always receives an empty ViewDataDictionary and none set the model directly in the controller ViewDataDictionary so this possibility was never used an probably never "discovered" by users.

What changes with ViewComponents is that they receive the caller View ViewDataDictionary.

I dont find counter-intuitive for the developer setting the ViewData.Model to null (if he is well aware of the default behavior...)...nothing strange with this, I found counter-intuitive that when developer do nothing he get the caller View ViewModel in the ViewComponent View!

Most people doing nothing proibably will do it for error because they forget or are not aware of the default behavior. Probably, a lot of people will do the sane error of @simon25608 also if this point will be well documented.

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.

[dazinator]

I just raised a potentially related problem with some further info: #5597

[ajaybhargavb]

Spoke to @Eilon and @dougbu. We decided to flow the Model of the caller only in the cases when null is not explicitly passed to the View() method.

[Eilon]

@ajaybhargavb and we should move this bug to 2.0.0, right?

[ajaybhargavb]

Yes and also #5597

[ajaybhargavb]

4404833

[hadevnet]

This bug fixed?

[ajaybhargavb]

Yes. The new behavior is explained in this announcement aspnet/Announcements#221.