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

Hot Reload doesn't work when the root element is named #5

Closed
stevemonaco opened this issue May 2, 2024 · 11 comments
Closed

Hot Reload doesn't work when the root element is named #5

stevemonaco opened this issue May 2, 2024 · 11 comments

Comments

@stevemonaco
Copy link

When an x:Name is assigned to a root element (Window or UserControl), hot reloading doesn't work. I assume that some synchronization is happening and Avalonia doesn't like it when the Name is set again.

Repro (with XAML comments): https://github.com/stevemonaco/HotAvaloniaNameRepro

There are some behavioral differences in Rider and VS and I tested in both. Namely, the Window Title (and bindings to the Title) can be hot reloaded in Rider, but not in VS. Child state modifications never seem to work when the root element is named in either IDE.

I've been able to break hot reloading in a (real) project I made in Rider simply by adding an x:Name to the Window. Conversely, I've been able to fix another (real) project maintained in VS simply by removing a Window x:Name.

HotAvalonia version: 1.1.0
Avalonia versions: 11.0.10 and 11.1.0-beta2
.NET 8
VS: 17.9.3
Rider: 2024.1

Avalonia source where exception is thrown

Stack Trace:

[HotReload]Failed to reload 'HotAvaloniaNameRepro.MainWindow' ('avares://HotAvaloniaNameRepro/MainWindow.axaml'): 'System.InvalidOperationException: Cannot set Name : styled element already styled.
   at Builder_24391e343bd24033bf98f239fe7114c6_avares___HotAvaloniaNameRepro_MainWindow_axaml.__AvaloniaXamlIlPopulate(IServiceProvider, MainWindow)
   at Avalonia.Markup.Xaml.XamlIl.AvaloniaXamlIlRuntimeCompiler.LoadOrPopulate(Type created, Object rootInstance, IServiceProvider parentServiceProvider)
   at Avalonia.Markup.Xaml.XamlIl.AvaloniaXamlIlRuntimeCompiler.<>c.<LoadGroupSreCore>b__16_2(ValueTuple`2 t)
   at System.Linq.Enumerable.SelectEnumerableIterator`2.ToArray()
   at Avalonia.Markup.Xaml.XamlIl.AvaloniaXamlIlRuntimeCompiler.LoadGroupSreCore(IReadOnlyCollection`1 documents, RuntimeXamlLoaderConfiguration configuration)
   at Avalonia.Markup.Xaml.XamlIl.AvaloniaXamlIlRuntimeCompiler.LoadSreCore(RuntimeXamlLoaderDocument document, RuntimeXamlLoaderConfiguration configuration)
   at Avalonia.Markup.Xaml.XamlIl.AvaloniaXamlIlRuntimeCompiler.LoadSre(RuntimeXamlLoaderDocument document, RuntimeXamlLoaderConfiguration configuration)
   at Avalonia.Markup.Xaml.XamlIl.AvaloniaXamlIlRuntimeCompiler.Load(RuntimeXamlLoaderDocument document, RuntimeXamlLoaderConfiguration configuration)
   at Avalonia.Markup.Xaml.AvaloniaRuntimeXamlLoader.Load(Stream stream, Assembly localAssembly, Object rootInstance, Uri uri, Boolean designMode)
   at Avalonia.Markup.Xaml.AvaloniaRuntimeXamlLoader.Load(String xaml, Assembly localAssembly, Object rootInstance, Uri uri, Boolean designMode)
   at HotAvalonia.Helpers.AvaloniaControlHelper.Load(String xaml, Uri uri, Object control, MethodInfo& compiledPopulateMethod) in /_/src/HotAvalonia/Helpers/AvaloniaControlHelper.cs:line 34
   at HotAvalonia.AvaloniaControlManager.ReloadAsync(String xaml, CancellationToken cancellationToken) in /_/src/HotAvalonia/AvaloniaControlManager.cs:line 93
   at HotAvalonia.AvaloniaControlManager.ReloadAsync(CancellationToken cancellationToken) in /_/src/HotAvalonia/AvaloniaControlManager.cs:line 76
   at HotAvalonia.AvaloniaHotReloadContext.OnChanged(Object sender, FileSystemEventArgs args) in /_/src/HotAvalonia/AvaloniaHotReloadContext.cs:line 112'(AvaloniaHotReloadContext #35567111)
@Kir-Antipov
Copy link
Owner

Our world would be a much better place if all bug reports were this thorough and thoughtful. Thanks a lot for all the effort you put into this issue. The MRE is also hugely appreciated :)

I'm gonna ignore the fact that VS and Rider behave differently in this situation for a bit, in hope that when the problem is gone, this detail will no longer be relevant. I always knew that there were some problems with named components (e.g., if you rely on auto-generated fields for those, their values are not being updated whenever the control is repopulated, because Avalonia puts this logic in a completely irrelevant to the process place, so I need to re-implement this functionality on my side), but it never occurred to me, for some reason, that I should at least test if they at least somewhat work :D

So, the issue stems from Avalonia.StyledElement.Name, which blatantly throws an error if you try to re-assign the name of a control:

public string? Name
{
    get => _name;

    set
    {
        if (_stylesApplied)
            throw new InvalidOperationException("Cannot set Name : styled element already styled.");

        _name = value;
    }
}

Should be pretty easy to hack our way around this. Give me a moment!

@Kir-Antipov
Copy link
Owner

Your MRE seems to work now :)

Reloaded Window

Also, please, let me know if your projects utilize auto-generated fields for named controls. If not, I'll just push a quick minor update to fix the issue at hand; otherwise, I'll finally deal with them properly.

@stevemonaco
Copy link
Author

stevemonaco commented May 2, 2024

Wow, that was fast.

Auto-generated fields for named Controls? Do you mean the Avalonia XAML name reference generator? If so, then yes. It's most of the reason why I name controls in the first place though it does help automation, too.

I probably wouldn't try hot reloading adding/modifying a name for a control that existed at startup though.

Edit: Oh, I never use the root control name in code-behind...since that's this. If I name the root control, it's solely so I can use an ElementName-style binding.

@Kir-Antipov
Copy link
Owner

Do you mean the Avalonia XAML name reference generator?

I mean AvaloniaUI/Avalonia.NameGenerator, so yeah. There are a few issues with that project, at least for a tool like HotAvalonia:

  1. Even after being merged with the main repo, it didn't evolve into something more intricately connected to the core codebase; instead, it remained a simple source generator. While this might seem fine on its own, it actually disrupts the well-established workflow within the framework itself. HotAvalonia exists, at least in its current form, solely because Avalonia internally distinguishes between building a control from ground zero and populating an already existing control. Although Avalonia only utilizes the population routine to hydrate newly created controls, I discovered that, although with some tinkering, it can also be used to re-hydrate already populated controls. However, the fields generated by the source generator and the initialization logic associated with them are not part of that routine, or any routine for that matter. These fields are initialized within the auto-generated InitializeComponent method, which is only invoked after the control is created, assuming that it should be populated by then and never to be re-populated. Therefore, when this assumption doesn't hold true with HotAvalonia on board, those fields end up storing stale control instances that are no longer part of the logical tree, as newer ones have taken their place.

  2. While I can understand some reasoning behind why the source generator hasn't been more deeply integrated into the codebase, I simply cannot comprehend why the source generator in question doesn't follow ANY conventions. Source generators (and other tooling) should mark generated code as such, for example, using the GeneratedCodeAttribute. However, the AvaloniaNameSourceGenerator doesn't do that. As a result, it's literally impossible to differentiate between the code it produces and the code manually written by a user. Because of that, instead of simply retrieving fields produced by the generator through a few calls to reflection, I had to locate the InitializeComponent method and analyze its bytecode to identify specific patterns indicating fields holding named references. Only after that could I set up the re-initialization logic for them on every reload. While this task isn't particularly difficult, especially considering everything I've already implemented for this project, it seemed tedious and uneventful, so I procrastinated on it for quite some time :D

Anyways, as of be3e5b8, source-generated fields are now also refreshed whenever their parent control is re-populated. I would greatly appreciate if you could check if everything related to named controls works fine now on your end, as you mentioned that there are some additional quirks with Visual Studio, and I do not have a Windows machine on me to test that.

If everything is okay, I will push v1.1.1 asap :)

@stevemonaco
Copy link
Author

I see. Before I narrowed down this issue, I assumed controls were being recreated, the visual tree was being manipulated, and some state synchronization was happening between old and new. I've looked through the HotAvalonia code base, but I don't really understand the specifics in how it works. In particular, the scenarios where method injection and the various populate override methods are used. Whether things like the IL gen is wholly necessary or just there for perf. I'm interested in seeing how much of this can be applied to other state synchronization scenarios; layout (de)serialization and snapshot testing.

We could get [GeneratedCode] usage into Avalonia, but that would lock this fix behind (probably) Avalonia 11.2. It should probably be rewritten as an IIncrementalGenerator someday, too.

Anyways, I have tested this on VS / Windows by pulling my repro into the solution. Everything seems to work well, so this should be resolved. Thanks!

@stevemonaco
Copy link
Author

A bit late for this issue, but I submitted an Avalonia PR for the Name Generator attributes. It should be making its way in sooner-than-later, but it will still have some min version dependence.

AvaloniaUI/Avalonia#15616

@Kir-Antipov
Copy link
Owner

I assumed controls were being recreated, the visual tree was being manipulated, and some state synchronization was happening between old and new

Well, you're not wrong! At the end of the day, these are required components for any hot reload system :D Everything you mentioned does happen. However, some tasks are performed implicitly, while others are handled in a rather unusual manner.

The "proper way" to implement such a thing would be to keep a snapshot of the current logical tree and its relationship with the source XAML it's been built from, then track down changes in the source files, create a diff, and submit updates to affected nodes of the tree, yada yada. A number of people have tried to bring hot reload to Avalonia this exact way; all of them had some success with their prototypes, and all of them halted their attempts in the end because such an undertaking is quite a commitment to say the least. HotAvalonia doesn't do that. In fact, I believe there's not a single line of code in this repo that tries to parse XAML or address it in any other way than a simple string. HotAvalonia tracks changes on a per-file basis. So, whenever you change a source XAML file, controls associated with it are going to be completely rebuilt from ground zero. This approach has its downsides, sure, but also a huge benefit that makes it viable:

  • In terms of cons, obviously, this is highly suboptimal. For example, if you were to change a placeholder of a textbox, the "proper implementation" would only reload that exact textbox, and to be exact, only that single Placeholder property. HotAvalonia, however, will reload the entire UserControl that contains the affected textbox. So, if your XAML definition grows unconditionally to thousands of lines of code, you may experience significant slowdowns whenever hot reload takes place, as each time it needs to fully process the entire markup again, and again, and again (well, okay, the "proper implementation" needs to process it too in order to identify the changes, but it doesn't need to compile all this beauty, thus, in theory, performing the same task faster). However! On regularly sized files, this slowdown is not really noticeable, and rebuilding a single file is already infinitely faster than rebuilding the whole project, so I do not consider this a particularly huge downside.

  • On the bright side, this means much less maintenance for me, i.e., the project won't die the moment I go on vacation, get burned out, or anything in-between. The "proper implementation" needs to be deeply integrated into the Avalonia ecosystem - both to be possible at all and to not break every time Avalonia changes something, and it would still require an enormous amount of effort to make this work. This is why all previous attempts at introducing a first-party hot reload functionality have stalled out - the core team has bigger priorities at their hands, and stray enthusiasts who could bring this solution to life burn out long before it's anywhere near being finished. That's why I chose a different approach here - I leveraged already existing tooling, features, and quirks of Avalonia, found a few hacky ways to glue them together, and made a hot reload plugin out of that :D

I've looked through the HotAvalonia code base, but I don't really understand the specifics in how it works.

Arguably the most important component here is Avalonia.Markup.Xaml.Loader - the official library, provided by the core team and responsible for parsing and compiling XAML during runtime. So, whenever a source XAML file changes, I can simply take its updated content, shove it into that library, and get an updated method that's able to populate a control associated with the file in question. Once again, thanks to that, there are literally no maintenance costs associated with XAML for this project.

HotAvalonia's responsibility is to discover all controls within the running assembly, identify the XAML files they were built from, and then listen for change events on those files, just to throw Avalonia.Markup.Xaml.Loader at them, so it can then replace stale population methods with the newly compiled ones. That's basically everything HotAvalonia does. How it does all of that, however, is a different story :D

In particular, the scenarios where method injection and the various populate override methods are used. Whether things like the IL gen is wholly necessary or just there for perf.

As you could probably tell from the first paragraph, in this project, and this project only, I'm not really obsessed with optimizations. So, method injection and IL generation associated with it are pretty important for HotAvalonia to work.

Whenever we compile a new populate method for some control, it's easy for us to repopulate it - just call that generated method. However, Avalonia knows nothing about these shenanigans, so as soon as you create a new instance of the same control, it will revert to its "defaults" (i.e., its state at the moment of project compilation). To prevent that, HotAvalonia hijacks calls to all population methods within the assembly, so it can redirect those to freshly compiled alternatives that reflect the current state of your source files.

You could also notice that there's a fallback mechanism employed when method hijacking is not possible (well, at least its primitive version I implemented here). There's an interesting atavism within the code produced by Avalonia for user controls (i.e., controls for which x:Class is specified) - a special static Action<object> !XamlIlPopulateOverride field. It does exactly what's advertised and precisely what we need. Namely, if set, it takes precedence over the originally compiled !XamlIlPopulate method. I call this an atavism because, as far as I understand, it was used in the dark ages of Avalonia when all XAML was parsed during runtime. Thus, !XamlIlPopulate was meant to be a slow route where XAML is initially compiled and then an optimized method gets stored into the respective !XamlIlPopulateOverride field, making subsequent calls to the same routine much, much faster. Nowadays, however, Avalonia compiles your XAML during build time, rendering this whole endeavor unnecessary. Nonetheless, this artifact from the past persists and can be utilized by things like HotAvalonia.

Initially, I hoped to implement method hijacking with !XamlIlPopulateOverride. However, I later discovered that it's only generated for controls with x:Class specified in their source files. Thus, for example, this approach cannot be used to hot reload styles, resources, etc. Consequently, my initial attempt has been demoted to a mere fallback, and I opted to implement a more "proper" general-use method injector.

A bit late for this issue, but I submitted an Avalonia PR for the Name Generator attributes. It should be making its way in sooner-than-later, but it will still have some min version dependence.

Great job! Also, nice catch with the ExcludeFromCodeCoverageAttribute. I always forget about that one myself!

Even though I cannot solely rely on these attributes, since I want to support Avalonia 11+, I will make a quick commit that will help to disambiguate some things in the future, in cases where the attributes are present. Thus, your contribution is still very much helpful! Once again, nicely done! :)

Anyways, I have tested this on VS / Windows by pulling my repro into the solution. Everything seems to work well, so this should be resolved. Thanks!

That's great! I'm going to trigger the CI in a moment, so you can expect the updated package to be indexed by NuGet in a few hours :)

@Kir-Antipov
Copy link
Owner

My goodness, I need to stop writing these giant walls of text

@stevemonaco
Copy link
Author

Thanks for the fix and the wall of text. It was quite useful for me.

@leowest77
Copy link

@Kir-Antipov YOU ARE AMAZING, I have been having the same issue as steve without knowing the issue until now. Thank you for fixing it.

@Kir-Antipov
Copy link
Owner

@leowest77, thanks a lot for the kind words! And thanks to @stevemonaco for reporting the issue in the first place :D

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

3 participants