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

Proposal: Add generic globbing support for .xaml files #12520

Open
stevenbrix opened this issue Jul 16, 2020 · 7 comments
Open

Proposal: Add generic globbing support for .xaml files #12520

stevenbrix opened this issue Jul 16, 2020 · 7 comments

Comments

@stevenbrix
Copy link

stevenbrix commented Jul 16, 2020

Now that other XAML based UI frameworks are moving to .NET5, there needs to be a generic way for the globbing mechanism that keeps the .csproj clean that all the frameworks can use. Since the frameworks themselves don't share the same compiler toolchain, and the frameworks aren't generally compatible (integration between WinUI and WPF is in its infancy), what's added here shouldn't dictate how those projects build. It should be simple and generic, and something that WinUI, WPF, MAUI, Avalonia, and Uno can all build off of.

Proposal A: .NET SDK adds items to Page group

In this proposal, the .NET sdk globs xaml files into the existing Page item group. It doesn't know anything about the ApplicationDefinition, and nor should it, because that logic differs between Xaml frameworks. For example, WinUI always uses App.xaml as the file name, even for VB projects. The .NET SDK uses the value of the DefaultXamlFramework and places that on each Page item in the XamlFramework metadata. The XamlFramework metadata is read by the necessary toolchains to understand which Xaml framework the file belongs to.

(new) Microsoft.NET.Sdk.Xaml.targets:

<PropertyGroup>
  <EnableDefaultPageItems Condition="'$(EnableDefaultPageItems)'==''">true</EnableDefaultPageItems>
</PropertyGroup>

<ItemGroup Condition="'$(EnableDefaultPageItems)' == 'true'">
   <Page Include="**/*.xaml"
              Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder)">
      <XamlFramework>$(DefaultXamlFramework)</ XamlFramework >
      <Generator>MSBuild:Compile</Generator>
  </Page>
   <None Remove="**/*.xaml"/>
</ItemGroup>

Microsoft.NET.Sdk.WindowsDesktop.targets:

  <PropertyGroup>
      <DefaultXamlFramework Condition="'$(DefaultXamlFramework)'==''">Wpf</DefaultXamlFramework>
      <EnableDefaultApplicationDefinition Condition="'$(EnableDefaultApplicationDefinition)'==''">true</EnableDefaultApplicationDefinition>
  </PropertyGroup>
  <ItemGroup Condition=" '$(_EnableWindowsDesktopGlobbing)' == 'true' "> 
     <ApplicationDefinition Include="@(Page)" 
                            Condition="'%(Identity)'=='App.xaml' and '$(EnableDefaultApplicationDefinition)' != 'false' And Exists('$(MSBuildProjectDirectory)/App.xaml') And '$(MSBuildProjectExtension)' == '.csproj'" /> 
     <ApplicationDefinition Include="@(Page)" 
                            Condition="'%(Identity)'=='Application.xaml' and '$(EnableDefaultApplicationDefinition)' != 'false' And Exists('$(MSBuildProjectDirectory)/Application.xaml') And '$(MSBuildProjectExtension)' == '.vbproj'"/>
     <Page Remove="@(ApplicationDefinition)" />
   </ItemGroup> 

   <!-- An example of how WPF pages are determined to pass to the WPF markup compiler -->
   <ItemGroup>
     <WpfPages Include="@(Page)" Condition="'%(XamlFlavor)'=='Wpf'"/>
   </ItemGroup>
   <Target Name="MarkupCompile Inputs="@(WpfPages)" >
      ...
   </Target>

Microsoft.WinUI.NET.Markup.Compiler.targets:

  <PropertyGroup>
      <DefaultXamlFramework Condition="'$(DefaultXamlFramework)'=='' and $(UseWpf)'!='true'">WinUI</DefaultXamlFramework>
      <EnableDefaultApplicationDefinition Condition="'$(EnableDefaultApplicationDefinition)'==''">true</EnableDefaultApplicationDefinition>
      <!-- In a WPF app, don't create default WinUI application definition -->
      <EnableDefaultWinUIApplicationDefinition Condition="'$(EnableDefaultWinUIApplicationDefinition)'=='' and '$(UseWpf)'=='true'">false</EnableDefaultWinUIApplicationDefinition >
  </PropertyGroup>
  <ItemGroup Condition=" '$(_EnableWinUI)' == 'true' "> 
    <ApplicationDefinition Include="@(Page)" 
                            Condition="'%(Identity)'=='App.xaml' and '$(EnableDefaultWinUIApplicationDefinition)' != 'false' And Exists('$(MSBuildProjectDirectory)/App.xaml')"/>
  
     <Page Remove="@(ApplicationDefinition)" />
   </ItemGroup> 

   <!-- An example of how WinUI pages are determined to pass to the WinUI markup compiler -->
   <ItemGroup>
     <WinUIPages Include="@(Page)" Condition="'%(XamlFlavor)'=='WinUI'"/>
   </ItemGroup>
   <Target Name="MarkupCompile Inputs="@(WinUIPages)" >
      ...
   </Target>

