Caliburn-Micro / Caliburn.Micro

A small, yet powerful framework, designed for building applications across all XAML platforms. Its strong support for MV* patterns will enable you to build your solution quickly, without the need to sacrifice code quality or testability.
http://caliburnmicro.com/
MIT License
2.79k stars 775 forks source link

Avalonia -> dont call InitialiseComponent, apps crash otherwise #852

Closed danwalmsley closed 1 year ago

danwalmsley commented 1 year ago

Recent changes meant that initialisecomponent was getting called twice, once by the call generated by avalonia from the view ctor, and again via reflection, causing apps to crash.

danwalmsley commented 1 year ago

@Stannieman I know you changed this, however in the latest avalonia, the initialisecomponent call is automatically generated in the view ctor, so it will be called implicitly when the instance is created.

In my test apps, it was crashing because xaml was being processed twice.

Let me know though if you think this should be called in some cases that im unaware of though.

Stannieman commented 1 year ago

@danwalmsley will check against latest Avalonia nightly.

Stannieman commented 1 year ago

It is not working for me. I changed my DI container to return "new SomeView()" which gets called when Caliburn resolves the view. SomeView is derived from UserControl. The only way to get something to show is creating a constructor and call InitializeComponent from there. My views have no other constructors.

But I wonder, these calls to InitializeComponent are not generated at build time, are they? Maybe something goes wrong in that step.

I will try to find a minimal repro.

Stannieman commented 1 year ago

There might be a misunderstanding. The VS template indeed generates a constructor that calls InitializeComponent, but when you remove that it is not called automagically anymore.

E.g. this simple app does not show the red background of MyView.

<Window xmlns="https://github.com/avaloniaui"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
        xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
        mc:Ignorable="d" d:DesignWidth="800" d:DesignHeight="450"
        x:Class="AvaloniaCaliburnMinimalRepro.MainWindow"
        Title="AvaloniaCaliburnMinimalRepro"
        xmlns:root="clr-namespace:AvaloniaCaliburnMinimalRepro">
    <root:MyView></root:MyView>
</Window>
using Avalonia.Controls;

namespace AvaloniaCaliburnMinimalRepro
{
    public partial class MainWindow : Window
    {
        public MainWindow()
        {
            InitializeComponent();
        }
    }
}
<UserControl xmlns="https://github.com/avaloniaui"
             xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
             xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
             xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
             mc:Ignorable="d" d:DesignWidth="800" d:DesignHeight="450"
             x:Class="AvaloniaCaliburnMinimalRepro.MyView"
             Background="Red">
</UserControl>
using Avalonia.Controls;

namespace AvaloniaCaliburnMinimalRepro
{
    public partial class MyView : UserControl
    {
        // Uncomment this and it shows, comment it and it does not.
        //public MyView()
        //{
        //    InitializeComponent();
        //}
    }
}

Is there something on the control that we could check before calling InitializeComponent to know if it already happened or not?

Stannieman commented 1 year ago

Also I am not sure why it crashes for you. What exception do you get? I just tried calling InitializeComponent multiple times in the constructor and it does not crash for me.

danwalmsley commented 1 year ago

@Stannieman what is the reason to remove the ctor? Avalonia expects the ctor to be left intact?

Stannieman commented 1 year ago

The same is true for WPF. It generates this but you can remove it in which case Caliburn still handles it for you. Not sure if we want to change this Caliburn behavior.

Although in WPF it is convenient because you can just leave away the entire code behind file, for Avalonia that does not work.

KasperSK commented 1 year ago

Does InitializeComponent() not protect itself from being called multiple times in Avalonia?

Stannieman commented 1 year ago

For me it seems to protect (or at least not explode), I can call it as much as I want. No crashes and tried with and without Avalonia.Diagnostics dependency added (it the parameters of InitializeComponent). I'd like to know what exception @danwalmsley is seeing.

KasperSK commented 1 year ago

