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

Breaking changes for better usability discussion #64

Open
gritsenko opened this issue Oct 1, 2024 · 7 comments
Open

Breaking changes for better usability discussion #64

gritsenko opened this issue Oct 1, 2024 · 7 comments
Assignees
Labels
enhancement New feature or request

Comments

@gritsenko
Copy link
Collaborator

I think using @ prefix for binding definition was not a good idea. It leads to increasing complexity and involves unnecessary string parsing logic under the hood. So I think this should be deprecated in future releases.

@gritsenko gritsenko added the enhancement New feature or request label Oct 1, 2024
@gritsenko gritsenko self-assigned this Oct 1, 2024
@Terria-K
Copy link

Terria-K commented Oct 8, 2024

I do agree with this one, at first I thought it was just a naming convention until I realize it wasn't. I suggest using a function with a name Binding to match the .axaml Binding syntax.

@Terria-K
Copy link

Terria-K commented Oct 8, 2024

Also, I just notice that compiling with AOT refuses the binding to work properly. I wonder if this is related to the @ syntax. Might test on some test project to see if its true.

@gritsenko
Copy link
Collaborator Author

Also, I just notice that compiling with AOT refuses the binding to work properly. I wonder if this is related to the @ syntax. Might test on some test project to see if its true.

I assume that it can caused by that classic avalonia binding is using string path to property, so some types can be trimmed while building.

also take a look on this sample where we have a workaround to prevent class from trimming

[method: DynamicDependency(DynamicallyAccessedMemberTypes.PublicProperties, typeof(SimpleComponent))]

@gritsenko
Copy link
Collaborator Author

gritsenko commented Nov 20, 2024

After some experience of making big app with this package I started to think that mapping extensions to every property was not the best idea. And the better approach at the moment will be using property initializers like this:

public class Component : ComponentBase
{
    protected override object Build() =>
        new StackPanel()
            .Children(
                new TextBlock() { Text = "Hello world" }
                    .Ref(out _textBlock1)
                new TextBlock() { Text = Bind(() => $"Counter: {(Counter == 0 ? "zero" : Counter)}") },
                new Button() { Content = "Click me" }
                    .OnClick(OnButtonClick)
            );
            . . . . . 

So we keep only common, not autogenerated extensions, i.e. for Children to visually split declaration of inner nodes from other properties and reduce amount of closing braces.

For Styles and event handlers I have to think yet.

So this change will lead to:
pros:

  • reduce complexity of this assembly dramatically and remove need using extension generation tool on external assemblies
  • allow to see native documentation from avalonia properties
  • increase build performance and better IDE support (now it sometimes show errors when can't infer return types of extensions)
  • better autocomplete support
  • more compact syntax, because it's still readable when you define property initializers in one line

cons:

  • you will need to rewrite your extension based code
  • less consistent syntax new View(){Prop1=1,Prop2=2}.Content(item,item,...) vs new View().Prop1(1).Prop2(2).Content(item,item)
  • can't put breakpoint in the middle of component building. So if something is broken debugger will highlight whole object creation block. But If we use "Children" Extension it's not that critical, as we still can localize exception in child components instead of root of hierarchy

I'm going to test the new approach for a couple weeks and I'd like to hear any comments from you about this as well

@gritsenko gritsenko changed the title Remove magical setter stuff uses @ for binding Breaking changes for better usability discussion Nov 20, 2024
@IvanJosipovic
Copy link
Contributor

IvanJosipovic commented Nov 21, 2024

I feel that the current Extension Method approach provides a cleaner and more consistent developer experience.

Its also inline with other frameworks like Uno. I just noticed that they are able to use their Source Generator to generate code for external libraries. See here, https://platform.uno/docs/articles/external/uno.extensions/doc/Learn/Markup/GeneratingExtensions.html#using-the-generator-for-3rd-party-libraries

Some comments on the pros:
"reduce complexity of this assembly dramatically and remove need using extension generation tool on external assemblies"
"increase build performance and better IDE support (now it sometimes show errors when can't infer return types of extensions)"

  • Can this not be solved by adding the Source Generator to the main Avalonia library so that all 3rd party packages come bundled with the extensions? or update the Source Gen to load external types like Uno does.

"allow to see native documentation from Avalonia properties"

  • Is this not solvable with the current approach? Can't we just copy the comments to the extension method?

"more compact syntax, because it's still readable when you define property initializers in one line"

  • I feel the syntax is less readable and confusing with the combined approach of methods/property initalizer

The 2 approaches side by side:

public class Component : ComponentBase
{
    protected override object Build() =>
        new StackPanel()
            .Children(
                new TextBlock() { Text = "Hello world" }
                    .Ref(out _textBlock1)
                new TextBlock() { Text = Bind(() => $"Counter: {(Counter == 0 ? "zero" : Counter)}") },
                new Button() { Content = "Click me" }
                    .OnClick(OnButtonClick)
            );
            . . . . . 
public class Component : ComponentBase
{
    protected override object Build() =>
        new StackPanel()
            .Children(
                new TextBlock()
                    .Ref(out _textBlock1)
                    .Text("Hello world"),
                new TextBlock()
                    .Text(Bind(() => $"Counter: {(Counter == 0 ? "zero" : Counter)}")),
                new Button()
                    .Content("Click me")
                    .OnClick(OnButtonClick)
            );
            . . . . . 

@IvanJosipovic
Copy link
Contributor

IvanJosipovic commented Nov 24, 2024

@gritsenko, I think we can generate extensions for local and external classes with a Source Generator.
I'm testing the idea on a branch here, #73

My plan is to hack a proof of concept. If it works, this should simplify the approach to a single codebase. Then we can look into performance by converting it to a Incremental Source Generator.

@gritsenko
Copy link
Collaborator Author

@IvanJosipovic thank you for your enthusiasm!) I'm going to check out it

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

No branches or pull requests

3 participants