Mvc: Different working in ViewComponents in v.1.1.0
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
@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
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.
simon25608
All 34 comments
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.
frankabbruzzese
on 23 Nov 2016
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+
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
Microsoft.AspNetCore.Mvc.Razor.RazorView+
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
Microsoft.AspNetCore.Mvc.ViewComponents.ViewViewComponentResult+
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
Microsoft.AspNetCore.Mvc.ViewComponents.DefaultViewComponentInvoker+
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
Microsoft.AspNetCore.Mvc.ViewComponents.DefaultViewComponentHelper+
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
System.Runtime.CompilerServices.TaskAwaiter.GetResult()
AspNetCore._Views_Shared_Components_Archivos_Listado_cshtml+
simon25608
on 24 Nov 2016
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)).
frankabbruzzese
on 24 Nov 2016
If the exception says that the last model is of type IList
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
simon25608
on 24 Nov 2016
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.
frankabbruzzese
on 24 Nov 2016
@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)
iscifoni
on 24 Nov 2016
@frankabbruzzese
Correct. If you want the files to play it, let me know.
simon25608
on 24 Nov 2016
@iscifoni
The problem is with the view component, not with partial views, that is working correctly.
I do not understand the relationship: S
simon25608
on 24 Nov 2016
@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.
frankabbruzzese
on 24 Nov 2016
@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
simon25608
on 24 Nov 2016
@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,
iscifoni
on 24 Nov 2016
@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
simon25608
on 24 Nov 2016
@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 ?
iscifoni
on 24 Nov 2016
@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.
simon25608
on 24 Nov 2016
@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
on 24 Nov 2016
@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
on 24 Nov 2016
@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:
frankabbruzzese
on 25 Nov 2016
@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.
simon25608
on 25 Nov 2016
@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.
iscifoni
on 25 Nov 2016
@iscifoni
Yes, it's what you said yesterday
simon25608
on 25 Nov 2016
@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.
frankabbruzzese
on 25 Nov 2016
@ajaybhargavb can you help investigate?
Eilon
on 26 Nov 2016
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.
frankabbruzzese
on 27 Nov 2016
@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.
dougbu
on 27 Nov 2016
@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.
frankabbruzzese
on 27 Nov 2016
when controllers are invoked their
ViewData.Modelare 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?
dougbu
on 28 Nov 2016
@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.
frankabbruzzese
on 28 Nov 2016
I just raised a potentially related problem with some further info: #5597
dazinator
on 6 Dec 2016
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.
ajaybhargavb
on 2 Feb 2017
@ajaybhargavb and we should move this bug to 2.0.0, right?
Eilon
on 2 Feb 2017
Yes and also https://github.com/aspnet/Mvc/issues/5597
ajaybhargavb
on 2 Feb 2017
44048331e936de39073ca5eab97bd5b5cdb0a0f2
ajaybhargavb
on 14 Feb 2017
This bug fixed?
hatefghani
on 18 Mar 2017
Yes. The new behavior is explained in this announcement https://github.com/aspnet/Announcements/issues/221.
ajaybhargavb
on 18 Mar 2017
Related issues
saf-itpro
路
3Comments
miroslavsiska
路
4Comments
reekoheek
路
3Comments
CezaryRynkowski
路
4Comments
CzechsMix
路
3Comments
Most helpful comment
@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 ?