dotnet / maui

.NET MAUI is the .NET Multi-platform App UI, a framework for building native device applications spanning mobile, tablet, and desktop.
https://dot.net/maui
MIT License
22.21k stars 1.75k forks source link

XamlC XC0045 on Reference Bindings pointed to XAML #23711

Closed daltzctr closed 1 month ago

daltzctr commented 3 months ago

Description

It's quite common to wire together XAML items together like the following

<ContentView
    x:Class="MyApp.Views.MyView"
    xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
    xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
    xmlns:models="clr-namespace:phoenix_tuner_x.Models"
    xmlns:vm="clr-namespace:MyApp.ViewModels."
    x:Name="root"
    x:DataType="vm:MyViewModel">
    <CollectionView ItemsSource="{Binding Items}">
        <CollectionView.ItemTemplate>
           <DataTemplate x:DataType=models:MyItem>
              <Button Text="{Binding Name}" Command="{Binding Source={x:Reference root}, Path=BindingContext.MyCommand}"
           </DataTemplate>
        </CollectionView.ItemTemplate>
    </CollectionView>
</ContentView>

with the introduction of #20610, this broke as all bindings need to be compiled now. Great. Let's just declare a DataType as recommended by @simonrozsival.

<Button Text="{Binding Name}" Command="{Binding Source={x:Reference root}, Path=BindingContext.MyCommand, x:DataType=ContentView}"

but oh no

C:\MyApp\Views\MyView.xaml(33,47): XamlC error XC0045: Binding: Property "MyCommand" not found on "System.Object".

Steps to Reproduce

Simply create a binding that binds to the property of another XAML item (such as ContentView), then try to explicitly declare DataType

Link to public reproduction project repository

No response

Version with bug

9.0.0-preview.6.24327.7

Is this a regression from previous behavior?

Yes, this used to work in .NET MAUI

Last version that worked well

9.0.0-preview.3.10457

Affected platforms

iOS, Android, Windows, macOS

Affected platform versions

No response

Did you find any workaround?

Explicitly declare null datatype inside of binding.

<Button Text="{Binding Name}" Command="{Binding Source={x:Reference root}, Path=BindingContext.MyCommand, x:DataType={x:Null}}"/>

Relevant log output

No response

github-actions[bot] commented 3 months ago

Hi I'm an AI powered bot that finds similar issues based off the issue title.

Please view the issues below to see if they solve your problem, and if the issue describes your problem please consider closing this one and thumbs upping the other issue to help us prioritize it. Thank you!

Open similar issues:

Closed similar issues:

Note: You can give me feedback by thumbs upping or thumbs downing this comment.

simonrozsival commented 3 months ago

@daltzctr thanks for the additional context. The problem in this case is that BindingContext is of type object and you need to cast it. Unfortunately, we don't support casting in compiled bindings which is something we should look into (cc @StephaneDelcroix).

To overcome this issue, you need to use RelativeSource instead of Reference:

<Button Text="{Binding Name}" Command="{Binding Source={RelativeSource Mode=FindAncestorBindingContext, AncestorType={x:Type vm:MyViewModel}}, Path=MyCommand, x:DataType=vm:MyViewModel}" />

You can find more information in the docs.

Does this solve your problem?

formerlymisterhenson commented 3 months ago

Quote: "all bindings need to be compiled now" Is there any further reading on the reasoning for this big breaking change it would be?

nor0x commented 3 months ago

@daltzctr thanks for the additional context. The problem in this case is that BindingContext is of type object and you need to cast it. Unfortunately, we don't support casting in compiled bindings which is something we should look into (cc @StephaneDelcroix).

To overcome this issue, you need to use RelativeSource instead of Reference:

<Button Text="{Binding Name}" Command="{Binding Source={RelativeSource Mode=FindAncestorBindingContext, AncestorType={x:Type vm:MyViewModel}}, Path=MyCommand, x:DataType=vm:MyViewModel}" />

You can find more information in the docs.

Does this solve your problem?

I have applied this workaround in my XAML which causes following Exception on Windows

System.InvalidOperationException
Operation is not valid due to the current state of the object.

   at Microsoft.Maui.Controls.Binding.ApplyRelativeSourceBinding(RelativeBindingSource relativeSource, BindableObject targetObject, BindableProperty targetProperty, SetterSpecificity specificity)
   at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__128_0(Object state)
   at Microsoft.UI.Dispatching.DispatcherQueueSynchronizationContext.<>c__DisplayClass2_0.<Post>b__0()
Axemasta commented 3 months ago

As @nor0x mentioned, this has been a HUGE issue for the MCT TouchBehaviour. We are telling people to use x:Reference because the RelativeSource just doesn't work. I haven't actually seen a working example of it in Maui yet (it DID work in forms...), .NET 9 might have quite a few pain points when upgrading from 8 if we don't look into exactly why this happens!

codeaphex commented 3 months ago

