Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Using a different base component class #52

Closed
hillin opened this issue Jun 4, 2021 · 15 comments · Fixed by #63
Closed

Using a different base component class #52

hillin opened this issue Jun 4, 2021 · 15 comments · Fixed by #63
Labels
type: feature New feature or enhancement

Comments

@hillin
Copy link
Contributor

hillin commented Jun 4, 2021

MvvmComponentBase inherits from ComponentBase. What's the best practice if we want to use a different base component class, say OwningComponentBase or AbpComponentBase as we are using the ABP framework, yet still want to keep the functionalities provided by MvvmComponentBase?

@klemmchr
Copy link
Owner

klemmchr commented Jun 4, 2021

I would guess that this is not possible atm. But I see that this would make sense, especially regarding interoperability with other libraries. Gonna make a concept and implement it.

@klemmchr klemmchr added the type: feature New feature or enhancement label Jun 4, 2021
@klemmchr
Copy link
Owner

klemmchr commented Jun 6, 2021

Turns out, this is a non trivial problem that I don't really have a proper solution for. I would need the possibility to inherit from a generic which isn't supported in C#. Besides that, the only possible solution that seems feasible to me would be to copy the code over into MvvmOwningComponentBase but this way AbpComponentBase wouldn't be supported either,

@hillin
Copy link
Contributor Author

hillin commented Jun 6, 2021

No you can't inherit class T from a Generic<T>, that's for sure.

My ideas is maybe we can use composition over inheritation. The binding functionalities could be provided by a individual object, say, an instance of the class Binder<TViewModel>. The MvvmComponentBase<TViewModel> class can have the binder built in, keeping the current API; while other component classes can instantialize their own binder object and use it to handle the bindings.

@inject Binder<MyViewModel> Binder;
<div>@Binder.Bind(vm=>vm.Text)</div>

@klemmchr
Copy link
Owner

klemmchr commented Jun 6, 2021

This isn't trivial either. Moving out the binding related code into a separate class is not easy since you would need a reference back to the component from Binder<T> to invoke the value changed event. I tried to extract relevant code into a class named Binder, however I see no really pleasant way to have this needed back reference without making the syntax look horrible.

@hillin
Copy link
Contributor Author

hillin commented Jun 7, 2021

I think the syntax may get a little bit messy, but acceptable. Pseudo code:

class Binder<T> where T : ViewModelBase
{
  public Action StateHasChangedCallback { get; init; }
  public Binder() {}
  public Binder(Action stateHasChangedCallback) { this.StateHasChangedCallback = stateHasChangedCallback; }
  public void OnInitialized() { /*...*/ }
  public void OnParametersSet() { /*...*/ }
  /* and other lifecycle methods */
}

abstract class MyComponentBase: SomeThirdPartyComponentBase 
{
  public Binder<MyViewModel> Binder { get; }
  public MyComponent() 
  {
    this.Binder = new Binder<MyViewModel>(this.StateHasChanged);
  }
  public override void OnInitialized() 
  {
    base.OnInitialized();
    this.Binder.OnInitialized();
  }
  public override void OnParametersSet() 
  {
    base.OnParametersSet();
    this.Binder.OnParametersSet();
  }
 /* and other lifecycle methods */
}

It's the component's responsibility to set up the binder correctly, including supplying a StateHasChangedCallback, call the lifecycle methods, and dispose the binder if necessary. This is clumsy, but it won't be a developer's daily life. The point here is to provide a chance to allow creating a base component class which does not derive from MvvmComponentBase, and one project would only need to do that once and for all. So to a certain degree I think it's an acceptable mess to handle, at least it's far better than my current approach (practically I copy-pasted the whole MvvmComponentBase code to make a new class inheriting from AbpComponentBase, plus some ugly code to reflect the internal types like MvvmBlazor.Internal.WeakEventListener.IWeakEventManagerFactory).

For projects which doesn't need to work with other 3rdparty component base classes, they can still stick with the pre-built MvvmComponentBase. Just by the way, OwningComponentBase is equally useful in blazor, so maybe we should have a built-in MvvmOwningComponentBase too.

