Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[XC] Produce warning when x:DataType is inherited from outer scope of DataTemplate #22803

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
Copy link
Member Author

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

@simonrozsival simonrozsival merged commit 4ca773f into dotnet:net9.0 Jun 21, 2024
50 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 22, 2024
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