I hope this ticket gets the attention it needs? Seems kinda serious, if this gets unfixed into the final .NET 9, right?

simonrozsival commented 3 months ago

all bindings need to be compiled now

@daltzctr That's not accurate. XamlC will try to compile the binding IF there is an x:DataType available. In this case, there is a data type inherited from its parents (x:DataType="vm:MyViewModel"). If you want to opt out of the compilation, you can unset the data type via {Binding ..., x:DataType={x:Null}}. You will get a warning from XamlC which you can choose to ignore or suppress.

We are telling people to use x:Reference because the RelativeSource just doesn't work. I haven't actually seen a working example of it in Maui yet (it DID work in forms...)

@Axemasta Relative source work just fine. You can see several examples in MAUI's unit tests (https://github.com/dotnet/maui/blob/main/src/Controls/tests/Core.UnitTests/RelativeSourceBindingTests.cs) and I also validated the binding in my previous comment and it also works correctly. If you find any issues with relative sources, please report it so we can fix it.

I agree that using x:Reference is more straightforward. In the cases where it's necessary to use a cast in the middle of a binding path, I think there are currently several options:

Is there any further reading on the reasoning for this big breaking change it would be?

@formerlymisterhenson The motivation to add support for compilation of bindings with explicitly set sources is to support trimming (see #18658 and #20610). When bindings aren't compiled, the trimmer doesn't see which properties will be needed at runtime and it can trim them. As a side benefit, compiled bindings improve performance.


@StephaneDelcroix @Redth @jonathanpeppers @davidortinau I have some suggestions how to reduce the friction to the new behavior:

Axemasta commented 3 months ago

@simonrozsival would this issue be a good place to put a replication of the issue we are seeing in the MCT touch effect. Basically we cannot use relative source as it crashes with a similar stack trace to the original issue.

I understand it's not a Maui control but we haven't been able to understand why it's breaking as it should work (the same did in xamarin). I can post a basic reproduction and happy to answer questions and help fix any issues, although they're possibly on our end!

simonrozsival commented 3 months ago

@Axemasta could you open a new issue? I think it would be better to keep those separate.

symbiogenesis commented 2 months ago

Can a Reference binding really be to any object, or will it always be to an Element? I have never seen it tied to something that is not an Element.

If the compiler always assumes that it is an Element, then this would be enough to provide access to the BindingContext, irrespective of whether casting is ever added as a feature.

simonrozsival commented 2 months ago

If the compiler always assumes that it is an Element, then this would be enough to provide access to the BindingContext, irrespective of whether casting is ever added as a feature.

The problem is not finding BindingContext itself. The problem is the next step: BindingContext is of type object and the compiler is looking for a property, in this case MyCommand, on object and it can't find one. To make this work, we would need something like this:

Command="{Binding Source={x:Reference root}, Path=(BindingContext as vm:MyViewModel).MyCommand}"
symbiogenesis commented 2 months ago

I see. The FindAncestorBindingContext mode for relative binding is kind of nice. Maybe something similar for Reference binding could be added.

Here is one possible syntax:

Command="{Binding Source={x:BindingContextReference root}, Path=MyCommand}"

It would know that the BindingContext is vm:MyViewModel because it would be annotated via x:DataType and throw a compile-time exception if not.

MartyIX commented 2 months ago

Today I tested the .NET 9p7 and I got an error on

<HorizontalStackLayout IsVisible="{Binding Source={x:Reference toggleAll}, Path=IsVisible}">
   <Button x:Name="toggleAll" Text="Hello"/>
</HorizontalStackLayout>

it seems that x:Reference is not taken into account at all as the error mentions:

XamlC error XC0045: Binding: Property "IsVisible" not found on "Project.MyViewModel".

(the view model is indeed set by x:DataType="vm:MyViewModel)

edit: It looks that my issue is explained here https://github.com/dotnet/maui/issues/23711#issuecomment-2283918930. Still it would be great if it were working out of the box.

simonrozsival commented 2 months ago

@MartyIX I opened a tracking issue to implement the Source type inference here: https://github.com/dotnet/maui/issues/21834. Feel free to add any feedback or comments to that thread.

maonaoda commented 2 months ago

So, what should I do with the current net9, keep the code as is and wait for https://github.com/dotnet/maui/pull/24238 to be merged? BindingContext="{Binding Source={RelativeSource TemplatedParent},Path=BindingContext}"

Command="{Binding Source={x:Reference XXXXPageViewModel}, Path=BindingContext.ItemTappedCommand}"


I use quite a lot of x:References, and changing them all would be a disaster.

simonrozsival commented 2 months ago

@maonaoda right now, if you want to make sure that a binding is not compiled, set x:DataType={x:Null} like this:

Command="{Binding Source={x:Reference XXXXPageViewModel}, Path=BindingContext.ItemTappedCommand, x:DataType={x:Null}}"

You can then choose to ignore this warning by including the warning code in your project file:

<WarningsNotAsErrors>$(WarningsNotAsErrors);XC0023</WarningsNotAsErrors>
maonaoda commented 2 months ago

@simonrozsival thanks, x:DataType={x:Null} works fine. Is this just to avoid having to perform binding checks at compile time?

However, when I simply add <WarningsNotAsErrors>$(WarningsNotAsErrors);XC0045</WarningsNotAsErrors>, the errors are not ignored as expected.

simonrozsival commented 2 months ago

Is this just to avoid having to perform binding checks at compile time?

When the data type isn't known at compile time, the binding cannot be compiled. In that case, we produce a warning and fall back to a reflection-based binding the same way it is in .NET 8.

However, when I simply add $(WarningsNotAsErrors);XC0045, the errors are not ignored as expected.

Hmm, you might also need to set <NoWarn>...</NoWarn>. I'm not able to run that code myself at the moment unfortunately so I'll try to get back to you later if you can't figure it out yourself by then.

QianaJiao commented 2 months ago

I can repro this issue on the latest 17.12.0 Preview 1.0 (9.0.0-preview.7.24407.4) with the sample https://github.com/CommunityToolkit/Maui . and it does not repro on 9.0.0-preview.2.10293 image

AlleSchonWeg commented 2 months ago

@daltzctr thanks for the additional context. The problem in this case is that BindingContext is of type object and you need to cast it. Unfortunately, we don't support casting in compiled bindings which is something we should look into (cc @StephaneDelcroix).

To overcome this issue, you need to use RelativeSource instead of Reference:

<Button Text="{Binding Name}" Command="{Binding Source={RelativeSource Mode=FindAncestorBindingContext, AncestorType={x:Type vm:MyViewModel}}, Path=MyCommand, x:DataType=vm:MyViewModel}" />

You can find more information in the docs.

Does this solve your problem?

Hi, i tried this with a button inside a DataTemplate. But i receive warnings in .Net 8:

[0:] Microsoft.Maui.Controls.Xaml.Diagnostics.BindingDiagnostics: Warning: Mismatch between the specified x:DataType and the current binding context

it works but i don't know how to set x:DataType correctly. The only thing helped is to set DataType to null:

<DataTemplate  x:DataType="{x:Null}">
    <Button Text="{Binding Name}" Command="{Binding Source={RelativeSource Mode=FindAncestorBindingContext, AncestorType={x:Type vm:MyViewModel}}, Path=MyCommand, x:DataType=vm:MyViewModel}" />
</DataTemplate>
DDHSchmidt commented 1 month ago

It feels like this issue was taken for granted as "collateral damage", so that everyone could happily announce they finally reached trimming completeness I implore the contributors to also take developer experience into consideration. Having to omit compiled bindings in a ton of places doesn't feel right and also opens the door for new runtime errors ☹

simonrozsival commented 1 month ago

@daltzctr @DDHSchmidt I opened a PR that would make the new compilation optimization that caused problems for you optional. Please feel free to leave feedback on the PR: https://github.com/dotnet/maui/pull/24924

DDHSchmidt commented 1 month ago

@daltzctr @DDHSchmidt I opened a PR that would make the new compilation optimization that caused problems for you optional. Please feel free to leave feedback on the PR: #24924

Thanks, as I understood it enabling this switch will default back to the .Net8 mode of interpreting the Source-Bindings as before. That should help my case, yes. What is unclear to me (and sorry if that was already mentioned, it's pretty late here already), if I ever were to try and go full AOT, will Source-Bindings (x:Reference, RelativeSource, whatever) ever be supported or should we discard that approach in favour of another approach? And if yes: What is the preferred alternative?

simonrozsival commented 1 month ago

@DDHSchmidt thanks for the question. I'll try to summarize the problem and the possible fixes and workarounds.

as I understood it enabling this switch will default back to the .Net8 mode of interpreting the Source-Bindings as before.

Once #24924 is merged, the compiler will behave the same way it did in .NET 8. In other words, the optimization is NOT enabled by default. That should make it easier to adopt .NET 9 for you. If we can get the PR merged today, it will get into RC2 so you should be able to soon test it out. Thanks everyone for your feedback in this thread!

The optimization will be enabled if you explicitly set a feature switch in your project file by adding < MauiEnableXamlCBindingWithSourceCompilation>true</MauiEnableXamlCBindingWithSourceCompilation>, or when NativeAOT is enabled by <PublishAot>true</PublishAot> or when trim mode is set to full.

What is unclear to me (and sorry if that was already mentioned, it's pretty late here already), if I ever were to try and go full AOT, will Source-Bindings (x:Reference, RelativeSource, whatever) ever be supported or should we discard that approach in favour of another approach?

When the optimization is enabled, source-bindings are supported in .NET 9. For example, this scenario will be compiled just fine:

<Entry x:Name="Name" />
<Label Text="{Binding Text, Source={x:Reference Name}}" x:DataType="Entry" />

There are just two potential issues you need to keep in mind when enabling it:

Please let me know if anything is unclear.