microsoft / microsoft-ui-xaml

Windows UI Library: the latest Windows 10 native controls and Fluent styles for your applications
MIT License
6.28k stars 675 forks source link

Lack of validation for XAML Style BasedOn & TargetType properties #2476

Open mrlacey opened 4 years ago

mrlacey commented 4 years ago

Describe the bug

There is no validation (in the designer/language service or at compile time) to confirm that a style is based on an appropriate type. Without this check developers do not know what is wrong until running the app and experiencing a XamlParseException with very limited details.

Steps to reproduce the bug

Steps to reproduce the behavior:

  1. Create a new, blank app
  2. Add the following to MainPage.xaml
<Page.Resources>
        <Style
            x:Key="MyTextBlockStyle"
            BasedOn="{StaticResource MyButtonStyle}"
            TargetType="TextBlock">
            <Setter Property="Foreground" Value="Red" />
        </Style>
        <Style x:Key="MyButtonStyle" TargetType="Button">
            <Setter Property="Background" Value="Aqua" />
        </Style>
    </Page.Resources>
    <Grid>
        <TextBlock Style="{StaticResource MyTextBlockStyle}" />
    </Grid>
  1. Run the app.
  2. See it crash.

Expected behavior

Screenshots

Version Info

NuGet package version:

This is a WinUI3 issue relating to code that's not public (yet?) It is tied to the XAML language service as used within VS, not the shipping libraries

Windows 10 version Saw the problem?
Insider Build (xxxxx) Yes/No?
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
Mobile
Xbox
Surface Hub
IoT

Additional context This was originally reported to the XAML team but they claim it is not their issue.

stevenbrix commented 3 years ago

@mrlacey can you confirm if this issue repros in WinUI2.x? It sounds like from your issue this is WinUI3.0 only, but I just want to confirm.

mrlacey commented 3 years ago

@stevenbrix Yes, the initial repro was for UWP (WinUI 2)

With WinUI3 (preview 3), the same exception is still thrown but a better error message is included: Can only base on a Style with target type that is base type of this style's target type.

Ideally it would still be better to see this issue at compile or design time.

stevenbrix commented 3 years ago

Thanks @mrlacey. I agree that the designer should provide a better error message here. Thanks for reporting this!

@RealTommyKlein for FYI - it looks like we are missing some test cases here.

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 5 days.

mrlacey commented 1 year ago

keep open