Use App.xaml in samples

[cmeeren]

Closes #319

[cmeeren]

I started with this commit, but I'm having trouble reducing noise in App.xaml. If I remove anything that's now there, from the innermost <ResourceDictionary /> and outwards, compilation invariably fails in App.xaml.cs with the error The name 'InitializeComponent' does not exist in the current context.

@TysonMN any ideas?

[TysonMN]

I don't have that problem on your branch. After stashing or committing all changes, try closing Visual Studio and running git clean -fdx.

[cmeeren]

I often do, and unfortunately it doesn't help this time. Check out the latest commit, which fails for me even after git clean -fdx.

[TysonMN]

I am now experiencing that problem. Not sure what changed. Investigating now.

[TysonMN]

App.g.i.cs in the obj folder has an entry point and is missing Initialize Component. This makes me think that the <StartupObject> line isn't being respected. Try targeting .NET Framework or Core to see if that works.

[TysonMN]

I pushed commit a51c734 to your branch. It compiles for me now.

It seems like <StartupObject> only works if App.Resources contains merged dictionaries with at least one that is nonempty. This has to be a WPF complication bug.

[cmeeren]

I see. Do you want to report it, and then we can link to the issue in a comment in App.xaml?

[TysonMN]

I don't understand. This code compiles.

[TysonMN]

Oh, I wasn't calling InitializeComponent in the constructor of App.

[TysonMN]

Wait, why do you have InitializeComponent in the constructor of App? If you create a new WPF application in Visual Studio, then you will see that the template does not include a call to InitializeComponent in the constructor of App. This makes me think that App should not call InitializeComponent in its constructor.

My guess is that you added App.xaml to the project by creating a Window or UserControl, both of which do include a call to InitializeComponent in their constructor, and then you renamed the element to Application but did not delete the constructor or the call to InitializeComponent.

[cmeeren]

