dotnet / wpf

WPF is a .NET Core UI framework for building Windows desktop applications.
MIT License
7.03k stars 1.16k forks source link

Add globbing support for WinUI #3245

Open stevenbrix opened 4 years ago

stevenbrix commented 4 years ago

The WinUI team needs support for globbing support so that Visual Studio will not edit the .csproj whenever you add a .xaml file to the project. See this bug as a reference: https://github.com/microsoft/microsoft-ui-xaml/issues/2498

In following the same pattern as WPF, we can add a UseWinUI property that will pull in the Microsoft.NET.WindowsDesktop.Sdk and enable the globbing support.

If both UseWinUI and UseWpf are true, then the SDK can assume all xaml files are for WPF. The WinUI nuget will respect the use of UseWpf and not invoke the winui xaml compiler if that is the case. If only UseWinUI is true, then the SDK should not invoke PBT. Below is a table describing the different combinations and what should happen:

UseWpf UseWinUI Xaml Compiler invoked
True True PBT
True False PBT
False True WinUI Compiler
False False neither
vatsan-madhavan commented 4 years ago

Should all globbing support live in the base SDK instead of living in the WindowsDesktop SDK? This may not work well if WPF's targets are deciding which markup compiler to invoke, but perhaps WPF's targets should not be deciding that anyway. IIRC WPF's targets are making local decisions for itself, based on UseWpf==true. IMHO other compilers should do so based on their own rules. Why do these rules have to be all coordinated in the same place anyway?

In .NET Framework/old-style projects, when there was no use of UseWpf, UseWinUI etc., *.xaml files were implicitly inferred to be WPF, UWP etc. These inferences required heuristic judgment. We did away with these heuristics and decided that the projects/developers had to explicitly state the intended XAML dialect being used by means these properties. This in turn disambiguates the XAML targets/compiler etc. Given this design, is there even a need for a central point-of-coordination based on a matrix of these property values? If both UseWpf and UseWinUI are set to true for the same XAML file, then both compilers would be invoked and things won't work out - but that's just a "developer beware" scenario that we can mitigate with warnings from the base SDK.

https://github.com/microsoft/microsoft-ui-xaml/issues/2498#issuecomment-631781043

We were originally planning on making an MSBuild SDK, but then didn't because we heard that the Microsoft.NET.Sdk.WindowsDesktop was going away. Is this still the plan?

BTW, does WinUI have its own SDK yet? If it does, you can set up globbing etc. in that SDK's props/targets. OTOH, if the plan is never to have an SDK (for e.g., perhaps because WinUI is going to ship as a first-class component in .NET 5, and taking a clue from WindowsDesktop SDK's merger with the base SDK, has now decided not to invent a separate SDK for itself), then a better place to make these changes would be base Microsoft.NET.SDK.

BTW, should Avalonia be rationalized here as well? https://github.com/AvaloniaUI/Avalonia/issues/4102.

Related: https://github.com/microsoft/microsoft-ui-xaml/issues/1438 /cc @grokys, @dsplaisted , @jkoritzinsky

stevenbrix commented 4 years ago

We aren't planning on having an MSBuild SDK.

I don't know the timeframe, or if WinUI will be a first-class component of .NET. Definitely not in the .NET5 timeframe, but maybe for .NET6 when we can be an optional component. I think providing an error like you suggest for having UseWinUI and UseWpf both set in the same project is a good idea. If a WPF project wants to pick up WinUI and use Xaml Islands, it should just add a nuget reference to WinUI, and not set UseWinUI. If it wants to create custom controls, it then needs to have a separate WinUI Class Library project that has UseWinUI set and reference that from the WPF app.

As far as I'm concerned, Avalonia is welcome to piggy back on this as well.

dsplaisted commented 4 years ago

I think the targets for the different UI frameworks will need to coordinate at some level to avoid stepping on each other's toes.

Do you think there will eventually be XAML island scenarios where you will want to have both WPF and WinUI xaml files in the same project? If so, then eventually I think you would want to be able to invoke both compilers, and determine which xaml files to pass to which compilers via a combination of convention and item metadata. For example, for a project that uses both, if the default is to interpret xaml files as WPF Pages, then you could have the following to select a file to be built with the WinUI compiler:

<ItemGroup>
    <Page Update="WinUIPage.xaml" XamlCompiler="WinUI" />
