Make ICustomPropertyProvider AOT safe#1677
Conversation
| { | ||
| for (var curSymbol = symbol; curSymbol != null; curSymbol = curSymbol.BaseType) | ||
| { | ||
| foreach (var propertySymbol in curSymbol.GetMembers(). |
There was a problem hiding this comment.
Note: we should also make sure this works correctly with partial properties. We can handle this in a follow up.
There was a problem hiding this comment.
We are using symbol data to get this data, so I believe it should. But we can confirm after.
There was a problem hiding this comment.
I was more worried about the fact you might see two different property symbols for the same member (one being the definition, one being the implementation part), and that the fact they'd be "duplicate" might cause problems.
| if (!RuntimeFeature.IsDynamicCodeCompiled) | ||
| { | ||
| throw new NotSupportedException( | ||
| $"ICustomProperty support used by XAML binding for '{target.GetType()}' requires the type to marked with 'WinRT.BindableCustomPropertyAttribute'. " + |
There was a problem hiding this comment.
any plans to document this in greater detail, like with an md on the repo? Xaml Compiler could then point errors/warnings to that page.
There was a problem hiding this comment.
I can get our aot.md updated to mention this in a follow up PR.
| WF::IInspectable const& bindableObject, | ||
| hstring property, | ||
| Windows::UI::Xaml::Interop::TypeName const& indexerType, | ||
| bool validateOnlyExists, |
There was a problem hiding this comment.
enums (or name:value syntax) would be easier to read at call sites
| return true; | ||
| } | ||
|
|
||
| if (customProperty.Name() != property || |
There was a problem hiding this comment.
what's the purpose of passing these bools in? given they have to match customProperty, why not just use the latter? (they seem redundant)
There was a problem hiding this comment.
This is validating expected vs actual. I will have a follow up PR to address this along with your enum comment for easier readability.
BindableCustomPropertyAttributeto allow to specify that public properties from the class are bindable. Either all public properties can be bindable by passing no arguments to the constructor. Or the names of the properties and the types of the indexer types can be passed to specify which ones to make bindable.Microsoft.UI.Xaml.Data.IBindableCustomPropertyImplementationgets implemented on that class via the source generator which provides the implementation to supportICustomPropertyProvider. This is done as an interface instead of an attribute because this allows to support generic types while the attribute model won't work with it. In addition, we only need to worry about supporting classes and not other types.