@klemmchr
Copy link
Owner

klemmchr commented Jun 7, 2021

I'm thinking about using a source generator to prevent duplicating code this way (since it would be the same for MvvmComponentBase and OwningMvvmComponentBase) and this would also make integrating a lot easier. I've never really used them but this looks like a legitimate use case that could benefit from it.

Just by the way, OwningComponentBase is equally useful in blazor, so maybe we should have a built-in MvvmOwningComponentBase too.

Yeah, totally missed that in the current implementation. My plan was to include it in the PR for this issue too.

@hillin
Copy link
Contributor Author

hillin commented Jun 7, 2021

Talking about code generation, it might be worth trying to write a Fody weaver. It's cumbersome to write one, but aside from the built-in components, other developers can also benefit from using the weaver to create their own mvvm component base classes easily.

@klemmchr
Copy link
Owner

klemmchr commented Jun 7, 2021

Since we now got code generators in .NET I'm not a huge fan of Fody weavers any more. They served their purpose, but the built in source generators should do the job. The docs cover our use case.

My plan would be to ship that source generator too (either in a separate NuGet package or directly with the main one). This way, any integration would happen by defining a partial class with a given attribute that would be picked up by the source generator.

@hillin
Copy link
Contributor Author

hillin commented Jun 7, 2021

Great, looking forward to it

@klemmchr
Copy link
Owner

Sorry for the super delayed response, had a lot of moving parts recently. Some months ago I made a working implementation in https://github.com/klemmchr/MvvmBlazor/tree/52-using-a-different-base-component-clas however it was not working with the demo projects. I made some research and I'm quite sure it has to do with the way how those analyzers are being referenced inside a project. However, I was not able to resolve that problem. Would be great if you could have a look ✌🏻

@hillin
Copy link
Contributor Author

hillin commented Sep 13, 2021

@klemmchr I cloned the branch but it does not seem to compile:

BlazorSample.Components\Pages\Clock.razor(20,20,20,24): error CS0103: The name 'Bind' does not exist in the current context
BlazorSample.Components\Pages\Parameters.razor(16,23,16,27): error CS0103: The name 'Bind' does not exist in the current context
BlazorSample.Components\Pages\Parameters.razor(19,29,19,43): error CS0103: The name 'BindingContext' does not exist in the current context

et cetera

Is that the problem we are hunting?

hillin added a commit to hillin/MvvmBlazor that referenced this issue Sep 13, 2021
@hillin
Copy link
Contributor Author

hillin commented Sep 13, 2021

So the problem here is misusing of the ISyntaxContextReceiver. For each compilation unit (assembly), only one instance of each ISyntaxContextReceiver is created and they are being reused again and again, for all GeneratorSyntaxContexts.

In the MvvmBlazor project, there are 3 classes annotated with the MvvmComponentAttribute. The MvvmSyntaxReceiver visits them one by one, and only keeps the ComponentClass and ComponentSymbol values for the last of them, which I see is MvvmOwningComponentBase. Therefore you can see the code was only generated for that last class, but not for other classes, e.g. MvvmComponentBase<T>, which is used in the sample project and thus it did not compile.

With that fixed, some minor issues expose, like MvvmComponentBase<T> and MvvmComponentBase has the same identifier, but you have to generate their code to different files, and the generic source template missed the partial modifier etc.

I've fixed those problems here, the solution now compiles, except for a test project which I did not bother to look: #54

@klemmchr
Copy link
Owner

Looks great, kinda curious how did you debug this?

klemmchr pushed a commit that referenced this issue Sep 13, 2021
@hillin
Copy link
Contributor Author

hillin commented Sep 14, 2021

You place Debugger.Launch() in the source generator code, then you can debug them while building the solution.

@klemmchr
Copy link
Owner

Ok, thats so simple that I'm kinda embarassed that I didn't thought of this 😄

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

Successfully merging a pull request may close this issue.

2 participants