</ItemGroup>

It might have been a better idea if early on WinUI switched to a different file extension (ie .winui). That ship has probably already sailed though.

stevenbrix commented 4 years ago

Do you think there will eventually be XAML island scenarios where you will want to have both WPF and WinUI xaml files in the same project?

@marb2000 and @mikehillberg are better equipped to answer that.

MikeHillberg commented 4 years ago

It's a good scenario, but it's not in plan for WinUI3, which is the current focus.

vatsan-madhavan commented 4 years ago

we can add a UseWinUI property that will pull in the Microsoft.NET.WindowsDesktop.Sdk and enable the globbing support.

This is what's really going on:

https://github.com/dotnet/wpf/blob/9ec4c6b1c4f500830518807d23484fe8775154b0/packaging/Microsoft.NET.Sdk.WindowsDesktop/targets/Microsoft.NET.Sdk.WindowsDesktop.props#L3-L32 https://github.com/dotnet/wpf/blob/9ec4c6b1c4f500830518807d23484fe8775154b0/packaging/Microsoft.NET.Sdk.WindowsDesktop/targets/Microsoft.NET.Sdk.WindowsDesktop.targets#L84-L151

Instead of enlightening WindowsDesktop SDK with more information about specific frameworks, why don't we consider making it less reliant on such knowledge in .NET 5 (including WPF) ? 😜 Bear with me - let me explain.

Given that one-project will always map to one XAML dialect (like WPF, WinUI etc.), we don't need to annotate XAML Item's with its dialect (like XamlCompiler=WinUI etc.). In fact, we can take away the $(UseWpf)==true check from CheckForDuplicateItems (_WindowsDesktopFrameworkRequiresUseWpfOrUseWindowsForms probably needs a fixup as well, although this might require some thinking).

With these changes, XAML files would always be globbed, and checked for duplicates etc. - and in NET.Sdk (you won't need NET.Sdk.WindowsDesktop). There would be no need to teach any pat of the framework anything special about WinUI etc. A WinUI project should just get the right XAML globbing inherited from WindowsDesktop's logic and it can just do its own compilation.

Do I have it right so far @dsplaisted ?

stevenbrix commented 4 years ago

Our requirements are fairly simple, and I'm all for this being in the right place and done properly. If UseWinUI isn't needed for the globbing support then that's great and we probably don't need it then :)

Just brainstorming...one convention we could adopt for WinUI and WPF in the same project is for WinUI pages have the .winui.xaml convention. Then you could have something like this to gather the appropriate pages to pass to the correct compiler:

<ItemGroup>
  *<WinUIPages Include="@(Page->EndsWith('.winui.xaml'))"/>
  <WpfPages Include="@(Page)" Exclude="@(WinUIPages)"/>
</ItemGroup>

*ignore the fact that this isn't currently possible in MSBuild but it totally should be, you have to do something ugly like this:

<WinUIPages Include="@(Page)" Condition="$([System.String]::new('%(Identity)').EndsWith('.winui.xaml'))"/>

The VS Wizard would need to know the WinUI Item is being added to a WPF project so that it adds the appropriate file name, but this would allow the .csproj to stay clean and xaml files to still have the xaml extension

jkoritzinsky commented 4 years ago

From the Avalonia perspective, I really like the XamlCompiler metadata if VS could use that to determine which XAML editor to load. That would solve one of the big issues Avalonia has when trying to work in VS.

vatsan-madhavan commented 4 years ago

@stevenbrix if the same project is not going to have multiple XAML dialects (WinUI + WPF for e.g.), then where is the need to exclude WinUI XAML pages during globbing? A project can simply assume that all XAML pages belong to its own dialect (i.e, WPF project can assume everything is WPF, WinUI can assume that everything is WinUI etc.).

It looks like globbing for XAML needs to work as a first-class construct. Beyond that, it's not clear whether any dialect-specific logic is needed at all.

@jkoritzinsky Item-metadata hints to identify and load a different designer - iff one is registered - maybe interesting. /cc @diverdan92

stevenbrix commented 4 years ago

@vatsan-madhavan and @dsplaisted so should this issue move to the https://github.com/dotnet/sdk repo then?

dsplaisted commented 4 years ago

If we are going to eventually support multiple XAML dialects in the same project, then we should try to make design decisions today with that in mind. For example, if eventually WinUI XAML files would use a .winui.xaml extension, then ideally we would already require or encourage that today.

I can see the value of moving the XAML globs down into the .NET SDK, especially since WinUI doesn't currently have their own targets outside of a NuGet package. However, are the globs for WPF and WinUI actually the same? The WPF globs are not all that simple (see the snippets that @vatsan-madhavan linked).

If we do move the globs into the .NET SDK, note that they would still only be active if UseWPF or UseWinUI was set to true. We don't want to suddenly change how we handle projects that don't use either UI framework but happen to have a .xaml file in the project folder. This also means that WinUI projects should explicitly set UseWinUI to true in the project file. If it were just set in targets from the WinUI NuGet package, then you would have different globs depending on whether the NuGet package was restored, which isn't a great experience in VS.

vatsan-madhavan commented 4 years ago

@dsplaisted if we move XAML globbing to .NET.SDK (which is an idea I like), then we should make it opt-in generically using EnableXamlGlobbing=true or something like that, and then UseWpf, UseWinUI etc. should turn it on. My larger point is that we should not design exclusively for WPF/WinUI - we should make allowances for other dialects like Avalonia or if MAUI wants to reuse/enable globbing etc.

stevenbrix commented 4 years ago

I would also add that it shouldn't do anything with ApplicationDefinition - it should be bare minimal globbing of .xaml files so that VS is happy. I would imagine that most of WPFs targets stay the same, except instead of doing the globbing, they referenced the pre-globbed xaml items from the .NET SDK

ryalanms commented 4 years ago

Can this issue be moved to https://github.com/dotnet/sdk? Thanks.

stevenbrix commented 4 years ago

@ryalanms I was initially thinking this issue could just move, but we'll probably need a tracking issue to update the Windows SDK to w/e is done, so it makes sense to keep this around for that.

@dsplaisted while I agree with your general sentiment, I just think that if we're able to make the globbing in the .NET SDK simple and generic enough, then we won't be putting ourselves into a corner. We don't support WinUI files in a WPF project, so if we decide they should adopt the .winui.xaml extension, then we can do that once we decide to. WinUI files don't need this extension in a project that is purely WinUI - it would only be if you want to integrate. Another option is the special metadata that says which framework they are for. Either way, I think we'll be ok for when we want to make future improvements. Let me know if you feel differently, you have more experience in this area than I do so I could be missing something.

I've filed this issue in the sdk repo: https://github.com/dotnet/sdk/issues/12520

jtbrower commented 4 years ago

I have been globbing in WPF NetCore for so long now that I automatically started doing it in my Directory.Build.targets when I began writing WinUI code a few weeks ago without really knowing that it wasn't supported in WinUI. One sporadic issue that I see happen sometimes is that Visual Studio decides to add a "None Remove" to some of my files without re-adding them back in as a Page (I would rather it not touch them at all like you stated). Since it removes the file it goes missing from my solution and it causes quite the confusion when it happens.

@stevenbrix

I would also add that it shouldn't do anything with ApplicationDefinition - it should be bare minimal globbing of .xaml files so that VS is happy. I would imagine that most of WPFs targets stay the same, except instead of doing the globbing, they referenced the pre-globbed xaml items from the .NET SDK

I caused myself quite a headache today regarding ApplicationDefinition though. I renamed App.xaml, didn't assign a new ApplicationDefinition and was bouncing my head off of a null reference exception that is hit in the Microsoft.UI.Xaml.Markup.Compiler.interop.targets MarkupCompilePass2 target. There is no specific error message, just the "object reference not set to an instance of an object". This happened to me in the middle of a renaming refactor so I assumed the null ref was caused by some meta file, left-over binaries, manifest, something, anything..... until finally I gave up and reverted all of the changes to my code so I could start narrowing it down. Needless to say, I really punished myself just to rename that file for no good reason (My assembly name ended in App so rather than be forced to rename namespaces, the assembly and the folder containing the code, I figured it would be faster for my demo code to just rename App.xaml. Dear lord was I wrong).

I really do not miss the days of Visual Studio messing with my project files and look forward to when it stops touching the ones for WinUI. The power of globbing and SDK style projects are one of my top 3 favorite things that happened to NetCore.

tibitoth commented 11 months ago

Is there any workaround for now if I want to use both UseWPF and UseWinUI in a project?