Proposal B: .NET SDK has new Xaml item group.

This proposal is very similar to A, but instead of the .NET Sdk placing items in the Page ItemGroup, it uses the Xaml item group instead. The WindowsDesktop SDK and WinUI would grab the xaml items out of this group, and place them in the Page and ApplicationDefinition group as they need. The way that the DefaultXamlFramework property works is identical as described in Proposal A.

(new) Microsoft.NET.Sdk.Xaml.targets:

<PropertyGroup>
  <EnableDefaultXamlItems Condition="'$(EnableDefaultXamlItems)'==''">true</EnableDefaultXamlItems>
</PropertyGroup>

<ItemGroup Condition="'$(EnableDefaultXamlItems)' == 'true'">
    <Xaml Include="**/*.xaml" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder)">
     <XamlFramework="$(DefaultXamlFramework)>
   </Xaml>
    <None Remove="**/*.xaml"/>
</ItemGroup>

Microsoft.NET.Sdk.WindowsDesktop.targets:

  <PropertyGroup>
      <DefaultXamlFramework Condition="'$(DefaultXamlFramework)'==''">Wpf</DefaultXamlFramework>
      <EnableDefaultPageItems Condition="'$(EnableDefaultPageItems)'==''">true</EnableDefaultPageItems>
      <EnableDefaultApplicationDefinition Condition="'$(EnableDefaultApplicationDefinition)'==''">true</EnableDefaultApplicationDefinition>
  </PropertyGroup>
  <ItemGroup Condition=" '$(_EnableWindowsDesktopGlobbing)' == 'true' "> 
     <ApplicationDefinition Include="@(Xaml)" 
                            Condition="'%(Identity)'=='App.xaml' and '$(EnableDefaultApplicationDefinition)' != 'false' And Exists('$(MSBuildProjectDirectory)/App.xaml') And '$(MSBuildProjectExtension)' == '.csproj'">
         <Generator>MSBuild:Compile</Generator>
     </ApplicationDefinition> 
     <ApplicationDefinition Include="@(Xaml)" 
                            Condition="'%(Identity)'=='Application.xaml' and '$(EnableDefaultApplicationDefinition)' != 'false' And Exists('$(MSBuildProjectDirectory)/Application.xaml') And '$(MSBuildProjectExtension)' == '.vbproj'">
       <Generator>MSBuild:Compile</Generator>
     </ApplicationDefinition> 
     <Page Include="@(Xaml)" Exclude="@(ApplicationDefinition) />
   </ItemGroup> 

   <!-- An example of how WPF pages are determined to pass to the WPF markup compiler -->
   <ItemGroup>
     <WpfPages Include="@(Page)" Condition="'%(XamlFlavor)'=='Wpf'"/>
   </ItemGroup>
   <Target Name="MarkupCompile Inputs="@(WpfPages)" >
      ...
   </Target>

Microsoft.WinUI.NET.Markup.Compiler.targets:

  <PropertyGroup>
      <DefaultXamlFramework Condition="'$(DefaultXamlFramework)'=='' and $(UseWpf)'!='true'">WinUI</DefaultXamlFramework>
      <EnableDefaultApplicationDefinition Condition="'$(EnableDefaultApplicationDefinition)'==''">true</EnableDefaultApplicationDefinition>
      <!-- In a WPF app, don't create default WinUI application definition -->
      <EnableDefaultWinUIApplicationDefinition Condition="'$(EnableDefaultWinUIApplicationDefinition)'=='' and '$(UseWpf)'=='true'">false</EnableDefaultWinUIApplicationDefinition >
  </PropertyGroup>
  <ItemGroup Condition=" '$(_EnableWinUIGlobbing)' == 'true' "> 
   <ApplicationDefinition Include="@(Xaml)" 
                            Condition="'%(Identity)'=='App.xaml' and '$(EnableDefaultWinUIApplicationDefinition)' != 'false' And Exists('$(MSBuildProjectDirectory)/App.xaml')">
         <Generator>MSBuild:Compile</Generator>
     </ApplicationDefinition> 
  
     <Page Include="@(Xaml)" Exclude="@(ApplicationDefinition)"/>
   </ItemGroup> 

   <!-- An example of how WinUI pages are determined to pass to the WinUI markup compiler -->
   <ItemGroup>
     <WinUIPages Include="@(Page)" Condition="'%(XamlFlavor)'=='WinUI'"/>
   </ItemGroup>
   <Target Name="MarkupCompile Inputs="@(WinUIPages)" >
      ...
   </Target>

Forward thinking

What happens when we want to merge WinUI and WPF into the same project? If we look at the user scenario for this, this is the scenario of a WPF app using Xaml Islands. If I'm using Visual Studio in my WPF project, and add a WinUI item, then VS should make the following change to the .csproj:

<ItemGroup>
   <Page Update="WinUIUserControl.xaml" XamlFramework="WinUI" />
</ItemGroup>

This will ensure the existing tooling properly recognizes the page as being a WinUI page, rather than a WPF page.

I don't have a strong preference for which one, I prefer the .NET sdk having a new item group, but I'm not sure if VS would understand that if our VSIX adds .xaml files to the Page item group.

/cc @ryalanms @dsplaisted @jkoritzinsky @davkean

Related Issues:
dotnet/wpf#3245
microsoft/microsoft-ui-xaml#2498

@stevenbrix stevenbrix changed the title Add generic globbing support for .xaml files Proposal: Add generic globbing support for .xaml files Jul 16, 2020
@stevenbrix
Copy link
Author

stevenbrix commented Aug 6, 2020

@dsplaisted after our meeting earlier this week, I've updated the proposal to match what we discussed.

This comment was largely deleted and the proposal was grouped in with the original comment in this issue

@stevenbrix
Copy link
Author

stevenbrix commented Aug 6, 2020

One benefit of using the DefaultXamlFramework property, is that a lot of VS code is still dependent on Project level properties. So this lets us design the tooling (language service, compiler, designer, etc) properly to look at the specific item metadata, while also not requiring a large refactor of other parts of the Visual Studio code base. Then down the road when we fully support intermixing WinUI and WPF in the same project (if we ever do), we can make whatever changes necessary there to use the MSBuild item metadata. I feel confident with this approach that we aren't backing ourselves into any corners.

@jkoritzinsky
Copy link
Member

I like that idea. That would enable Avalonia to opt in easily and remove a lot of our tooling troubles.

@chabiss
Copy link

chabiss commented Aug 7, 2020

Can confuse users as to what is the difference between "DefaultXamlFramework" and "TargetFramework"? We used to reserve the word "Framework" for .Net. Maybe we should use instead XamlPlatform or maybe better XamlRuntimePlatform?

@dsplaisted
Copy link
Member

Overall I like and am supportive of this.

However, the details may be tricky to get right.

For example, the globs are currently (and should probably continue to be) defined in .props files instead of .targets files. This allows them to be modified in the body of the project file (for example updating them to add metadata, or removing them from one item and adding to another one). For the example of updating the XamlFramework metadata to work, the default Page globs would need to happen beforehand, ie in the .props files.

<ItemGroup>
   <Page Update="WinUIUserControl.xaml" XamlFramework="WinUI" />
</ItemGroup>

Even though they are in props files, because of the multiple phases of MSBuild evaluation, you can have items in the .props files that are conditioned on properties set in the body of the project or in .targets files. See here for some discussion of this: https://github.com/dotnet/designs/blob/master/accepted/2020/workloads/workload-resolvers.md#workload-props-files

Another tricky thing will be the mechanics of changing the targets in dotnet/sdk and the WindowsDesktop targets which are in dotnet/wpf in sync.

I generally prefer proposal A to proposal B.

Would it be reasonable to not set the XamlFramework metadata on Page items normally? IE the XamlFramework would only need to be set on an item if it was for something else than the DefaultXamlFramework.

I like XamlFramework better than XamlFlavor which I had originally suggested, but @chabiss makes a good point that it conflicts semantically with TargetFramework. So maybe there's a better term we can use.

@stevenbrix
Copy link
Author

Can confuse users as to what is the difference between "DefaultXamlFramework" and "TargetFramework"?

@chabiss that's a fair point. I'm a fan of either XamlPlatform or XamlRuntime over XamlRuntimePlatform, but I also just prefer less characters :)

For example, the globs are currently (and should probably continue to be) defined in .props files instead of .targets files. This allows them to be modified in the body of the project file (for example updating them to add metadata, or removing them from one item and adding to another one).

@dsplaisted makes sense 👍

Would it be reasonable to not set the XamlFramework metadata on Page items normally? IE the XamlFramework would only need to be set on an item if it was for something else than the DefaultXamlFramework.

Can you share an example so I can better understand what you mean? I'm not quite sure I understand the benefit of doing it that way.

Another tricky thing will be the mechanics of changing the targets in dotnet/sdk and the WindowsDesktop targets which are in dotnet/wpf in sync.

One benefit of a new item group is that it would be simpler to coordinate changes. The changes in dotnet/sdk can happen and we can update dotnet/wpf at our own pace w/o worrying about conflicting changes.

If we went with Proposal A, I'd imagine that the WPF build will break once it tries to consume the .NET SDK that has this default globbing, and then we'll fix WPF and wait for a coherent build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants
@dsplaisted @jkoritzinsky @stevenbrix @chabiss and others