That would be my expectation, that it should protect itself, as for the ctor with more parameters added based on which dependencies are included that I do not like. Why not have it be an additional constructor instead of replacing the one parameter ctor?

edit: Spelling mistakes

Stannieman commented 1 year ago

@KasperSK It comes from here: https://github.com/AvaloniaUI/Avalonia/blob/1da8891d846cbffd3c1f7ca6a837ea6e457a4bfc/src/tools/Avalonia.Generators/NameGenerator/InitializeComponentCodeGenerator.cs

TBH I do not have too much problems with this particular case.

KasperSK commented 1 year ago

@Stannieman It might be okay for this, but looking at the code in your link it does not look like the method guards getting called twice? Perhaps there is logic somewhere else to handle the guarding?

Stannieman commented 1 year ago

I assume AvaloniaXamlLoader.Load guards, but that actually calls into compiled XAML so it is harder to find out how that works.

KasperSK commented 1 year ago

Does Avalonia have any navigation features build in like the Frame class in WPF and UWP?

vb2ae commented 1 year ago

@danwalmsley I manually merged this into the Avalonia ui branch the other day. I did notice that the templates in the features project based on type are not working with this change.

<UserControl.DataTemplates>
    <DataTemplate DataType="{x:Type local:PhotoActivityViewModel}">
        <StackPanel>
            <StackPanel Orientation="Horizontal">
                <Rectangle Height="60" Width="100" Fill="Blue" Margin="0,0,20,0" />
                <Rectangle Height="60" Width="100" Fill="Blue" Margin="0,0,20,0" />
                <Rectangle Height="60" Width="100" Fill="Blue" Margin="0,0,20,0"  />
            </StackPanel>

            <TextBlock Text="{Binding Caption}"/>

        </StackPanel>
    </DataTemplate>
    <DataTemplate DataType="{x:Type local:MessageActivityViewModel}">
        <Grid>
            <Grid.ColumnDefinitions>
                <ColumnDefinition Width="Auto"/>
                <ColumnDefinition Width="*"/>
            </Grid.ColumnDefinitions>

            <Ellipse Grid.Column="0" Width="48" Height="48" Fill="Blue" Margin="0,0,12,0" VerticalAlignment="Top" />

            <StackPanel Grid.Column="1">
                <TextBlock Text="{Binding Path=Title}" FontSize="16" FontWeight="SemiBold" />
                <TextBlock Text="{Binding Path=Message}"/>
            </StackPanel>
        </Grid>
    </DataTemplate>
  </UserControl.DataTemplates> 
vb2ae commented 1 year ago

@danwalmsley any update on when new Avalonia nuget package will be available?

danwalmsley commented 1 year ago

is Caliburn beh

It wont work with dependency injection!

danwalmsley commented 1 year ago

Does Avalonia have any navigation features build in like the Frame class in WPF and UWP?

<ContentControl Content="{Binding MyPage}" />

vb2ae commented 1 year ago

@danwalmsley all dependency injection containers? or just Simple Container?

Stannieman commented 1 year ago

is Caliburn beh

It wont work with dependency injection!

You mean if 1 view gets other views injected into it by DI? Indeed they will not be initialized if InitializeCompoment is not called from constructor, but that works the same for WPF where you removed InitializeCompoment from the constructor today.

My experience with both WPF and Avalonia 11 is that you can choose to have InitializeCompoment in the constructor or not, and if you do have it then calling it a 2nd time (by Caliburn) does not explode.

Could you perhaps show the code that makes it crash for you?

danwalmsley commented 1 year ago

@Stannieman

This is why you cant call it by reflection! Stuff will break! :D

image

I discussed it with the rest of the team, and the conclusion is this:

Currently avalonia does not support AvaloniaXamlLoader.Load(this) (which is what initialisecomponet does) more than once. Its not detected or protected against.

The scenario of removing the codebehind file from xaml or from removing the ctor from code behind is not supported.

KasperSK commented 1 year ago

@danwalmsley is there a particular reason why there is no guard against calling AvaloniaXamlLoader.Load(this) twice?