Skip to content

Conversation

simonrozsival
Copy link
Member

Description of Change

Produces the following warning:

path/to/document.xaml(1, 2): XamlC warning XC0024: Binding might be compiled incorrectly since the x:DataType annotation comes from an outer scope. Make sure you annotate all DataTemplate XAML elements with the correct x:DataType. See https://learn.microsoft.com/dotnet/maui/fundamentals/data-binding/compiled-bindings for more information.
  • Please feel free to suggest changes to the warning message.
  • The docs will need to be updated to reflect this change if this gets merged

Issues Fixed

Fixes #22714

/cc @StephaneDelcroix @jonathanpeppers @PureWeen @mattleibow

@simonrozsival simonrozsival added the area-xaml XAML, CSS, Triggers, Behaviors label Jun 3, 2024
@simonrozsival simonrozsival requested a review from a team as a code owner June 3, 2024 17:04
Comment on lines +428 to +429
if (n.XmlType.Name == nameof(Microsoft.Maui.Controls.DataTemplate)
&& n.XmlType.NamespaceUri == XamlParser.MauiUri)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any other types of scopes we would need future changes for?

Wondering if any of these have an issue:

  • ResourceDictionary
  • ControlTemplate

In theory, you could have a {Binding} inside a Style in a {ResourceDictionary} and the type could be different based on the XAML using the Style? Maybe the existing warning already works for this case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this applies to ResourceDictionary (BTW we're not compiling these bindings at the moment: #22023), but it might apply to ControlTemplate 🤔 In general, this should be whenever there's a different BindingContext in the outer scope and in the inner scope. What do you think, @StephaneDelcroix?

jonathanpeppers
jonathanpeppers previously approved these changes Jun 4, 2024
Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the other cases we can check in the future.

@rmarinho
Copy link
Member

rmarinho commented Jun 4, 2024

Sorry needs rebase

@simonrozsival simonrozsival modified the milestone: 9.0-preview5 Jun 4, 2024
ivanpovazan
ivanpovazan previously approved these changes Jun 4, 2024
@simonrozsival simonrozsival dismissed stale reviews from ivanpovazan and jonathanpeppers via ea44d26 June 4, 2024 15:41
@simonrozsival
Copy link
Member Author

@rmarinho @jonathanpeppers do you think we can go ahead with this PR? There are currently no merge conflicts.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-xaml XAML, CSS, Triggers, Behaviors fixed-in-9.0.0-preview.6.24327.7
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants