Required parameters to blazor components
montyclt opened this issue ยท 36 comments
Is your feature request related to a problem? Please describe.
I'm building a component where optional parameter doesn't make sense.
Describe the solution you'd like
Add required parameter to Parameter attribute
[Parameter(required = true)]
protected string Name { get; set; }This is an old idea. It was moved from original Blazor repository to this #5455. Unfortunately it is now closed.
Thank you @Andrzej-W. I close it as duplicate.
Reopening this. We do plan to do a feature like this but it won't happen in 3.0.
The implementation of this may need to mix with the #nullable variable feature in c# 8. The null checks will flag [Parameter(required = true)] protected T myVar {get; set;} as uninitialized, but in fact this attribute is saying that the variable must be initialized; but it is initialized elsewhere.
Clearing the milestone as I'd like to consider this for 5.0 now that we have a bunch of components that have required parameters.
This could have came in handy for me too, for now I just wrapped the property.
[Parameter]
public string MessageText
{
get => _MessageText ?? throw new NotImplementedException();
set => _MessageText = value;
}
private string _MessageText { get; set; }
Is not possible to add this to 3.1?
now we to set #nullable disable in each params section to disable nullable warnings.
now we to set #nullable disable in each params section to disable nullable warnings.
@kattunga
Use null! instead, much more nice:
[Parameter]
public UserEditViewModel UserEdit { get; set; } = null!;Use null! instead, much more nice:
That works, but it really just tells the compiler to ignore the problem.
It'd be nicer if we can have the compiler surface the problem on the component consumer's end:
<MyComponent /> <!-- compiler warning: UserEdit isn't set -->
<MyComponent UserEdit="@foo" /> <!-- no warning! -->
It would be nice if there is a possibility to tell the components, that some arguments are mandatory to function like in HTML.
For example it is from an html view totally ok to not specify an alt-tag, but an img-tag without src would not work.
It would be nice if there is a possibility to tell the components, that some arguments are mandatory to function like in HTML.
For example it is from an html view totally ok to not specify an alt-tag, but an img-tag without src would not work.
@MarkusRodler The idea of required parameters is that should throw an error when building the project if some component is called without its required parameter.
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.
Moving this out of 5.0 given the cost.
Splitting this work in to two parts. For 5.0, we'll introduce the property on ParameterAttribute. This should allow us to scavenge this metadata at a later point when tooling can support it. Tooling work tracked via https://github.com/dotnet/aspnetcore/issues/25055.
The implementation in #25054 would have implemented a Required property on the ComponentParameter. The implementation verified that a value for each required component parameter was specified as part of SetParametersAsync. When we discussed this implementation, a few issues came up:
-
The runtime check could not be guaranteed in the event
SetParametersAsyncis overridden. We may recommend overriding this method for certain kinds of components, which makes the benefit of the runtime check a bit lax. -
The runtime check does not guarantee non-nullness of reference types. Often components may need to indicate that a parameter is specified and not null.
Given these limitations, it seems the benefit of the runtime check is questionable. Instead, we think could focus on the tooling aspects of this. Tooling can look for an attributed on parameters that indicates if a parameter is required. We can look up such attributes by name, so it would not require a runtime feature and we could enable it any point before 6.0:
[Parameter]
[UIRequired] // Name is TBD
public string MyRequiredParameter { get; set; }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.
We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.
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.
I believe the purpose of the request is for some kind of analyzer to output an error if we don't set a required parameter's value when consuming the component in a razor file.
Am I correct @montyclt ?
FYI: https://github.com/excubo-ag/Generators.Blazor/ has experimental support for this kind of feature.
The way we interpreted this was to have a design time gesture that indicated that parameters that were marked as required were not specified. There's a risk there are false positives (particularly with splatting) that could not be statically inferred so we think treating it as a nudge-in-the-right-direction sort of feature is less bothersome.
I believe the purpose of the request is for some kind of analyzer to output an error if we don't set a required parameter's value when consuming the component in a razor file.
Am I correct @montyclt ?
Yes, but if it is possible, I would like to analyze OpenComponent method of BuildRenderTree
Yes, but if it is possible, I would like to analyze OpenComponent method of BuildRenderTree
This is exactly what the experimental analyzer in https://github.com/excubo-ag/Generators.Blazor/ does. See https://github.com/excubo-ag/Generators.Blazor/blob/main/Blazor/RequiredParameterAnalyzer.cs
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.
I have solved this in a basic way with the following extension method during runtime:
#nullable enable
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.Linq;
using System.Reflection;
using FluentAssertions.Common;
using Microsoft.AspNetCore.Components;
namespace Blazor
{
public static class ComponentExtension
{
public record ParameterInfo(PropertyInfo PropertyInfo, bool IsRequired);
private static readonly ConcurrentDictionary<Type, ParameterInfo[]> _typeInfo = new();
/// <summary>
/// Validate parameters
/// </summary>
/// <param name="component"></param>
public static void ValidateParameters(this IComponent component)
{
#if DEBUG
var componentType = component.GetType();
if (!_typeInfo.TryGetValue(componentType, out var parameterInfo))
{
var props = componentType
.GetProperties()
.Where(x => x.IsDecoratedWith<ParameterAttribute>());
var list = new List<ParameterInfo>();
foreach (var prop in props)
{
list.Add(new ParameterInfo(prop, prop.GetCustomAttribute<RequiredAttribute>() != null));
}
_typeInfo[component.GetType()] = parameterInfo = list.ToArray();
}
foreach (var property in parameterInfo.Where(x=>x.IsRequired) )
{
var value = property.PropertyInfo.GetValue(component);
if (value == null)
throw new ArgumentNullException(property.PropertyInfo.Name);
switch (property.PropertyInfo.PropertyType)
{
case Type t when t == typeof(Guid):
{
if ((Guid)value == default)
throw new ArgumentNullException(property.PropertyInfo.Name);
break;
}
}
}
#endif
}
}
}In every component where I want to have validation I call this method
[Parameter] [Required] public Guid Id { get; set; } = default!;
protected override void OnInitialized()
{
this.ValidateParameters();
}@JvanderStad the issue isn't checking in the component at runtime, but checking in the consumer at compile time.
@JvanderStad the issue isn't checking in the component at runtime, but checking in the consumer at compile time.
I'm aware of this, but it's better than nothing. I'm just sharing what works for me right now.
With a little luck the compile-time version will use the same attribute.
Solving it at compile time has been possible (and stable) for many months now (see above comments). No need to work with runtime-only. The only real question is whether this should be a separate attribute ([Required][Parameter] public T Value {get;set;}) or as part of the Parameter attribute ([Parameter(required=true)] public T Value {get;set;}).
Didn't see that. Ah wel ยฏ_(ใ)_/ยฏ
The only real question is whether this should be a separate attribute (
[Required][Parameter] public T Value {get;set;}) or as part of theParameterattribute ([Parameter(required=true)] public T Value {get;set;}).
Would one ever use a [Required] attribute without a [Parameter] attribute? I don't think so. Based on that, it seems like the [Parameter(required=true)] makes more sense. The "required" modifies the "Parameter" attribute, but is not an attribute on its own.
I'm not in the position to make that decision, nor do I have a strong preference either way. But there is a reason for a separate attribute, and that is precedent and existing code: RequiredAttribute (System.ComponentModel.DataAnnotations) https://docs.microsoft.com/en-us/dotnet/api/system.componentmodel.dataannotations.requiredattribute?view=net-5.0
One attribute = fewer reflection calls
At compile time, there's no concern about reflection. It doesn't matter to a source generator whether there are one or two attributes.
What precedent is there? In data annotations, [Required] is independent; it doesn't rely on any other attribute to define its usefulness.
In this case, [Required] would only mean something if [Parameter] is present, yeah?
I think using [Required] could be confusing because [Required] is used by some components like EditForm in other properties. I vote for using [Parameter(Required = true)].