Allow MudDatePicker to bind to non-nullable DateTime field with working validation

[lieszkol]

Feature request type

Enhance component

Component name

MudDatePicker

Is your feature request related to a problem?

A similar topic has been raised as a discussion on github a year ago.

Using EFCorePowerTools & database first approach, after reverse engineering the DB, DATETIME NOT NULL columns are generated as non-nullable datetimes:

public DateTime InsertedOn { get; set; }

I am new to MudBlazor & perhaps I'm on the wrong track, but I've spent days trying to get MudDatePicker to work with such an entity. I have not been able to configure EFCorePowerTools to generate the field as DateTime? and I'm not convinced that is a good method anyway, since such a field always has a value & the UI library shouldn't dictate the semantics of the model.

Describe the solution you'd like

My request is for MudDatePicker to natively support binding to a DateTime field, including properly handling data validation with Fluent validators when For tag references a non-nullable field.

Have you seen this feature anywhere else?

Most other Blazor UI libs support this scenario.

Describe alternatives you've considered

I was able to create a component that can be bound to either a nullable or a non-nullable DateTime field, "ZenDatePicker.razor", as follows:

@using System.Linq.Expressions

<MudDatePicker @bind-Date="InternalDate" Required="@Required" For="@For" />

/* Based on: https://github.com/MudBlazor/MudBlazor/blob/5e66ce70acbecfe6664e7de6c508716a0feb44ca/src/MudBlazor/Components/Table/MudColumn.razor.cs
	*/
	DateTime? InternalDate
	{
		get => Required ? new DateTime?(DateRequired) : Date;
		set
		{
			if (!EqualityComparer<DateTime>.Default.Equals(value ?? default, Required ? DateRequired : Date ?? default))
			{
				if (Required)
				{
					DateRequired = value ?? DefaultValue ?? default;
					DateRequiredChanged.InvokeAsync(DateRequired);
				} else {
					Date = value;
					DateChanged.InvokeAsync(Date);
				}
			}
		}
	}

	[Parameter]
	public DateTime? DefaultValue { get; set; }

	[Parameter] public EventCallback<DateTime?> DateChanged { get; set; }

	[Parameter]
	public DateTime? Date { get; set; }

	[Parameter] public EventCallback<DateTime> DateRequiredChanged { get; set; }

	[Parameter]
	public DateTime DateRequired { get; set; }

This can then be used to bind either to nullable or non-nullable datetime fields:

<ZenDatePicker @bind-Date="entity.SomeNullableDateTimeField" For="@(() => entity.SomeNullableDateTimeField)" />
<ZenDatePicker @bind-DateRequired="entity.InsertedOn" DefaultValue="@(DateTime.Now)" For="@(() => entity.InsertedOn)" />

InsertedOn is just used as an example here, so I'd prefer to avoid arguments over whether a field such as InsertedOn should be set in the database and not on the client side etc.

The above workaround works fine except for one issue. If For="@(() => entity.InsertedOn)" is set then MudBlazor throws exceptions in several places:

(1) ExpressionExtensions.cs:

public static string GetLabelString<T>(this Expression<Func<T>> expression)
        {
            var memberExpression = (MemberExpression)expression.Body;

this could be fixed by updating the above line to: var memberExpression = (expression.Body as MemberExpression ?? (MemberExpression)((UnaryExpression)expression.Body).Operand);

(2) MudFormComponent.cs:

        protected override void OnParametersSet()
        {
            if (For is not null && For != _currentFor)
            {
                // Extract validation attributes
                // Sourced from https://stackoverflow.com/a/43076222/4839162
                // and also https://stackoverflow.com/questions/59407225/getting-a-custom-attribute-from-a-property-using-an-expression
                var expression = (MemberExpression)For.Body;

                // Currently we have no solution for this which is trimming incompatible
                // A possible solution is to use source gen
#pragma warning disable IL2075
                var propertyInfo = expression.Expression?.Type.GetProperty(expression.Member.Name);
#pragma warning restore IL2075                
                _validationAttrsFor = propertyInfo?.GetCustomAttributes(typeof(ValidationAttribute), true).Cast<ValidationAttribute>();

                _fieldIdentifier = FieldIdentifier.Create(For);

the first error is thrown at var expression = (MemberExpression)For.Body;, this could be fixed by updating the last line to: var expression = (For.Body as MemberExpression ?? (MemberExpression)((UnaryExpression)For.Body).Operand);

However, a second error is thrown at FieldIdentifier.Create(For); which I have no idea how to fix.

The problem is that FieldIdentifier.Create calls ParseAccessor:

 private static void ParseAccessor<T>(Expression<Func<T>> accessor, out object model, out string fieldName)
    {
        var accessorBody = accessor.Body;

        // Unwrap casts to object
        if (accessorBody is UnaryExpression unaryExpression
            && unaryExpression.NodeType == ExpressionType.Convert
            && unaryExpression.Type == typeof(object))
        {
            accessorBody = unaryExpression.Operand;
        }

But the unaryExpression.Type == typeof(object) fails, so accessorBody = unaryExpression.Operand; is never called and an error is thrown: "The provided expression contains a UnaryExpression which is not supported. FieldIdentifier only supports simple member accessors (fields, properties) of an object."

Pull Request

• I would like to do a Pull Request

Code of Conduct

• I agree to follow this project's Code of Conduct

[ScarletKuro]

Hi.

It has been discussed multiple times where people requested DateOnly, DateTimeOffset, etc.
The problem is that C# does not support discriminated unions.
How do you support DateOnly, DateTimeOffset, DateTime, DateOnly?, DateTimeOffset?, DateTime??
Blazor does not offer any solution to this problem, so one option is to have its own [Parameter] for each type, but this works well only if there are two types maximum like int? and int. However, if there are 6+ types, the code will become spaghetti-like. We are not going to support this.
Another approach involves using generics in MudDatePicker. However, C# lacks date time constraints for generics like for generic maths, making the code complex. Progress on this solution can be tracked at #8567. There is no ETA, and I wouldn't count on this for now.

I always recommend against directly using database entities as models with UI components, except for DataGrid (only if it can directly work with EF). It's not a good practice. Instead, always opt to map the entity to a DTO and vice versa. This can save you from bugs and reduce debugging efforts in the future. Examples of problems you can run into when using EF entities include over-posting attacks, the N+1 problem, and tight coupling between your UI form data and database tables etc etc etc.

[ScarletKuro]

Most other Blazor UI libs support this scenario.

FluentUI for example uses only DateTime? as well.
Radzen does allow multiple types, but they are using object, internally it's all getting converted to DataTime? tho. I don't like this approach either, and they are using a lot of "illegal" patterns(logic in parameter setter and getter as one of them) in Blazor, we have recently banned all this "bad" patterns for the v7 and we are slowly getting rid of it on our code as well.

[lieszkol]

First of all I appreciate MudBlazor and I think it holds promise to become a great library. I also appreciate your quick response and your suggestions to always opt to map the entity to a DTO. I understand that this is a great approach for many projects, unfortunately it is not a good fit for the one I am working on now.

Getting back to the issue, it is not my place to argue about your design decisions, I can only explain my case as a user of MudBlazor. I am in fact trying to use MudDatePicker in a DataGrid, but doing so is taking a lot of work at the moment (for example, configuring a proper filtertemplate to enable the user to filter for rows where the date is on or past a given period etc.). The issue I raised here was another blocker I ran into and I think many other users will too. If I understand the official EF Core documentation right, it is accepted convention to back required properties with non-nullable fields. I would argue that having separate [parameter] for DateTime? and DateTime would cover 95% of use cases.

In any case I have no problem using the method I described above where I create a custom component that can be bound to a non-nullable property. Others can create their own custom components if they need to bind to DateTimeOffset or Date or Time or whatever. I think that's a good approach, MudBlazor gives the basic components and it is left up to users to implement their more-specific use cases. I would however ask that the two places where you cast (MemberExpression)For.Body be changed to be able to handle UnaryExpressions. I can make a pull request if you would like, although it would be a lot easier for you to just fix these two lines. This is a very minor fix and surely would not break anything. If anyone could give some pointers on how to fix the line _fieldIdentifier = FieldIdentifier.Create(For); that would be great help as well. I suspect it should be possible to just do something like _fieldIdentifier = new FieldIdentifier(model, fieldName);.

If you are unwilling to do the above, then at least allow us to override MudFormComponent.OnParametersSet() so that we (the users of MudBlazor) can resolve this issue without having to fork MudBlazor and then deal with merging updates back to our codebase. Currently this is not possible since you make so many properties/fields private. If you changed the following fields to protected then I could at least fix this issue myself without forking MudBlazor (by creating a class that inherits from MudDatePicker and overrides OnParametersSet):

private Expression<Func<T>>? _currentFor;
private IEnumerable<ValidationAttribute>? _validationAttrsFor;
private FieldIdentifier _fieldIdentifier;
private EditContext? _currentEditContext;
private void OnValidationStateChanged(object? sender, ValidationStateChangedEventArgs e)

With this approach, the MudDatePicker would still be bound to a DateTime? property, so you would not have to deal with any issues with binding to a non-nullable property. It is just the For attribute that would be set to the true underlying DateTime field. This way you would at least allow us to work around the problem.

As a side-note, I also played around with creating a DateTime? InternalFor property in ZenDatePicker and then setting <MudDatePicker For="@InternalFor" but this doesn't work for many reasons, for example then expression.Member.Name obviously returns "InternalFor" and not the name of the underlying model field etc.

[ScarletKuro]

I would argue that having separate [parameter] for DateTime? and DateTime would cover 95% of use cases.

Now that you've mentioned For, I recalled why we can't have more than one parameter (and couldn't have File and Files in MudUploadFile). Internally, the MudFormComponent has only one _value, and the MudDatePicker is bound to that:

[Parameter]
public DateTime? Date
{
	get => _value;
	set => SetDateAsync(value, true);
}

and everything revolves around that single value; you can't have two for validation. Since this logic is delicate, we aren't planning to change it for now; the validation logic rewrite has been pushed to the v8 milestone.

If you are unwilling to do the above, then at least allow us to override MudFormComponent.OnParametersSet()

Isn't this already possible?

public class MyMudFormComponent<T, U> : MudFormComponent<T, U>
{
  public MyMudFormComponent(MudBlazor.Converter<T, U> converter) : base(converter)
  {
  }

  protected override void OnParametersSet()
  {
	  base.OnParametersSet();
  }
}

Unless you mean something else.

However, as I mentioned, this is being addressed in #8567. We don't have a solution here and now, except for simply wrapping MudDatePicker and performing the conversion on your side. Someone even posted a code example for it.

cc: @henon if you have anything to add.

[henon]

FYI @ArieGato your PR should fix this issue.

As for validating multiple values, I have already noticed that this is a shortcoming of our validation system which needs to be changed in v8 for components such as MudList, MudChipSet, MudTreeView and MudSelect where we have multi-selection. In singleselection they use SelectedValue (or in case of MudSelect the Value property) which can be validated in a form. But when they use multi-selection they use SelectedValues and there is currently no way to validate the selected values.

[lieszkol]

By "allow us to override MudFormComponent.OnParametersSet()" I mean that I could work around this issue if the following were set as protected instead of private in MudFormComponent.cs:

private Expression<Func<T>>? _currentFor;
private IEnumerable<ValidationAttribute>? _validationAttrsFor;
private FieldIdentifier _fieldIdentifier;
private EditContext? _currentEditContext;
private void OnValidationStateChanged(object? sender, ValidationStateChangedEventArgs e)
private void DetachValidationStateChangedListener()
private EditContext? EditContext { get; set; } = default!;
internal IForm? Form { get; set; }

Specifically, then the following approach would work without throwing "System.InvalidCastException: 'Unable to cast object of type 'System.Linq.Expressions.UnaryExpression' to type 'System.Linq.Expressions.MemberExpression'.' on line 654 of MudFormComponent.cs and errors in several other places.

(1) I create a component that inherits from MudDatePicker but overrides OnParametersSet, correctly handling the case where For= points to a non-nullable DateTime field:

public partial class ZenDatePickerBase: MudDatePicker
	{
		#region Properties

		[Parameter]
		[MudBlazor.Category(CategoryTypes.FormComponent.Data)]
		public DateTime DateRequired
		{
			get => (DateTime)_value;
			set => SetDateAsync(value, true).AndForget();
		}

protected override void OnParametersSet()
		{
			if (For is not null && For != _currentFor)
			{
				// Extract validation attributes
				// Sourced from https://stackoverflow.com/a/43076222/4839162
				// and also https://stackoverflow.com/questions/59407225/getting-a-custom-attribute-from-a-property-using-an-expression
				// FIX below for: var expression = (MemberExpression)For.Body;
				var expression = (For.Body as MemberExpression ?? (MemberExpression)((UnaryExpression)For.Body).Operand);

				// Currently we have no solution for this which is trimming incompatible
				// A possible solution is to use source gen
#pragma warning disable IL2075
				var propertyInfo = expression.Expression?.Type.GetProperty(expression.Member.Name);
#pragma warning restore IL2075
				_validationAttrsFor = propertyInfo?.GetCustomAttributes(typeof(ValidationAttribute), true).Cast<ValidationAttribute>();

				//FIX below for: _fieldIdentifier = FieldIdentifier.Create(For);

				if (For.Body is UnaryExpression unaryExpression
					&& unaryExpression.NodeType == ExpressionType.Convert
					&& unaryExpression.Operand is MemberExpression memberExpression
					&& memberExpression.Expression is MemberExpression member
					&& member.Expression is ConstantExpression model
				)
				{
					var fieldName = memberExpression.Member.Name;
					object? value = model.Value ?? throw new ArgumentException("The provided expression must evaluate to a non-null value.");
					Func<object, object>? accessor = CreateAccessor((value.GetType(), member.Member.Name));
					if (accessor == null)
					{
						throw new InvalidOperationException($"Unable to compile expression: {member}");
					}
					_fieldIdentifier = new FieldIdentifier(model, fieldName);
				}
				else
				{
					_fieldIdentifier = Field
				}
				_currentFor = For;
			}

			if (EditContext is not null && EditContext != _currentEditContext)
			{
				DetachValidationStateChangedListener();
				EditContext.OnValidationStateChanged += OnValidationStateChanged;
				_currentEditContext = EditContext;
			}

			[UnconditionalSuppressMessage(
			"Trimming",
			"IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code",
			Justification = "Application code does not get trimmed. We expect the members in the expression to not be trimmed.")]
			static Func<object, object> CreateAccessor((Type model, string member) arg)
			{
				var parameter = Expression.Parameter(typeof(object), "value");
				Expression expression = Expression.Convert(parameter, arg.model);
				expression = Expression.PropertyOrField(expression, arg.member);
				expression = Expression.Convert(expression, typeof(object));
				var lambda = Expression.Lambda<Func<object, object>>(expression, parameter);

				var func = lambda.Compile();
				return func;
			}
		}

		protected override async Task ValidateModelWithFullPathOfMember(Func<object, string, Task<IEnumerable<string?>>> func, List<string> errors)
		{
			try
			{
				if (Form?.Model is null)
				{
					return;
				}

				if (For is null)
				{
					errors.Add($"For is null, please set parameter For on the form input component of type {GetType().Name}");
					return;
				}

				//FIX below for: foreach (var error in await func(Form.Model, For.GetFullPathOfMember()))
				foreach (var error in await func(Form.Model, For.GetFullPathOfMemberV2()))
					if (!System.String.IsNullOrEmpty(error))
						errors.Add(error);
			}
			catch (Exception e)
			{
				errors.Add("Error in validation func: " + e.Message);
			}
		}
	}
}

ValidateModelWithFullPathOfMember only needs to be overridden because ExpressionExtensions.GetFullPathOfMember needs a one-line fix to be able to handle UnaryExpressions.

(2) GetFullPathOfMemberV2 is a fixed version of GetFullPathOfMember defined in a helper class:

public static string GetFullPathOfMemberV2<T>(this Expression<Func<T>> property)
		{
			var resultingString = string.Empty;
			//var p = property.Body as MemberExpression;
			var p = (property.Body as MemberExpression ?? (MemberExpression)((UnaryExpression)property.Body).Operand);

			while (p is not null)
			{
				if (p.Expression is MemberExpression)
				{
					resultingString = p.Member.Name + (resultingString != string.Empty ? "." : string.Empty) + resultingString;
				}
				p = p.Expression as MemberExpression;
			}
			return resultingString;
		}

The only change is this:
var p = property.Body as MemberExpression;
→ var p = (property.Body as MemberExpression ?? (MemberExpression)((UnaryExpression)property.Body).Operand);

(3) ZenDatePicker.razor now builds on ZenDatePickerBase.cs:

<ZenDatePickerBase @bind-Date="InternalDate" @ref="_elementReference"
For="@For" Required="@Required">
...
</ZenDatePickerBase>

@code {
DateTime? InternalDate
	{
		get => Required ? new DateTime?(DateRequired) : Date;
		set
		{
			if (!EqualityComparer<DateTime>.Default.Equals(value ?? default, Required ? DateRequired : Date ?? default))
			{
				if (Required)
				{
					DateRequired = value ?? DefaultValue ?? default;
					DateRequiredChanged.InvokeAsync(DateRequired);
				} else {
					Date = value;
					DateChanged.InvokeAsync(Date);
				}
			}
		}
	}

[Parameter]
	public DateTime? DefaultValue { get; set; }

	[Parameter]
	public DateTime? Date { get; set; }

	[Parameter] public EventCallback<DateTime?> DateChanged { get; set; }

[Parameter]
	public DateTime DateRequired { get; set; }

	[Parameter] public EventCallback<DateTime> DateRequiredChanged { get; set; }

[Parameter]
	public Expression<Func<DateTime?>>? For { get; set; }

[Parameter]
	public bool Required { get; set; }
}

(4) This can now be used in a form or dialog to bind either nullable or non-nullable datetime backing property:

<ZenDatePicker @bind-Date="entity.SomeNullableDateTimeField" For="@(() => entity.SomeNullableDateTimeField)" />
<ZenDatePicker @bind-DateRequired="entity.InsertedOn" DefaultValue="@(DateTime.Now)" For="@(() => entity.InsertedOn)" />

Works great, but for this to work I need the fields/members/methods I name at the top to be set as protected and not private.

[lieszkol]

In general, I've run into a lot of walls with MudBlazor where I could easily work around a problem but I cannot because so many properties/fields/methods are market private or internal instead of protected.

[ScarletKuro]

In general, I've run into a lot of walls with MudBlazor where I could easily work around a problem but I cannot because so many properties/fields/methods are market private or internal instead of protected.

This is just the basics of encapsulation. Without it, we would hinder the framework's ability to evolve without every change becoming a breaking one. Most of them can be removed in v8. Perhaps some of them make sense to be protected, like the EditContext, but definitely not all of them. But as I understand, the main problem is the UnaryExpression? I don't see that you are doing anything beyond that. Why don't you just want to PR the UnaryExpression support?

upd: Haha @henon, we replied at same time almost the same way.

[henon]

@lieszkol The reason why we want to keep as many internals private or internal is because making them protected de-facto makes them public and we can't rename, change or remove them w/o a breaking change. That's the rationale behind keeping the public API surface as small as possible.

That being said, we can make some things protected if you need that, no problem. The trade-off for giving more access to internals would be that we can't guarantee to keep the protected API stable to not constrict us when we need to change internals.

But of course it would be best for everyone if we'd just fix the underlying problem (if possible) instead of keeping the component in a less-than-optimal state and have every user work around it. As said above a PR is in the works to solve exactly this problem. #8567