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

Remove ObservableObject base from relay commands #2

Closed
Sergio0694 opened this issue Nov 2, 2021 · 0 comments · Fixed by #30
Closed

Remove ObservableObject base from relay commands #2

Sergio0694 opened this issue Nov 2, 2021 · 0 comments · Fixed by #30
Assignees
Labels
improvements ✨ Improvements to an existing functionality introduce breaking changes 💥 This change would be a breaking change mvvm-toolkit 🧰 Issues/PRs for the MVVM Toolkit optimization ☄ Performance or memory usage improvements

Comments

@Sergio0694
Copy link
Member

Describe the problem this feature would solve

Right now both asynchronous command types inherit from ObservableObject.
This has a number of issues:

  • This also gives the type support for INotifyPropertyChanging, which is not applicable here.
  • This prevents us from being able to add a number of optimizations in the internal properties.

Describe the solution

The following changes need to be done to the two asynchronous command types:

 namespace CommunityToolkit.Mvvm.Input
 {
     public sealed class AsyncRelayCommand :
-        ObservableObject, 
         IAsyncRelayCommand
     {
+        public event PropertyChangedEventHandler? PropertyChanged;
     }
 
     public sealed class AsyncRelayCommand<T> :
-        ObservableObject,
         IAsyncRelayCommand<T>
     {
+        public event PropertyChangedEventHandler? PropertyChanged;
     }
 }

Along with this, some specific optimizations and refactorings can also be applied on their implementation.

Describe alternatives you've considered

We could leave them as they are, but that'd be suboptimal. This would likely not be source breaking for almost everyone.
Still leaving the breaking change label as this could technically be a binary breaking change.

@Sergio0694 Sergio0694 added improvements ✨ Improvements to an existing functionality introduce breaking changes 💥 This change would be a breaking change optimization ☄ Performance or memory usage improvements labels Nov 2, 2021
@Sergio0694 Sergio0694 self-assigned this Nov 2, 2021
@Sergio0694 Sergio0694 changed the title Remove ObservableObject as base class from relay commands Remove ObservableObject base from relay commands Nov 2, 2021
@Sergio0694 Sergio0694 added the mvvm-toolkit 🧰 Issues/PRs for the MVVM Toolkit label Nov 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvements ✨ Improvements to an existing functionality introduce breaking changes 💥 This change would be a breaking change mvvm-toolkit 🧰 Issues/PRs for the MVVM Toolkit optimization ☄ Performance or memory usage improvements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant