Add support for composed implementations of IComponentConnector - #1149
Conversation
|
Seems good to me - although there should probably be some unit testing for it. I suppose this works if we have more than 2 classes in the hierarchy with XAML markup? E.g. BaseClass -> DerivedClass -> FurtherDerivedClass. I don't actually do that but it's probably something that others might do. |
Yes, will work on adding unit tests when time permits. And yes, I tested your scenario of 3 classes in the hierarchy to ensure it was fully general. |
Kenny Kerr (kennykerr)
left a comment
There was a problem hiding this comment.
Looks reasonable. I don't claim to understand all the Xaml fanciness going on here, perhaps Raymond Chen (@oldnewthing) can take a peek.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Did this cause: |
|
Yes, that appears to be it. It seems we'd need to either have a different string for Microsoft.UI.Xaml, or have the current one be able to detect if IComponentConnector2 actually exists in the current namespace. I think the former is better, since WinUI 3 is bound to diverge more over time while system XAML is essentially frozen at this point. Ideally this is a thing that WinUI 3 would ship by itself, but the code makes use of some Scott Jones (@Scottj1s) what approach is the best here? |
fixes #1140
This change adds a helper, ComponentConnectorT, to resolve the breaking change introduced with #1140. Note that this is manual opt-in for the developer, and not an automatic fix, as that would require coordinated work with the Xaml Compiler. I don't think it's worth the effort to do that, for several reasons:
When deriving from a base class with markup, IComponentConnector::Connect has never been reliable outside construction, such as when called back from FindName. If a base class has a property with both the same type (e.g., TextBlock) and connectionId as a derived class, then Connect would erroneously call the derived property setter. The same risk applies to DisconnectUnloadedObject. This was the opposite of the non-virtual dispatch that InitializeComponent was previously relying on. At steady state (outside construction), virtual dispatch would be functional and the derived Connect would always be called. However, unlike with calls from within InitializeComponent, where _dispatch_base can be used to correctly dispatch callbacks, there is no way to know how to do this with Xaml runtime calls for FindName (x:Load and template apply scenarios). The risk exists in both directions - a base property can erroneously override a derived property and vice versa.
For that reason, the Xaml team does not recommend bases with markup, although there is nothing to prevent that or inform the developer of the risks (see related discussion). To preserve the existing (risky) behavior outside construction, _dispatch_base defaults to false to dispatch to the derived class.