-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
[Proposal] Unify the internals of controls and resources #13085
Comments
There is no difference between XAML generation for controls and resources. But there is a difference, if XAML file has x:Class or not.
|
In general, I don't see any issues with generating PopulateTrampoline for both. cc @kekekeks |
Your approach with just calling Populate on an existing object has several problems:
So, rather than just introducing more undocumented fields, I'd prefer to expose some API that would be useful for a hot-reload plugin: static class DevRuntimeXamlHelpers
{
public static void ReplaceClassXamlLoader(Type targetType, Action<object> loader);
public static void ReRunClassXamlLoader(object loader);
public static CompiledXamlInfo GetCompiledXamlInfo(Type targetType);
}
class CompiledXamlInfo
{
// You need to reset those before reloading XAML
IReadOnlyList<Avalonia.Data.Core.IPropertyInfo> AssignedRootProperties {get; }
} We also need to change the way WPF has |
Oh, yeah, sure. I chose the terminology "controls" and "resources" since methods affiliated with class-less components are stored in the "!AvaloniaResources" class, and we usually don't create classes for resources, but usually do for controls.
I'm aware of the second and third points, and they are fixable. However, could you please provide an example of the first one? I think I'm missing something.
I'd prefer the official and properly implemented API built into the framework any day of the week. However, why not both? ;) Implementing such an API will require much more thought, effort, time, and testing than "introducing more undocumented fields", which, by the way, I don't see that way - they are not really new; those would be the same fields already used by "classed" components. So, the proposed change still brings the pros of more internal consistency, and it also makes these undocumented fields much more useful than they are at the moment.
While in my particular case with the setup I have, it's pretty easy to deal with re-initializing fields for named controls, I strongly agree that this process should be a part of the Populate method. Initializing Here are my thoughts on how this could be implemented: Avalonia could provide an interface for controls that contain named children, like this one: public interface INamedControlContainer
{
bool TryGetControl(string name, [NotNullWhen(true)] out object? control);
bool TrySetControl(string name, object control);
} With something like that being accessible, a code generator can create partial definitions for user controls as follows: partial class Foo : INamedControlContainer
{
internal Button Btn;
bool INamedControlContainer.TryGetControl(string name, out object? control)
{
_ = name ?? throw new ArgumentNullException(nameof(name));
switch (name)
{
case "Btn":
control = Btn;
return true;
default:
control = null;
return false;
}
}
bool INamedControlContainer.TrySetControl(string name, object control)
{
_ = name ?? throw new ArgumentNullException(nameof(name));
_ = control ?? throw new ArgumentNullException(nameof(control));
switch (name)
{
case "Btn":
// TODO: Move the exception to some shared location.
Btn = (control as Button) ?? throw new ArgumentException($"Invalid control type. Expected {typeof(Button)}. Received {control.GetType()}.", nameof(control));
return true;
default:
return false;
}
} And then we can call the (P_1 as INamedControlContainer)?.TrySetControl("Btn", button); Pros:
Cons:
|
Description
Currently,
CompileAvaloniaXamlTask
creates the following infrastructure for user-defined controls:This allows us to define a control as follows:
Func<FooControl>
.ctor
Action<IServiceProvider, FooControl>
!XamlIlPopulate
Action<FooControl>
!XamlIlPopulateOverride
Action<FooControl>
!XamlIlPopulateTrampoline
However, for resources like styles/resource dictionaries/etc. the generated code looks like this:
Thus, a resource can be described as:
Func<IServiceProvider, FooControl>
Build:/BarResource.axaml
Action<IServiceProvider, FooControl>
Populate:/BarResource.axaml
Although the structures above exhibit similarities, they don't conform to the same pattern. Most notably, resources lack the capability to override their population logic.
Proposal
To align controls and resources more closely in their internal representation, I propose modifying the generated code for the latter to resemble something along these lines:
Justification
This adjustment would make controls and resources more alike, as they would both loosely follow the same pattern:
Func<T>
Func<IServiceProvider, T>
.ctor
Build:/T
Action<IServiceProvider, T>
!XamlIlPopulate
Populate:/T
Action<T>
Action<IServiceProvider, T>
!XamlIlPopulateOverride
PopulateOverride:/T
Action<T>
Action<IServiceProvider, T>
!XamlIlPopulateTrampoline
PopulateTrampoline:/T
Moreover, this enables overriding the population logic of resources in the same manner as with regular user controls. Personally, I would say this is the most important part of this proposal. Recently, I developed a project that introduces hot reload capabilities to Avalonia – Kir-Antipov/HotAvalonia. The most significant hurdle I encountered was updating the population logic of recompiled styles/resource dictionaries, given the absence of a mechanism akin to
!XamlIlPopulateOverride
for those. At the moment, to achieve this, I had to commit a few federal crimes, and I would rather not do that in the future. This feature appears essential for implementing hot reload in Avalonia, whether done by third parties or natively by Avalonia itself (refer to this comment). Hence, I would greatly appreciate seeing this change implemented.Additional Context
This is a relatively minor and non-breaking modification that augments the consistency within Avalonia's internals, has potential advantages for Avalonia when it eventually introduces built-in hot reload capabilities, and third parties could already benefit from it.
If you greenlight this change, I would be more than happy to contribute and implement it myself.
The text was updated successfully, but these errors were encountered: