thomasclaudiushuber / mvvmgen

MvvmGen is a lightweight MVVM library for XAML applications. It generates your ViewModels on-the-fly for you via a Roslyn-based C# Source Generator.
MIT License
254 stars 21 forks source link

Issue with abstract classes #104

Open wdcossey opened 9 months ago

wdcossey commented 9 months ago

Hi,

Firstly thanks for the great package, it has been helping me get rid of so much boilerplate code!

I have an issue when trying to register a subscriber within an abstract class.

FYI: This is not using the [ViewModelAttribute], I just need subscribe to some events.

public abstract class BaseClass : IBaseClass
{
    protected BaseClass(IEventAggregator eventAggregator)
    {
        _eventAggregator = eventAggregator;
        _eventAggregator.RegisterSubscriber(this);
    }

    ///...
}

public class UseBaseClass : BaseClass, IEventSubscriber<SomeEvent>
{
    protected UseBaseClass(IEventAggregator eventAggregator) : base(eventAggregator) 
    {

    }

    public async void OnEvent(SomeEvent someEvent)
    {
        throw new InvalidOperationException("This will never be subscribed :/");
    }

    ///...
}

The issue is with the RegisterSubscriber method

var subscriberInterfaces = typeof(TSubscriber).GetInterfaces()
    .Where(t => t.IsGenericType && t.FullName?.StartsWith("MvvmGen.Events.IEventSubscriber") == true).ToList();

Specifically typeof(TSubscriber).GetInterfaces() as it will get the base type (i.e. the abstract class), replacing this with subscriber.GetType().GetInterfaces() would solve the issue.

Not sure if this was intentional or just an oversight but I am happy to correct it and open a PR [if it is a bug].

Thanks, Will.

wdcossey commented 9 months ago

Can hack my way around it but would prefer not to use reflection.

typeof(IEventAggregator).GetMethod(nameof(IEventAggregator.RegisterSubscriber))?
            .MakeGenericMethod(GetType())
            .Invoke(_eventAggregator, new object[] { this });
thomasclaudiushuber commented 6 months ago

Hi @wdcossey, sorry for the late reply. I will take a look at this to understand what's going on. And no, it was not intentional that this does not work by default.

I'll come back to you.