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

WPF SDK feedback - netfx/corefx multitargeting #88

Closed
gulbanana opened this issue Sep 24, 2018 · 7 comments
Closed

WPF SDK feedback - netfx/corefx multitargeting #88

gulbanana opened this issue Sep 24, 2018 · 7 comments
Labels
Milestone

Comments

@gulbanana
Copy link

gulbanana commented Sep 24, 2018

I built a proof of concept for sharing code between Desktop and Core versions of WPF:
wpfpoc

The repository is at https://github.com/gulbanana/wpf-multitarget. It works well enough to be very encouraging; here are some gotchas I ran into along the way.

  • The SDK doesn't contain a glob for Page items, nor does it nest them using DependentUpon.

  • Building in visual studio sometimes takes a couple of tries. in particular the first clean rebuild seems to fail. This seems related to the generated xaml project.

  • When importing the props and targets, netfx must not use Microsoft.NET.Sdk.Wpf, because it already has winfx.targets coming from Microsoft.NET.Sdk. However, corefx needs the WPF targets imported after the core sdk targets. this means we can't use Project Sdk= but must instead decompose the sdk into hand-written imports.

  • Error messages are poor; in particular, failures to load xaml will be reported at runtime as a ArgumentNullException failing to format an error. Here's an incomprehensible stack trace which really meant "you have failed to declare an xmlns in a resource dictionary":
    xaml-load-failure

  • dotnet build cannot be used yet, presumably due to the dependency on Microsoft.WinFX.targets. I'm sure this one is a known issue, but it's irritating... yet useful for multi-targetting compatibility. Can we make sure that multitargeting remains possible when this is somehow fixed? :)

  • The lack of a Properties.cs in .net core projects - for the most part a good thing - means that there's no obvious place to put the ThemeInfo attribute required for control xaml discovery.

  • You basically have to be a WPF expert to discover most of this. Sharing code between legacy netfx and new corefx apps is going to be a major use case, so it could really use templates and polish. At the moment, if you fall slightly off the interop path then you get upsettting msbuild errors ("required targets are missing" and everything unloads itself) or upsetting runtime errors ("load failure in something xaml did somewhere").

@dasMulli
Copy link

cc/fyi @nguerrera

@nguerrera nguerrera added the wpf label Sep 24, 2018
@nguerrera
Copy link

Thanks for this feedback.

The SDK doesn't contain a glob for Page items, nor does it nest them using DependentUpon.

We would like page items to be globbed. There are some challenges around it, but that is the goal.

Building in visual studio sometimes takes a couple of tries. in particular the first clean rebuild seems to fail. This seems related to the generated xaml project.

I haven't seen this yet, it might be worth opening a separate issue with repro steps.

When importing the props and targets, netfx must not use Microsoft.NET.Sdk.Wpf, because it already has winfx.targets coming from Microsoft.NET.Sdk. However, corefx needs the WPF targets imported after the core sdk targets. this means we can't use Project Sdk= but must instead decompose the sdk into hand-written imports.

This should sort itself out once we have build working on core.

dotnet build cannot be used yet

Folks are working on it.

in particular, failures to load xaml will be reported at runtime as a ArgumentNullException failing to format an error.

This is fixed in recent WPF builds. I'll try to get it inserted.

The lack of a Properties.cs in .net core projects - for the most part a good thing - means that there's no obvious place to put the ThemeInfo attribute required for control xaml discovery.

This one seems minor. You can just create a ThemeInfo.cs or even Properties.cs without the other assembly info attributes in it.

@gulbanana
Copy link
Author

gulbanana commented Sep 25, 2018

Good to hear that much of this stuff is already known. I'll create a repro for the rebuild-fails thing as well as for any issues i encounter when porting some existing libraries and applications to the alpha. On a couple of specific points:

You can just create a ThemeInfo.cs or even Properties.cs without the other assembly info attributes in it.

Yes, this is what I did in the linked repository. The new dotnet wpf template doesn't do that, though, which means if people try to build custom control libraries without having fully understood the requirements they might be surprised that what worked on framework seemingly doesn't work on core.

This should sort itself out once we have build working on core.

It would be great if Microsoft.NET.Sdk.Wpf just worked when multitargeting - with the way it works now you could achieve this by making its targets conditionally import winfx depending on whether it's building for full framework. But, the whole scenario is slightly weird: sharing code between desktop-wpf and core-wpf will be necessary for businesses to transition, which means multitargeting, which means using the new project system... which hasn't been supported previously.

In short, to get adoption for WPF on .NET Core it will require some nontrivial build process changes for WPF on .NET Framework! This probably doesn't have to be a first class scenario, because library authors are more likely to know what they're doing than app authors, but it does have to be possible (as it currently is, though I get the feeling the tooling is going to change a lot).

@dasMulli
Copy link

Agree that multi-targeting is important.

When library authors moved to netstandard, most of them switched to multi-targeting to avoid braking changes to existing consumers. If companies and control / lib authors now also want to enable consumption from .net core, multi-targeting is also going to be an important scenario.

@nguerrera
Copy link

@gulbanana Have you tried multi-targeting with Preview 1 or more recent bits. It should work now to dotnet new wpf and change TargetFramework to TargetFrameworks=net46;netcoreapp3.0.

@nguerrera nguerrera added this to the 3.0.1xx milestone Dec 17, 2018
@nguerrera
Copy link

I believe all of the feedback has been addressed at this point. Let me know if I'm missing something and we can reopen.

@gulbanana
Copy link
Author

Everything's working well in the builds I've tested.

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

No branches or pull requests

3 participants