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

[MVUX] [Bindable] attribute is not considered by uno's type info generator #2648

Open
dr1rrb opened this issue Dec 19, 2024 · 13 comments
Open
Assignees
Labels
kind/bug Something isn't working triage/untriaged Indicates an issue requires triaging or verification.

Comments

@dr1rrb
Copy link
Member

dr1rrb commented Dec 19, 2024

Current behavior

When we generate a ViewModel we are adding a [BindableAttribute]. This attribute should be considered by the bindable type provider generator to generate the type info for bindings.

However it seems it's not : #2515 (comment)

Expected behavior

_ View models_ generated by MVUX should be binding friendly!

How to reproduce it (as minimally and precisely as possible)

cf. #2515 (comment)

@dr1rrb dr1rrb added kind/bug Something isn't working triage/untriaged Indicates an issue requires triaging or verification. labels Dec 19, 2024
@dr1rrb
Copy link
Member Author

dr1rrb commented Dec 19, 2024

Note: AN hypothesis would also be that MVUX's generator is executed after uno's type info generator.

@ajpinedam ajpinedam self-assigned this Dec 20, 2024
@DevTKSS
Copy link

DevTKSS commented Jan 1, 2025

Looked this up while investigating logging of navigation to see in first instance, at which event the DataContext of my MainPage will be set correctly and why my Content of MainPage (NavigationView) is everytime kind of broken after HotReload.

  1. Which particular attribute is the correct one to set on MainPage Codebehind class, for the MetadataProvider Line in DebugConsole? Would say that would be the Reactive.Bindable.BindableAttrubute, !But! I dont think so because in the intellisense is stated it is meant for the viewModel which is not equal to the view itself.
  2. The MainPage seems to get called twice? No idea why that should be done. First one after FrameNavigator via default Route, thats alright! But then the strange part beginns: all Controls which I did name with x:Name are popping up The DepencyProperty [...] is not defined in MainPage which makes me curious why it should even expect this to be a DependencyProperty? Maybe the MainPage_.... Partial generated Class from Microsoft Generator Analyzer is also delayed as imagined for the ViewModel Class?
  3. Looked up also the generated MetaDataTypeProvider (or similar named, you know which one i am meaning) which should hold the links and types from model and Viewmodel. This is looking fine as expected, so I would also vote here for delayed.

BUT!
4. Given that thought, is it possible that HotReload is missing some reload action which would re-insert the needed DataContext? To replicate, do a simple uno mvux application incl. Navigator Extension add a nav-item and e.g. contentcontrol with uen:Region.Attached + uen:Region.Navigator="Visibility" like its told in the docs.
The current page will be invisible after HotReload (with changed something on mainpage like adding a textblock somewhere)
The Navigator is expected at this point to navigate at least to default Route and this would be this current page! Not one line of the logger set to trace is telling about DataContext to be set before Initialization of the Bindings of this page.
If you set this.DataContextChanged += (s,e) => this.DataContext = (MainViewModel)e.newValue you will be greeted at runtime with a InvalidCastException, which is telling you would try to Case ShellModel into MainViewModel. Speaking also for wrong timing or non-optimal Navigation Routing not looking for that type its going to at this moment. Can someone tell where exactly this navigator is expected to set this DataContext to check this? Because if this also not know this types, just silencing that warning, it would have to be fixed there also.

Idea for that Binding of viewModel in general... Could we check if it is possible to do a extension for the Page partial class generated currently from Microsoft Generator, move it for Reactive generated ViewModel to same analyser (possible that could help with initial issue also? More control and so on?) and if the user is applying x:Bind in main instance of SomePage.xaml (using Binding would tell me, he dont cares about performance, so that sourcegen stepping in would not be needed) the generator which should already knowing about the ViewModel he has to bind that View to, at least if also navigation extension with routes is used! Could add a property ViewModel or _vm or whatever you prefer in sourcegenerators to that generated class? Even if we can not tell a sourcegenerator from the Roslyn issue to depend on other generators, could we opt 1 depend on uno own generators? Opt.2 setup the correct bound and correctly updated on ViewModel changed in there? That would be at least a little step forward and for me who only read the ViewModelSourceGen code in the repo that sounds at least imaginable...

@Youssef1313
Copy link
Member

All Roslyn generators are given the same compilation. So there is no concept of order. No generator can see the output of another.

@Youssef1313
Copy link
Member

The safest action is to not generate [Bindable], and instead implement an analyzer that warns the user for this case and suggest adding a non-generated partial with [Bindable]. Yes it's more code, but that is how life is with SGs.

@Youssef1313
Copy link
Member

Alternatively, the bindable generator may be complicated so that it tries to anticipate what the view model generator would be generating and act as if the attribute is seen. This was already done in XAML generator to support CommunityToolkit.Mvvm SGs.

@dr1rrb
Copy link
Member Author

dr1rrb commented Jan 7, 2025

All Roslyn generators are given the same compilation. So there is no concept of order. No generator can see the output of another.

Yes ... unfortunately

The safest action is to not generate [Bindable], and instead implement an analyzer that warns the user for this case and suggest adding a non-generated partial with [Bindable]. Yes it's more code, but that is how life is with SGs.

This is not possible: the "bindable" applies to the code generated by the MVUX generator. Even if user adds the [Bindable] himself the code will not be present yet.

@dr1rrb
Copy link
Member Author

dr1rrb commented Jan 7, 2025

@ajpinedam For now we should only validate that the issue is indeed because the bindable generator is not ran after MVUX's VM generator (or more precisely is run using the same code base), meaning bin-gen does not have access to the code generated by mvux-gen.

If so, we will put this hold for now as we will have to work on a larger fix to allow some sort of dependencies between generators as this is not the single case (for instance we already had to workaround the exact same case for toolkit's MVVM code gen).

(An idea would be to have some sort of discovery + add-in support between generators so a generator would be able to give info about the code it will generate to another generator without having to create a new roslyn generation which would significantly impact performance of generators).

@Youssef1313
Copy link
Member

This is not possible: the "bindable" applies to the code generated by the MVUX generator. Even if user adds the [Bindable] himself the code will not be present yet.

I don't see any reason why it won't be possible. The user will add a partial of the class that will be generated. And the analyzer should be able to flag that easily and warn the user. A CodeFixProvider can also be implemented to make it easier for developers. The only downside I see is that the user has to take an action and things are not happen magically for him. But as mentioned, the analyzer/codefix will help guide users.


If so, we will put this hold for now as we will have to work on a larger fix to allow some sort of dependencies between generators as this is not the single case (for instance we already had to workaround the exact same case for toolkit's MVVM code gen).

This is not possible. Source generators cannot have dependencies or ordering, not they can communicate with each others. However, in some cases, you can go with hacks if one generator can anticipate what another generator will do (most likely by duplicating some logic between generators). This was the case for XamlCodeGenerator where it can guess what will be generated by CommunityToolkit.Mvvm.

@dr1rrb
Copy link
Member Author

dr1rrb commented Jan 7, 2025

This is not possible: the "bindable" applies to the code generated by the MVUX generator. Even if user adds the [Bindable] himself the code will not be present yet.

I don't see any reason why it won't be possible. The user will add a partial of the class that will be generated. And the analyzer should be able to flag that easily and warn the user. A CodeFixProvider can also be implemented to make it easier for developers. The only downside I see is that the user has to take an action and things are not happen magically for him. But as mentioned, the analyzer/codefix will help guide users.

Because even if the user creates a partial VM class it will be empty as content is fully generated by the mvux-gen (including properties), so the bin-gen won't be able to do anything.

If so, we will put this hold for now as we will have to work on a larger fix to allow some sort of dependencies between generators as this is not the single case (for instance we already had to workaround the exact same case for toolkit's MVVM code gen).

This is not possible. Source generators cannot have dependencies or ordering, not they can communicate with each others. However, in some cases, you can go with hacks if one generator can anticipate what another generator will do (most likely by duplicating some logic between generators). This was the case for XamlCodeGenerator where it can guess what will be generated by CommunityToolkit.Mvvm.

This somehow what we were discussing. The idea would be to have some magic / hacks to discover other generators, then a generator-A could ask to another generator-B info about what it is going to generate so gen-A could also generate content based on that (one of the possible tracks). But yes the goal is to avoid duplication and/or double execution of generators.

@Youssef1313
Copy link
Member

Youssef1313 commented Jan 7, 2025

Because even if the user creates a partial VM class it will be empty as content is fully generated by the mvux-gen (including properties), so the bin-gen won't be able to do anything.

Ah I see, it will only see the type without any of its members. That's not enough for the bindable gen. I don't see any viable solution for this though. This was clearly stated by Roslyn team that this scenario can never work.

@DevTKSS
Copy link

DevTKSS commented Jan 13, 2025

@dr1rrb was researching for something other using gpt as my mind came back to that generaor dependencys and just asked it about that things. To state clear: I absolutely do know, that ki can often be false and I am not experienced enough to validate that information so would like your or other people's thoughts about that interesting output of it:

Yes, it is possible to establish dependencies between code generators, especially in C# and .NET, where code generators are often used in the context of Source Generators (part of Roslyn) or build tools like MSBuild. You can implement dependencies between code generators to ensure that one generator relies on the output of another.

1. What Are Code Generator Dependencies?

A dependency between code generators means that one generator performs its work based on the output of another generator.

Typical Scenarios:

One generator creates foundational structures (e.g., interfaces or base classes), and another builds upon them.

Generators work in phases (e.g., parsing, code creation, optimization).


2. Methods to Implement Dependencies in Code Generators

2.1. Source Generators in Roslyn

With Roslyn Source Generators (introduced in .NET 5), you can realize dependencies by utilizing the generated files of one generator within the project before another generator executes.

  1. Generator Order:

Roslyn generators run independently.

However, you can control which code is analyzed by defining dependencies between generators, e.g., by using partial classes.

  1. Implementation:

One generator creates a file (.cs) that becomes part of the compilation process.

Another generator consumes this file through syntax trees.

Example:

// Generator 1 creates:
partial class Example
{
    public static string GeneratedProperty => "Hello from Generator 1!";
}

// Generator 2 uses Generator 1:
partial class Example
{
    public void PrintMessage()
    {
        Console.WriteLine(GeneratedProperty);
    }
}

2.2. Orchestration via MSBuild

You can use MSBuild to control the order of generator execution:

Custom Tasks: Define an MSBuild task for each generator.

Output/Input Dependencies: Pass the output of one generator as input to the next.

Example:

<Target Name="RunFirstGenerator">
  <Exec Command="dotnet run --project Generator1" />
</Target>

<Target Name="RunSecondGenerator" DependsOnTargets="RunFirstGenerator">
  <Exec Command="dotnet run --project Generator2" />
</Target>

2.3. Dependency Injection Between Generators

Use Dependency Injection (DI) to share data between generators:

  1. Define a shared service accessible to both generators.

  2. The first generator stores information in the service, which the second generator retrieves.

Example:

public interface IGeneratorContext
{
    void AddData(string key, string value);
    string GetData(string key);
}

// Generator 1:
context.AddData("Key1", "Value1");

// Generator 2:
var value = context.GetData("Key1");

2.4. Shared Codebase

Provide a shared library used by both generators:

Example: A shared assembly containing generator-specific logic.

This assembly acts as a bridge for data exchange.


3. Practical Example with Roslyn

Here's a simple example of how generators can interact in Roslyn:

Generator 1:

[Generator]
public class Generator1 : ISourceGenerator
{
    public void Execute(GeneratorExecutionContext context)
    {
        var source = @"
            public partial class Example
            {
                public static string GeneratedProperty => ""Hello from Generator1"";
            }";
        context.AddSource("Example_Generated", source);
    }

    public void Initialize(GeneratorInitializationContext context) { }
}

Generator 2:

[Generator]
public class Generator2 : ISourceGenerator
{
    public void Execute(GeneratorExecutionContext context)
    {
        // Find the class generated by Generator 1
        var syntaxTrees = context.Compilation.SyntaxTrees;
        var exampleClass = syntaxTrees.FirstOrDefault(tree => tree.ToString().Contains("GeneratedProperty"));

        if (exampleClass != null)
        {
            var source = @"
                public partial class Example
                {
                    public void PrintGeneratedProperty()
                    {
                        Console.WriteLine(GeneratedProperty);
                    }
                }";
            context.AddSource("Example_Generated_Method", source);
        }
    }

    public void Initialize(GeneratorInitializationContext context) { }
}

4. Challenges

  1. Synchronization:

Code generators work independently, so you must ensure dependencies are correctly defined.

  1. Error Handling:

Errors in one generator may prevent dependent generators from functioning.

  1. Complexity:

Too many dependencies can make the system harder to maintain.


5. Best Practices

  1. Split Generators into Modules:

Each generator should have a well-defined responsibility.

  1. Use partial Classes:

Extend or modify existing classes without overwriting them.

  1. Document Dependencies:

Clearly state which generator requires what data.

  1. Write Tests:

Ensure that generators function independently using unit tests.


Summary

Dependencies between code generators can be implemented via execution order, data sharing (e.g., syntax trees or shared services), and build tools like MSBuild. Roslyn Source Generators provide flexibility for dynamic dependencies and enable the creation of modular and maintainable code generation systems.


In my opinion and understanding, the interesting parts for me, it mentioned, are especially the orchestian via msbuild and the shared codebase🤔 like some kind of shared mind of them or to kind of run the other one if finished...

Thinking of:
We do have reactive bindable vm and uno did some nice DI and hosting extensions. Would that enable some kind of base? The sdk already updates the packages in the background, so maybe we do already have something like a head that could manage them like shown above?

Also it did mention afterwards incremental generators existing and that reactive mvux is using but seemed to me it hadnt enough knowledge to reactive so did not follow that up.

What would you say, does that sound anything near realistic or what do You think about this ideas?

@Youssef1313
Copy link
Member

@DevTKSS The AI-generated content you got doesn't look correct to me. No generator can access syntax/semantics that is generated by another generator. That is an unfortunate limitation, but that is how it is. Any attempts to do so will (if successful in the first place) mess up with performance so badly.

@DevTKSS
Copy link

DevTKSS commented Jan 22, 2025

@Youssef1313 thank you for your reply! Was already thinking that would be too good to be true 😅🤷 sometimes AI gaves new views on things that did not came into my mind so far, but i am careful in trusting it blindly.

Would it be possible to sum the results of that non solvable problem up for the currently required setup for that bindable attribute missing issue in mvux?

  • Codebehind class of the page/view: should there be a bindable attribute added in general?

  • if so, please tell the best usable namespace since we do have multiple options named

  • model/records: bindable attribute to be added?

  • which namespace?

  • which setup each of the mentioned ones is best recommend for mvux usage considering:

  1. partial
  2. sealed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working triage/untriaged Indicates an issue requires triaging or verification.
Projects
None yet
Development

No branches or pull requests

4 participants