ViewComponentTagHelper: Allow optional parameters #5011

pawchen · 2017-04-28T17:28:40Z

Say we have a ViewComponent class

class MyViewComponent
{
    IViewComponentResult Invoke( bool showSomething = false ) { ... }
}

It would be great to just only write <vc:my /> in Razor if one don't want to show that something. Currently you have to write <vc:my show-something="false" />.

The text was updated successfully, but these errors were encountered:

DamianEdwards · 2017-04-28T17:32:00Z

Agreed. No idea how feasible it is, but I agree with the sentiment 😄

rynowak · 2017-04-28T17:41:08Z

This seems pretty feasible. We need to teach this code about default values. Currently it says that each parameter is a required attribute

DamianEdwards · 2017-04-29T05:27:09Z

@rynowak candidate for preview2?

rynowak · 2017-04-29T05:53:59Z

I'm going to move it in and mark it up for grabs. At this point locking down our public APIs and completing the tooling story needs to take priority.

svallis · 2017-06-29T10:09:46Z

Really looking forward to this fix. As things stand, using the VC tag helper is very dangerous. Add a new parameter to an existing and widely used tag helper and miss updating one of the places it is called and it silently fails while emitting server side markup to the browser. I guess we're too far along now for this to make it in to 2.0.0?

rynowak · 2017-06-29T21:49:20Z

This is not going to make it for 2.0.0. We don't have time to update tooling at this point.

Liero · 2017-10-21T19:21:49Z

I believe it should be marked as a bug.

This is working:

@await Component.Invoke(typeof(MyViewComponent));

This is not working

<vc:My />

It is expected behavior and since you do not show any warning, error, debug message, nothing, it upsets a lot of people wondering why the hell the view component is not rendered

clockwiseq · 2017-12-31T17:00:49Z

So will this make it into the next release?

dseipp · 2018-11-02T12:21:59Z

Looking for an update on this issue. Will this be added any time soon? Rather frustrating one to deal with.

ASKemp · 2019-05-16T09:42:40Z

Came here to say same thing after finding this issue and spending some time tracking down why on of my tag helper called VC's wasn't presenting any output.

CamiloTerevinto · 2019-05-27T06:34:42Z

Hey @rynowak:

I thought I would give this a shot and after reading the source code here (your link is dead), it seems pretty easy to fix. However, there are at least two ways ( either create a replica of RequiredAttributeDescriptor and friends, or move to a base clas/interface) to do this and it's a substantial amount of code changes.

emumanu · 2019-07-23T10:21:21Z

@rynowak and/or @DamianEdwards, please tell something to @CamiloTerevinto as we want to see this improved.

Rick-Anderson · 2019-07-29T17:18:52Z

Updated View components in ASP.NET Core with

All view component parameters are required

Each parameter in a view component is a required attribute. See this GitHub issue. If any parameter is omitted:

The InvokeAsync method signature won't match, therefore the method won't execute.
The ViewComponent won't render any markup.
No errors will be thrown.

lvmajor · 2019-10-23T14:21:15Z

Just got bitten hard by it (the notice was not there at the time I implemented it in my app)

Is there a planned solution or will it stay like this for the foreseeable future? As this can be really dangerous, especially that it fails silently... Could there at least be a warning or something like that at compile time?

jerriep · 2019-10-30T06:09:07Z

Yeah, I agree with the last comment. I also got bitten hard by this recently and the fact that it simply fails silently is not good.

Rick-Anderson · 2019-10-30T21:39:26Z

@rynowak is it possible to generate an error?

staa99 · 2019-11-12T15:44:57Z

@rynowak Will there be any support for this in the future?

It's been over two years, and honestly, it is a pain to have to duplicate default parameters everywhere a view component is used.

Rick-Anderson · 2019-11-12T18:13:35Z

@rynowak what's the correct line number in

This seems pretty feasible. We need to teach this code about default values. Currently it says that each parameter is a required attribute

this code : updated link but maybe not correct line

josephr5000 · 2020-01-31T21:23:57Z

Thanks @Rick-Anderson for updating the docs, but it's ridiculous that you should even have to. This makes the tag helper UNUSABLE for serious projects. To have code fail silently when you refactor or add features should be completely unacceptable. I'm stunned that this is still open almost three years after first being reported. Is any fix planned?!

clockwiseq · 2020-02-25T19:01:18Z

Has anyone from the aspnetcore team looked at this and given an ultimate answer as to whether or not this will be implemented? I personally think it's something that makes view components actually useful and would reduce redundant code. At bare minimum, I would like to see someone from the team take ownership and make a decision on the fate of this feature.

Rick-Anderson · 2020-02-25T19:24:30Z

@mkArtakMSFT @rynowak please review and @rynowak can you check

