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

Setting property of type System.Type in XAML doesn't set the property #4161

Closed
azchohfi opened this issue Feb 11, 2021 · 12 comments
Closed

Setting property of type System.Type in XAML doesn't set the property #4161

azchohfi opened this issue Feb 11, 2021 · 12 comments
Assignees
Labels
area-XamlCompiler product-winui3 WinUI 3 issues team-Markup Issue for the Markup team wct

Comments

@azchohfi
Copy link
Contributor

I've validated that this works fine in OS/XAML:

Steps to reproduce the bug
Latest WinUI3-preview4 (but this is not a regression from preview3, it just never worked in WinUI3)

    public class GraphPresenter : ContentPresenter
    {
        public GraphPresenter()
        {
            Loaded += GraphPresenter_Loaded;
        }

        public Type ResponseType { get; set; }

        private void GraphPresenter_Loaded(object sender, RoutedEventArgs e)
        {
            Debug.WriteLine(ResponseType); // This should have the type, but its null
        }
    }
    public class AnyClass
    {
    }
<local:GraphPresenter ResponseType="local:AnyClass"/>

Works totally fine in OS/XAML and the property is never set in WInUI3.

Expected behavior
Simple C# properties of type System.Type should be populated by setting the property in Xaml.

Version Info
WinUI3 Desktop app with version 3.0.0-preview4.210210.4

NuGet package version:
[Microsoft.WinUI 3.0.0-preview4.210210.4]

Windows app type:

UWP Win32
Yes
Windows 10 version Saw the problem?
Insider Build (xxxxx)
October 2020 Update (19042) Yes
May 2020 Update (19041)
November 2019 Update (18363)
May 2019 Update (18362)
October 2018 Update (17763)
April 2018 Update (17134)
Fall Creators Update (16299)
Creators Update (15063)
Device form factor Saw the problem?
Desktop Yes
Xbox
Surface Hub
IoT

Additional context

@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Feb 11, 2021
@StephenLPeters StephenLPeters added product-winui3 WinUI 3 issues area-XamlCompiler team-Markup Issue for the Markup team labels Feb 12, 2021
@StephenLPeters
Copy link
Contributor

@RealTommyKlein FYI

@RealTommyKlein
Copy link
Contributor

@azchohfi - thanks for the report. To use AnyClass's type in markup, it needs to be marked with the Bindable attribute so the framework can discover it:

    [Microsoft.UI.Xaml.Data.Bindable]
    public class AnyClass
    {
    }

It may work without the Bindable attribute for Debug Windows.UI.Xaml apps, but I'd expect a similar issue or exception from Release Windows.UI.Xaml apps.

@azchohfi
Copy link
Contributor Author

Hey @RealTommyKlein , thanks for the quick reply. That does indeed fixes the issue.
I simplified the problem to be a minimal reproduction, but I see now. @michael-hawker were you aware of this when you developed the GraphPresenter? This is the class I'm referring to:
https://github.com/windows-toolkit/Graph-Controls/blob/main/Microsoft.Toolkit.Graph.Controls/Controls/GraphPresenter/GraphPresenter.cs

If this is not supported, we need to change our strategy there, since we are using those types to parse the Microsoft Graph calls.

@michael-hawker
Copy link
Collaborator

@RealTommyKlein we don't own the underlying type though. It's just a .NET type we're getting from an external library.

The documentation for Bindable says it only applies to C++, and to legacy {Binding} and that all C# classes should be Bindable:

If you're using C++/WinRT, then you need to add the BindableAttribute only if you're using the {Binding} markup extension. If you're using the {x:Bind} markup extension, then you don't need BindableAttribute (for more info, see XAML controls; bind to a C++/WinRT property).

Apply this attribute to C++-based data classes to enable their use as binding sources. Common language runtime (CLR) types, including all types defined in C# and Microsoft Visual Basic, are bindable by default.

We're not even trying to do any Binding here, we're just trying to resolve the Type class to set as the value of the property.

Is this a .NET Native issue with the type being optimized out in Release mode instead? Don't DataTemplates still work with x:DataType, does that do something different?

@RealTommyKlein
Copy link
Contributor

@michael-hawker - the root issue is the Xaml compiler doesn't know to generate type information about AnyClass (or whatever type you're trying to assign in markup), so the Xaml framework can't create the System.Type instance it needs to. Generally the Xaml compiler generates type information in two situations:

  1. The type is used directly in .xaml markup (e.g. a control, or the type of a property assignment done in markup)
  2. The type has the Bindable attribute on it

The Xaml compiler generally doesn't inspect property values themselves, so it doesn't know to generate type info for a property of type System.Type. Even if we did, there are other scenarios where it'd be insufficient (e.g. something like ReponseType={x:Bind OtherTypeProperty}, where the Xaml compiler wouldn't be able to determine the value(s) of OtherTypeProperty which is set in code-behind).

While this isn't a binding scenario, the Bindable attribute would be the best way of solving the issue. If you aren't authors of the type, you could also create a Bindable dummy type with dummy properties of the types you'd need, like so:

    [Microsoft.UI.Xaml.Data.Bindable]
    public class DummyClass
    {
        public AnyClass DummyProp { get; set; }
    }

Which would cause the Xaml compiler to also generate the needed type info for AnyClass.

DataTemplates work with x:DataType since it's only used for x:Bind, and x:Bind doesn't rely on the Xaml compiler's generated type information.

@michael-hawker
Copy link
Collaborator

@RealTommyKlein I've used this before and it was working when I shipped XAML Studio to the store back on Jan 4, 2020. So this seems like a 'recent' regression (we've been moving these prototypes of GraphPresenter and SwitchPresenter which use this pattern forward, and I haven't tried shipping an update to XAML Studio since).

From XAML Studio in my XAML I shipped to the store:

  <future:SwitchPresenter Value="{x:Bind HistoryFilter, Mode=OneWay}" TargetType="models:XamlBindingInfo">

In SwitchPresenter:

        public Type TargetType
        {
            get { return (Type)GetValue(DataTypeProperty); }
            set { SetValue(DataTypeProperty, value); }
        }

        // Using a DependencyProperty as the backing store for DataType.  This enables animation, styling, binding, etc...
        public static readonly DependencyProperty DataTypeProperty =
            DependencyProperty.Register(nameof(TargetType), typeof(Type), typeof(SwitchPresenter), new PropertyMetadata(null));

And the type/model was from a UWP class library (different assembly) with no decorations:

public class XamlBindingInfo : INotifyPropertyChanged

See this post from Jul 29th 2020 where it appears broken at that point: https://stackoverflow.com/questions/47366041/equivalent-of-xtype-in-uwp

@Arlodotexe did you file an issue elsewhere we could link to? FYI @MikeHillberg @azchohfi

@RealTommyKlein
Copy link
Contributor

@michael-hawker - my guess is in your working scenario, XamlBindingInfo is referenced in markup somehow, causing the Xaml compiler to generate the type information for it. You can verify by checking XamlTypeInfo.g.cs in the obj folder after building XAML Studio for any mentions of that XamlBindingInfo class (and if it's working but isn't there, I'd love to dig in deeper 😄 ). I don't believe this is a regression, UWP hasn't ever fully supported setting System.Type from markup, and we probably should've added a compiler warning to indicate that - see this post from 3 years ago which ran into the same limitation: https://stackoverflow.com/a/45176130

@michael-hawker
Copy link
Collaborator

@RealTommyKlein would I see that even in a Debug version of XamlTypeInfo.g.cs? In the last build of XAML Studio I have, I don't see any reference to XamlBindingInfo in XamlTypeInfo.g.cs. Only within my own files, as a DataTemplate Type, and as a DependencyProperty type on a Page. But those are the same cases we were testing recently as well.

I can try rebuilding the version I submitted to the store previously with the current tools, and I can follow-up with you after the long weekend here on Tuesday.

@RealTommyKlein
Copy link
Contributor

@michael-hawker you would need to build it as Release to see the expected codegen in XamlTypeInfo.g.cs, or add this MSBuild property to your project to make the Debug build's outputted codegen match the Release build WRT what's in XamlTypeInfo.g.cs:

<PropertyGroup>
  <EnableTypeInfoReflection>false</EnableTypeInfoReflection>
</PropertyGroup>

If XamlBindingInfo is used as a type for one of your Page's properties (sounds like it is if it's the type of a DependencyProperty), that would cause us to generate the type information for it.

@RealTommyKlein
Copy link
Contributor

I've opened a task for WinUI 3 so if the compiler encounters something like TypeProperty="local:Foo", it will generate type information for Foo so this scenario will work without a dummy class/adding the Bindable attribute.

@RealTommyKlein
Copy link
Contributor

This has been fixed for the upcoming Windows App SDK 1.1 release, closing this issue

@MikeHillberg MikeHillberg added the fixed-internally This bug has been fixed, and the fix will be shipped in the next version of WinUI 3. label Dec 13, 2021
@MikeHillberg MikeHillberg reopened this Dec 13, 2021
@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Dec 13, 2021
@MikeHillberg
Copy link
Contributor

Keeping this open until it's available in a public release

@krschau krschau added fixed-in-WinAppSDK1.1 and removed needs-triage Issue needs to be triaged by the area owners fixed-internally This bug has been fixed, and the fix will be shipped in the next version of WinUI 3. labels Jun 7, 2022
@krschau krschau closed this as completed Jun 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-XamlCompiler product-winui3 WinUI 3 issues team-Markup Issue for the Markup team wct
Projects
None yet
Development

No branches or pull requests

7 participants