Yes, you are right. I assumed all code-behind partial classes needed InitializeComponent. So I should simply remove InitializeComponent? (And even the constructor? What about the entire code-behind file? Even if it's possible, should we keep the file and/or constructor to make it easier to "plug in" functionality here as I did here?)

[TysonMN]

I just tested with your app-xaml-startup branch. If InitializeComponent is removed from the constructor of App, then the themes from Material Design in Xaml are not used. In that sense, calling InitializeComponent is necessary. However, it seems that the compiler doesn't create that method if it thinks it isn't necessary. Maybe there is a bug related to exactly which edge cases cause that method to exist or not exist.

I suggest that we find some code that causes InitializeComponent to be generated and include a call to it in the constructor of App.

[cmeeren]

I suggest that we find some code that causes InitializeComponent to be generated and include a call to it in the constructor of App.

So, pretty much like a51c734?

[TysonMN]

Yea, that seems to work.

[cmeeren]

Great. I'll put that and a comment in all the samples, then.

BTW, IIRC InitializeComponent is the call that actually kind of loads/initializes the XAML file in the first place.

[cmeeren]

Ok, I have experimented a bit more now. There are several ways to go about this. I found a fairly simple way that works with StartupUri, which AFAIK is what you originally were looking for. I think it's good to roll with the defaults you get when you create projects in VS, and this one also avoids the InitializeComponent issue: First of all it compiles with InitializeComponent and an empty resource dictionary, and second of all you don't even need InitializeComponent; resources work correctly without it.

See the latest commit above this comment (b9693c2). In short:

• Removed Program.cs
• Added StartupUri="MainWindow.xaml" in App.xaml
• Changed Program.runWindowWithConfig to Program.startElmishLoop
• On the first Activated event of App, start the Elmish loop with the app's MainWindow

In v4, we might consider renaming Program.startElmishLoop to Program.startElmishLoopWithConfig and adding a new Program.startElmishLoop without the config argument.

There are alternatives to the Activated event, but I like it best.

• Using the Loaded event of the MainWindow also seems fairly clean, but it "feels" better to keep app startup stuff in App. Feel free to disagree.
• Not using StartupUri is another option, but then we're back to previously discussed methods and workarounds.

Thoughts?

[cmeeren]

@TysonMN To be clear, I'm waiting for feedback from you before continuing. In particular, I'd like to know if the method from the latest commit works for your use-cases, and if you can see any obvious negatives or potential improvements.

[TysonMN]

Yes, sorry for the delay.

I found a fairly simple way that works with StartupUri, which AFAIK is what you originally were looking for.

I wrongly assumed that StartupUri was necessary to have application-level resources.

I have been thinking a lot about this and going back and forth on what I think is best. Here is the comparison as I see it.

StartupUri and Activated event

Advantages

1. App.InitializeComponet is (created and) called if necessary (see obj\Debug\net5.0-windows\App.g.i.cs).
2. Most similar to how a traditional WPF application starts.

Disadvantages

1. Subscribing to an event for only one raising is a bit hacky.
2. Event subscription and unsubscription feels brittle (compared to the same in F# where subscribing to an event returns an instance of IDisposable that unsubscribes when Dispose is called).

Custom entry point

Advantages

1. Very clear about how the flow of control starts and progresses.

Disadvantages

1. Need for nontraditional <StartupObject> entry in the project file.
2. Need an explicit call to App.InitializeComponent when application-level resources exist.

I think the approach using StartupUri and Activated event is better for newcomers because of the two advantages given above. At the same time, I think I will continue using the approach with a custom entry point for my application at work because one the one advantage given above.

What follows is an extended exposition about why I will continue using the approach with a custom entry point.

I strongly dislike all the "magic" that Microsoft puts into their code. Especially before Entity Framework Core, it was difficult to provide configuration options because some of the configuration was expected to just exist in the same assembly and it would found at runtime via reflection. Similarly, ASP.NET requests a "startup" type that doesn't have to implement any interface. Instead, it is just assumed that there will exist an instance method (or more?...I don't even know) with a certain name and certain inputs, and it will get called via reflection. (One could argue that the example with ASP.NET is a case of structural typing instead of the predominant use of nominal typing in all of .NET, but one would be wrong because the structure of the requested type isn't given either; it is just "magic".)

I do a really good job logging anything that can go wrong in my application at work. (I will eventually have a long series of blog posts about that.) This starts by configuring the logging before anything has an opportunity to go wrong. When using the traditional WPF entry point, not only is it a bit unclear what happens in what order (though I now know that the entry point is in obj\Debug\net5.0-windows\App.g.i.cs), but it is impossible to configure logging before the static and instance constructors of Application execute. I don't know of anything that can go wrong in them or where they log if something does go wrong, but it shouldn't be necessary to answer that questions in order to have a high degree of confidence in your application. (Also, I looked at the source code for Application, but I can't figure out where the MainWindow is instantiated.)

Here is a specific reason I want to continue using the custom entry point approach in my application at work. Constructing the MainWindow might fail because of some XAML problem even though the solution built successfully. I have a test that checks for this (also a future blog post). However, I sometimes makes a change and run the application before running tests. In this case, my logging gets configured first, which includes logging to Seq. Then when the application begins to crash by breaking in Visual Studio and displaying exception messages, I know that all of this is being logged, so I don't have to pay attention to every detail in the moment. I can make an assumption and try to fix the problem given that assumption. If that assumption is wrong, I can review exactly what went wrong in Seq instead of spending time trying to reproduce the problem. I could still have issues that occur when constructing the MainWindow logged by configuring logging in the constructor of App. However, then it would not be so clear that this is happening at the right time. Most of the time nothing is logged. Is that because everything is working or because logging is not configured correctly? This is why TDD wants you to see the test fail for the right reason before making it pass by changing the production code...because we want the test to pass for the right reason.

[cmeeren]

Disadvantages

1. Subscribing to an event for only one raising is a bit hacky.
2. Event subscription and unsubscription feels brittle (compared to the same in F# where subscribing to an event returns an instance of IDisposable that unsubscribes when Dispose is called).

Generally I don't agree with this. I do agree somewhat with the "feeling", and that the IDisposable method feels "safer" in the sense that unsubscription is guaranteed by language constructs. But I don't have any actual arguments against this pattern as used here. IIRC event handlers are called synchronously, so it's entirely deterministic and not brittle at all. And the control flow isn't particularly complicated in this case. Sure, I wouldn't base my business logic around events being subscribed and unsubscribed willy nilly, but that's not relevant here.

Of course, the above is based on assumptions about how the Activated event works. Would I stake my life on it? No way. Would I stake the startup code of a production app on it? Given that 1) the official documentation fits our use case, 2) it was suggested by a seemingly well written SO answer, and 3) it's observed to work, then yes. (In any case, this doesn't have anything to do with the fact that it's an event; this goes for any API.)

Disadvantages

1. Need for nontraditional <StartupObject> entry in the project file.
2. Need an explicit call to App.InitializeComponent when application-level resources exist.

I think the second one here is a fairly major disadvantage, since according to our observations,

1. it is not possible to call InitializeComponent if there are no resources (for certain vague and not entirely logical definitions of "no resources"), and
2. if you forget to call it when it's available, resources won't be loaded.

I think the approach using StartupUri and Activated event is better for newcomers because of the two advantages given above.

We agree on this point. I take it that you find it okay to make the samples use this pattern, then? We could of course also have a sample that just demonstrates the explicit entry point variant.

What follows is an extended exposition about why I will continue using the approach with a custom entry point.

Thanks for the detailed explanation. I can relate to the desire for removing all of the "magic".

[TysonMN]

I take it that you find it okay to make the samples use this pattern, then? We could of course also have a sample that just demonstrates the explicit entry point variant.

Indeed. One sample using the custom entry point would be nice but not necessary.

[cmeeren]

I have yet to update all samples, but I have made some important changes I'd like input on:

• Changes to Program.fs: startElmishLoop is deprecated and renamed to runElmishLoopWithConfig, and I have added runElmishLoop as a config-less alternative. I think this is a more graceful way to introduce the breaking change mentioned in Use App.xaml in samples #320 (comment). I think run is just as good as start (and has a parallel in runWindow), but I'm open to suggestions.
• Updated the doc comments of several Program.fs functions
• Updated the Getting started instructions in the readme (Edit: As well as other relevant sections of the readme)
• I cleaned up the sample code (removed the test resource), and I think the sample now represents how I want all samples to be adjusted in this PR.

[TysonMN]

I will look closely tomorrow.

I still like the name startElmishLoop.

[cmeeren]

I still like the name startElmishLoop.

Same here, I just wanted to have that name be "config-less" and rename the existing one to startElmishLoopWithConfig. That is a breaking change, so I figured that by having other names, it could be done by way of deprecations. That allows us to guide users to the new API using a compiler warning (or error if they have warnings as errors). But I'm certainly open to other suggestions, including simply keeping the existing name and then breaking it in v4. As breaking changes go, this is not one that will cause much churn for users, so I'm not too concerned about it.

[TysonMN]

Sure. I don't mind breaking changes when, as you said, the fix is straightforward.

I have reviewed now. This looks good. I especially like the comments in the project file. That is a nice touch.

[cmeeren]

I have reviewed now. This looks good.

It is my understanding then you can get behind all the current PR changes to Program.fs (in particular, the deprecation/rename). Is that correct?

[TysonMN]

I still prefer the name startElmishLoop instead of runElmishLoop, but I am indifferent enough that I don't mind deferring that decision to you.

I am happy with all the other changes.

[cmeeren]

I still prefer the name startElmishLoop instead of runElmishLoop

I agree. Perhaps I'll just revert the rename and remove the config-less variant for now, and we can bring it back in v4 with a breaking change. The required change for users would be trivial.

[cmeeren]

This is now ready for review.

I have not squashed the commits; I plan to do that in GH when merging.

[cmeeren]

Forgot to mention that I have tested all the samples.

[TysonMN]

Very good! I left some minor feedback.

[TysonMN]

I think this is ready to be merged

[cmeeren]

Great, thanks!