[BUG] IsListNullOrEmptyConverter (plus others) do not work with nullable target types
Is there an existing issue for this?
- [X] I have searched the existing issues
Did you read the "Reporting a bug" section on Contributing file?
- [X] I have read the "Reporting a bug" section on Contributing file: https://github.com/CommunityToolkit/Maui/blob/main/CONTRIBUTING.md#reporting-a-bug
Current Behavior
When using IsListNullOrEmptyConverter to bind to a bool? value (for example AutomationProperties.IsInAccessibleTree) an InvalidCastException is thrown.
Expected Behavior
The converter works.
Steps To Reproduce
Use code like the following in XAML
AutomationProperties.IsInAccessibleTree="{Binding Items, Converter={StaticResource IsListNullOrEmptyConverter}}"
Or the following C#
var converter = new IsListNullOrEmptyConverter() as IValueConverter;
converter.Convert(new List<string>(), typeof(bool?), null, CultureInfo.CurrentCulture);
The linked repo contains XAML that triggers straight away. You will need to break on exceptions to tell that it has happened.
Additionally, there is a button to trigger it manually.
Link to public reproduction project repository
https://github.com/GalaxiaGuy/NullableConverterBug
Environment
- .NET MAUI CommunityToolkit: 7.0.1
- OS: macOS
- .NET MAUI: 8.0.3
Anything else?
No response
Have you tried disabling exceptions for converters:
https://learn.microsoft.com/en-gb/dotnet/communitytoolkit/maui/get-started?tabs=CommunityToolkitMaui#using-the-nuget-packages
Hi,
I don't think that would make a difference since the exception is already caught when used in XAML. I previously thought it was preventing the converter from correctly executing with the binding, but looking at it now it does seem fine.
I would guess that is expected behavior? Calling the converter manually still throws, but that seems uncommon for someone to do.
In my case it is also annoying since I'm trying to upgrade a Forms app to MAUI and breaking on exceptions especially to track down binding issues is useful. That exception is getting thrown a lot for us so some check that doesn't rely on exception handling would be preferred.
For reference, the exact line that trigger the invalid cast is on line 24 here:
https://github.com/CommunityToolkit/Maui/blob/666f0e97ced203c8db310b1cb772c9dcb6f37f7c/src/CommunityToolkit.Maui/Extensions/ValueConverterExtension.shared.cs#L23-L24
From what I can tell, it seems T is bool and targetType is bool?, so I think the failure is trying to assign to the instanceOfT variable.
I thought there was a PR that fixed this issue! I need to dig around and work out what it was
This was the PR that fixed this issue https://github.com/CommunityToolkit/Maui/pull/1307 the code that you've linked to does appear to be quite different so perhaps we have regressed somehow.
I'll try and take a look at some point.
@GalaxiaGuy I am only testing in VS Code and I am not seeing the exception being thrown from XAML. I have tested your code by adding a try catch around the manual triggering code and with debugging it I don't see any exceptions.
@bijington Apologies, it seems I don't see an exposed exception after all, it was just been caught internally. I originally thought the correct result from the converter was not getting setting in that case but I think something else was causing me problems as the same time.
I don't know if limiting internal exceptions is a specific goal, but I honestly have to run with exception breakpoints enabled quite frequently, especially to track down XAML issues. I think swapping the these two conditions would have the same result without throwing an exception internally:
&& !IsValidTargetType<TTarget>(targetType) // Ensure targetType be converted to TTarget? Eg `Convert.ChangeType()` returns a non-null value
&& !(shouldAllowNullableValueTypes && typeof(TTarget).IsValueType && IsValidNullableValueType(targetType))) // Is TTarget a Value Type and targetType a Nullable Value Type? Eg TTarget is bool and targetType is bool?
If I confirmed that and were to submit a PR to that effect would it likely to be accepted?
It isn't catching an exception internally. I have run your same code in our sample application and stuck a breakpoint in the exception handling as well as when converting and no exceptions are thrown. Maybe I am missing something?
But to answer your question, if there is a bug then we would love to accept a PR to resolve it :)
This is the (caught) exception I see in Rider:
While I getting that, I confirmed it is definitely caught and the end value is converted correctly, so I don't think it's worth anyone else looking at.
@GalaxiaGuy apologies for not getting back to this sooner, I have managed to reproduce the issue and can confirm that your proposed solution does in fact solve the issue. Would you still be willing to submit the PR? I would be more than happy for you to take the credit for the work. If not I am happy to submit it instead.
Actually I take it back, while the proposed change does prevent the exception from being thrown the current sample app now crashes with the following:
<Label
Text="{Binding StringItemSource, Converter={StaticResource IsListNullOrEmptyConverter}}"
FontAttributes="Bold"
TextColor="Red" />
I think I have a solution which I will submit via PR shortly