@rynowak what's the correct line number in

This seems pretty feasible. We need to teach this code about default values. Currently it says that each parameter is a required attribute

this code : updated link but maybe not correct line

ghost · 2020-11-16T19:01:19Z

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

jishansiddique · 2021-02-04T09:42:15Z

This issue exists in .NET 5 as well as.
Any update?

mc0re · 2021-02-12T20:36:54Z

Also note, that this syntax works fine for tag helpers (as it does in ordinary HTML and in XAML -and I'm probably missing a dozen of other examples). I wanted to switch from a tag helper to a view component, as my HTML grew, and I didn't want to generate it in code. Now I have to add a lot of unnecessary HTML in all the other places :-(

Santas · 2021-02-24T17:02:30Z

Is there a plan to land this in .NET 6? Thanks!

jishansiddique · 2021-02-24T17:15:54Z

Hi @Santas,
As per the blog Click me,
I've checked but not able to find any optional parameter in the view component.

OlegWald · 2021-03-21T12:12:15Z

we really need optional parameters support in ViewComonents. please give us this

mkArtakMSFT · 2021-05-17T19:16:22Z

Let's try to time-box this effort. If we can do this in 2 days, we'll do this in current milestone (6.0-preview6)

lvmajor · 2021-05-17T19:18:23Z

Let's try to time-box this effort. If we can do this in 2 days, we'll do this in current milestone (6.0-preview6)

This would be really awesome!

…32979) Addresses #5011. To implement support for optional parameters, we largely needed to do two things: - Update the `ViewComponentTagHelperDescriptorFactory` that is responsible for constructing `VCTHD` objects to _not_ produce the `RequiredAttributeDescriptors` for parameters that have a default value provided. - Update the generated code for VCTHs to produce an arguments object to the `InvokeCoreAsync` method that contains only the provided parameters **Updates to the VCTH descriptor factory** The VCTH descriptor contains a `RequiredAttributeDescriptor` for every parameter to the `InvokeAsync` method on the VC. When a parameter has a default value provided, we don't need to configure the parameter as a `RequiredAttribute` on the VCTH descriptor. With this change in place, instances of the VCTH that don't contain the required attribute will still match with the VCTH. For example, the following VC: ```csharp public class FooViewComponent : ViewComponent { public IViewResult InvokeAsync(string foo, string bar = "default value") { return $"{foo} : {bar}"; } } ``` Would produce a VCTH descriptor that looks like this: ``` - ViewComponentTagHelper - RequiredAttributeDescriptor(propertyName = "foo") - BoundAttributeDescriptor(propertyName = "foo") - BoundAttributeDescriptor(propertyName = "bar") ``` And will match VCTH instances like: ``` <vc:foo foo="a-foo-value"></vc:foo> <vc:foo foo="a-foo-value" bar="a-bar-value"></vc:foo> ``` But not VCTH instances that looks like: ``` <vc:foo bar="a-bar-value"></vc:foo> <vc:foo></vc:foo> ``` All this can happen with very few changes to the tag helper matching logic that already exists. **Update the generated code for VCTHs** Presently, whenever a VCTH is encountered, generated code is produced that: - Writes a class representing the view component which contains properties for each parameter provided to the `InvokeAsync` method and a `ProcessAsync` method that passes the class properties as arguments to `InvokeAsync` - Instantiates an instance of the class above and sets class properties with their associated attributes Even if the VCTH is matched to the VC successfully as a result of the changes above, the generated code will be inaccurate because null values will be passed for parameters that have default values. Instead, we want to construct an arguments object that only contains the parameters that were provided as attributes to the VCTH instance. We do this by adding a new `ProcessInvokeAsyncArgs` that evaluates if each of the parameters was provided as an attribute to the VCTH instance and if so, sets its value in the arguments object. ```csharp private Dictionary<string, object> ProcessInvokeAsyncArgs(Microsoft.AspNetCore.Razor.TagHelpers.TagHelperContext __context) { Dictionary<string, object> args = new(); if (__context.AllAttributes.ContainsName("first-name")) { args[nameof(firstName)] = firstName; } return args; } ``` The net effect is that we only pass set parameters as arguments to the method. For parameters that aren't passed as arguments, the default value will be used.

captainsafia · 2021-06-01T23:58:39Z

Closing as resolved. This will ship in .NET 6 Preview 6. Thanks for the patience folks!

JeremyCaney · 2021-06-02T00:05:26Z

Oh, I’m so happy to hear that, @captainsafia! This will make view components a lot more flexible—and, especially, when distributed as part of e.g. Razor Class Libraries, where the lack of either intuitive defaults or obvious error messages has made them cumbersome for implementors and a headache from a support perspective. Thank you for taking the time to complete this!

tompazourek · 2021-06-02T04:43:39Z

This is great. Thank you all very much.

I have one a bit related/unrelated question: How do you update the documentation pages so that they'll reflect the new features and changes? Should there be a PR in https://github.com/dotnet/AspNetCore.Docs to update the information there?

This part is now no longer true, right?

All view component parameters are required
Each parameter in a view component is a required attribute.
(...)

https://github.com/dotnet/AspNetCore.Docs/blob/main/aspnetcore/mvc/views/view-components.md#all-view-component-parameters-are-required

jishansiddique · 2021-06-02T05:15:34Z

This is really great. Thank you all very much.

captainsafia · 2021-06-02T18:08:47Z

@tompazourek Good call.

@Rick-Anderson We can update the docs here for .NET 6 on as this feature is now supported. If a default value is passed to a parameter, then that parameter is option.

Rick-Anderson · 2021-06-02T18:16:33Z

If a default value is passed to a parameter, then that parameter is option.

then that parameter is ~~option~~ optional.

NTaylorMullen changed the title ~~ViewComponentTagHelper: Allow omitting attribute for parameters with default value~~ ViewComponentTagHelper: Allow optional parameters Jun 29, 2017

aspnet-hello transferred this issue from aspnet/Razor Dec 14, 2018

aspnet-hello added this to the Backlog milestone Dec 14, 2018

aspnet-hello added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature feature-razor-pages help wanted Up for grabs. We would accept a PR to help resolve this issue labels Dec 14, 2018

chrismarts mentioned this issue Jul 29, 2019

Add a note that parameters are required dotnet/AspNetCore.Docs#13537

Closed

Rick-Anderson mentioned this issue Jul 29, 2019

Update view-components.md dotnet/AspNetCore.Docs#13540

Merged

mkArtakMSFT assigned captainsafia Mar 17, 2021

javiercn added the feature-mvc-razor-views Features related to the Razor view engine for Razor pages and MVC views label Apr 19, 2021

mkArtakMSFT modified the milestones: Next sprint planning, 6.0-preview6 May 17, 2021

DanBloxham-sw mentioned this issue May 18, 2021

HEEDLS-377 - edit centre manger details implemented and some detail l… TechnologyEnhancedLearning/DLSV2#329

Merged

ghost added the Working label May 19, 2021

captainsafia mentioned this issue May 25, 2021

Add support for default parameter values in ViewComponent tag helper #32979

Merged

captainsafia closed this as completed Jun 2, 2021

ghost added Done This issue has been fixed and removed Working labels Jun 2, 2021

Rick-Anderson mentioned this issue Jun 2, 2021

VC's in .NET 6 accept default params dotnet/AspNetCore.Docs#22476

Closed

ghost locked as resolved and limited conversation to collaborators Jul 2, 2021

ViewComponentTagHelper: Allow optional parameters #5011

ViewComponentTagHelper: Allow optional parameters #5011

Comments

pawchen commented Apr 28, 2017

DamianEdwards commented Apr 28, 2017

rynowak commented Apr 28, 2017

DamianEdwards commented Apr 29, 2017

rynowak commented Apr 29, 2017

svallis commented Jun 29, 2017

rynowak commented Jun 29, 2017

Liero commented Oct 21, 2017 • edited Loading

clockwiseq commented Dec 31, 2017

dseipp commented Nov 2, 2018

ASKemp commented May 16, 2019

CamiloTerevinto commented May 27, 2019

emumanu commented Jul 23, 2019

Rick-Anderson commented Jul 29, 2019 • edited Loading

All view component parameters are required

lvmajor commented Oct 23, 2019

jerriep commented Oct 30, 2019

Rick-Anderson commented Oct 30, 2019

staa99 commented Nov 12, 2019

Rick-Anderson commented Nov 12, 2019 • edited Loading

josephr5000 commented Jan 31, 2020

clockwiseq commented Feb 25, 2020

Rick-Anderson commented Feb 25, 2020

ghost commented Nov 16, 2020

jishansiddique commented Feb 4, 2021

mc0re commented Feb 12, 2021

Santas commented Feb 24, 2021

jishansiddique commented Feb 24, 2021

OlegWald commented Mar 21, 2021

mkArtakMSFT commented May 17, 2021

lvmajor commented May 17, 2021

captainsafia commented Jun 1, 2021

JeremyCaney commented Jun 2, 2021

tompazourek commented Jun 2, 2021

jishansiddique commented Jun 2, 2021

captainsafia commented Jun 2, 2021

Rick-Anderson commented Jun 2, 2021

Liero commented Oct 21, 2017 •

edited

Loading

Rick-Anderson commented Jul 29, 2019 •

edited

Loading

Rick-Anderson commented Nov 12, 2019 •

edited

Loading