Add support for [ObservableProperty] on partial properties

[Sergio0694]

Overview

This issue tracks adding support for partial properties to the [ObservableProperty] generator. This is blocked on dotnet/csharplang#6420, so support for this will have to wait for that first (hopefully in the C# 12 timeframe).

API breakdown

The changes are pretty straightforward: properties will also be allowed on [ObservableProperty]:

-[AttributeUsage(AttributeTargets.Field, AllowMultiple = false, Inherited = false)]
+[AttributeUsage(AttributeTargets.Field | AttributeTargets.Property, AllowMultiple = false, Inherited = false)]
 public sealed class ObservablePropertyAttribute : Attribute
 {
 }

Analyzers will also need to be added to ensure that the annotated properties are partial and valid.

Usage example

Code like this:

[ObservableProperty] 
public partial string Name { get; private set; }

Will generate the following:

/// <inheritdoc cref="_itemsSource"/>
[global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator", "8.1.0.0")]
[global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage]
public partial string Name
{
    get => field;
    set
    {
        if (!global::System.Collections.Generic.EqualityComparer<string>.Default.Equals(field, value))
        {
            OnNameChanging(value);
            OnPropertyChanging(global::CommunityToolkit.Mvvm.ComponentModel.__Internals.__KnownINotifyPropertyChangingArgs.Name);
            
            field = value;

            OnItemsSourceChanged(value);
            OnNameChanged(global::CommunityToolkit.Mvvm.ComponentModel.__Internals.__KnownINotifyPropertyChangedArgs.Name);
        }
    }
}

/// <summary>Executes the logic for when <see cref="Name"/> is changing.</summary>
[global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator", "8.1.0.0")]
partial void OnNameChanging(string value);

/// <summary>Executes the logic for when <see cref="Name"/> just changed.</summary>
[global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator", "8.1.0.0")]
partial void OnNameChanged(string value);

This will also enable support for declaring custom accessibility modifiers for property accessors.

[cmiles]

@Sergio0694 I haven't tried to track a language proposal before and couldn't immediately tell from the Partial Properties proposal/labels/information whether it is still expected to be available in the November 2023 time frame that you mentioned in #291 - since you have been tracking and adding to the the Partial Properties proposal/discussion any chance you can add any current information about time frame estimates here?

Fwiw From what I've read here and in other issues Partial Properties seem like a great solution - just wondering if it is truly 'coming soon' or if this pushes into 2024 and beyond.

Thanks for your time and any update!

[Sergio0694]

Hey @cmiles! We don't have any updates/ETAs to share at this time. Keep in mind that no C# features at all are confirmed until the moment they actually ship, so the same applies here. We're still actively looking into this though, just no news just yet 🙂

[modmynitro]

Its not in C# 12
dotnet/csharplang#6420 (comment)

[cmiles]

@Sergio0694 since this did not make it into C#12 any chance that of any of the associated issues will be reconsidered for action in a more immediate time frame?

I understand partial properties would facilitate the ideal implementation of things like 'Required' keyword support - but there is some functionality blocked by this that would be immediately useful and appreciated even if there were breaking changes in the future if/when partial properties make it into C#.

[Sergio0694]

"but there is some functionality blocked by this"

What functionality is blocked?

[cmiles]

What functionality is blocked?

@Sergio0694 - It was my understanding that all of the issues/functionality listed below are blocked by wanting to wait and create an implementation based around the partial property language feature.

For me the functionality the required keyword offers is particularly attractive as we work on enabling nullable support in our code - but I'm not aware of any way to combine [ObservableProperty] and required?

Based on your question of 'What functionality is blocked?' maybe I am mistaken and there is a feature or approach that I'm not aware of to use [ObservableProperty] and required together - if so apologies for the wasted time/space in this issue but would very much appreciate a link to get me started!

Add support for "required" and "virtual" keywords to property generator · Issue #679 · CommunityToolkit/dotnet
Make ObservableProperty Generated Code Properties Virtual? · Issue #543 · CommunityToolkit/dotnet
Add support for setter protection level for ObservableProperty · Issue #758 · CommunityToolkit/dotnet
Add abilities to control the access level of generated [ObservableProperty] attribute · Issue #585 · CommunityToolkit/dotnet

[Sergio0694]

"all of the issues/functionality listed below are blocked"

"For me the functionality the required keyword offers is particularly attractive"

No functionality is actually blocked. You can easily use all that — just manually define the properties 🙂

I understand the frustration for wanting a simpler way to do that, and I do agree that of course using the generator is better. But I don't think it's a good idea to dedicate resources to implement all that functionality when we might very well have partial properties out in preview soon, especially because then we'd still have to keep supporting that "workaround" to avoid breaking consumers. Rather, it makes more sense to just wait for partial properties to be available in preview, add support for those, and then release a preview of the MVVM Toolkit too so that people wanting to try those out can do so as well.

[cmiles]

@Sergio0694 No frustration here - seems like we just have a different evaluation of the issue.

Fwiw I would still '+1' working on a solution now - but respect and understand your reasoning and decision and no doubt you have 100% better insight on the odds of partial properties arriving in a near future preview.

I hope this issue will continue to be re-evaluated if for some reason partial properties, or something equivalently useful for generators, don't arrive for the next version of C#.

Thanks for the prompt replies - appreciated!

[jhm-ciberman]

This was not added to C# 12. Is there any reason to not implement the other more traditional alternatives using attributes?

[Sergio0694]

This exact question and the answer for that are in the messages right above yours — the reason has not changed and it's the same as before 🙂

[AignerGames]

Partial properties are just in proposel state, it's not confirmed that they will be release any time soon (or at all). I don't think that it is a good idea to wait for some unknown future time and close all releated issues.

I understand your argument, that the current solution is a workaround, but it's a workaround that already exists, so adding more features to the already existing / working "workaround" would be better for every current user of the toolkit. You can declare it obsolete as soon as partial properties are implemented (if ever) and then you don't have to worry about maintaning it in the future.

Would it really take that many resources to add a additional parameter to control the accessor kind of the generated property to justify waiting for partial properties?

[DennisWelu]

@AignerGames is right on. I’ve had similar thoughts in the past about this issue. It could be years or never, yet the value is high and the effort seems at least relatively low. Worth the trouble of a depreciation period if needed.

[jhm-ciberman]

Personally, I don't use generators in MVVM Toolkit at all only for this sole issue.

[jhorv]

@Sergio0694 frustration here.

The fact that [ObservableProperty]s are public-public causes me to barely use it. If only I could specify a private set, then almost every one of my view models could have its lines of code cut in half. What's the point of deriving from ObservableObject if I'm always the one calling SetProperty()? I could have that with an extension method, for crying out loud.

It's becoming increasingly difficult to convince people (and believe myself) that I did a smart thing in choosing this library. To me it doesn't matter how, whether via attribute or some future C# language feature. Just add the ability and make these features useful for more than just demos.

[Sergio0694]

I understand the frustration, however how I've already said multiple times in several related issues, my goal is to structure the Toolkit in a way that ensures the best path forward for it and for users, and I don't think trying yo shoehorn additional APIs to an already arguably "hacky" API solely for a temporary workaround is the right solution. We have millions of users and if we rolled that out, it's also something we'd likely have to maintain for at least some time, as we wouldn't want to just break everyone by immediately obsoleting and removing that API once we did get partial properties.

You can also check the Roslyn feature status and you will see that partial properties have now been added to the working set. I'll prototype support for the MVVM Toolkit and publish a preview version with them as soon as they're available in a preview .NET SDK. Once again, this has always been the plan.

"What's the point of deriving from ObservableObject if I'm always the one calling SetProperty()? I could have that with an extension method, for crying out loud."

That is just incorrect. You wouldn't be able to raise an event from an extension method, only code within that class can raise events. This is just a C# limitation. Also I'm not even sure what you mean here, deriving from ObservableObject is not even required at all. It's just recommended to minimize binary size.

I will also say, as a side node and personal opinion, I don't really understand how not having support for private setters is a deal breaker. Yes, it's not as clean, but being pragmatic, it genuinely doesn't matter. Just ignore that, use the generators on fields, don't use the setters from the outside for now, and just go back and update the code once we get partial properties. This is exactly the same we're doing in the Microsoft Store app. The benefit of using the generators and saving hundreds and hundreds of lines of code vastly outweighs the fact that "oh this setter in this viewmodel is public but it should be private, even though literally nobody is setting from the outside anyway so it doesn't make any difference in practice". You're building an app, not a library, so you own the entire stack. Just don't use the setters for now, it will be fine.

[AignerGames]

I don't see the high maintenance or fear of breaking the feature in the future as valid, because the C# syntax will not suddenly change so drastically that a setter modifier will be handled totally different or handled differently all the time.

But after reading the comments and other issues it's very clear that you will not allow it no matter what, so I will give up.

[weitzhandler]

Having private/protected/virtual setters or changing/changed methods is essential. Please add properties to the Observable property attribute to control that.

[Sergio0694]

"I don't see the high maintenance or fear of breaking the feature in the future as valid, because the C# syntax will not suddenly change so drastically that a setter modifier will be handled totally different or handled differently all the time."

@AignerGames I think there might be a misunderstanding here. It's not the C# syntax that would change. It's the fact that if we released support for customizing setters on [ObservableProperty], then people would start using it, then C# would get support for partial properties (which again, it's literally in the working set now), and once that shipped we'd add support for that. We'd then be in the situation where we'd either have to support both, which we don't want, or deprecate and remove that temporary support for customizing setters on fields, which would mean breaking all of those consumers that had started using that new feature. We don't want that. We want people to just switch to the right thing (partial properties), only have to change their code once, and never get broken by us deprecating that feature in the first place.

"that you will not allow it no matter what, so I will give up"

It's not like I will not allow it just because, I just want to do things right from the start, especially now that partial properties are finally in the working set and might be available soon, is all. I do want this feature to be available to everyone, and I also do want it myself 🙂

"Having private/protected/virtual setters or changing/changed methods is essential. Please add properties to the Observable property attribute to control that."

@weitzhandler it is certainly a nice to have and an important feature, but it is not essential (or there wouldn't be several production apps using it despite that lack of support for now). As we mentioned in the rest of this thread, we have no plans on adding that feature for fields. We plan on supporting that for partial properties as soon as they're added to C#, however.

[todor-dk]

@Sergio0694 I am with you. I really want private, protected etc., but keep the implementation clean. I am willing to wait 6 months if that's what it takes.

On why I want private / protected / etc.; The answer is easy: maintainability of my code. If it is private, when refactoring code, I know there won't be anybody outside of my class fiddling with the property - less headache when refactoring a class. Same goes for the other modifiers. And this is especially important if the property is a critical one.

Second, a thing I hope something partial properties will solve, which the current infrastructure cannot solve, is the ability to find references in the IDE. Today, if I have an observable property, I cannot easily take references, as if I try on the field, let's say on _Name, I only get the auto-generated setter, and then I have to go one level further and find references on Name. This is just inconvenient, and if partial properties will solve this, I am willing to wait few months.

[DennisWelu]

Requests for this capability in this library go back to 2022 from what I can see. AFAICT, the C# feature we are waiting on (dotnet/csharplang#6420) is still in the proposal stage at this point? I'm not seeing any indication it will be included this fall. Could be missing something there. If that's true, I think people have been holding out for "a few months" for quite some time now. Am I correct in assuming that if later this feature is added to the language then related changes in ObservableProperty will largely be in the generated code? Seems like minimal impact on the usage protocol for ObservableProperty itself.

High value in this feature. Worth a deprecation period if necessary.

[jhm-ciberman]

If you don't want to keep supporting a feature in the long run, can we just add it with a message in the documentation that says something like:

"This feature will be deprecated and removed once partial properties are added to the C# language. If you use this feature, you acknowledge this."

[jphorv-bdo]

Why deprecate the attribute approach at all? It is already a compile error to add [ObservableProperty] in a type that isn't ObservableObject/[ObservableObject]/[INotifyPropertyChanged]. If access level can be specified via attribute, it could simply be a compile error to specify it on a partial property. There are companies who don't move to the latest/greatest versions immediately, and there will be people who would rather specify their observable properties via a field than a partial property.

[Sergio0694]

"is still in the proposal stage at this point?"

@DennisWelu like I mentioned, it's in the working set. Here you can see the test plan (and the VS 17.12 milestone) and a link to the feature branch. You can see work has started and there's a couple of open PRs starting to add support for it 🙂

"If you don't want to keep supporting a feature in the long run, can we just add it with a message in the documentation that says something like"

@jhm-ciberman I don't want to spend weeks implementing the feature just to deprecate it shortly after.

"Why deprecate the attribute approach at all?"

@jphorv-bdo because I don't want to keep having to maintain two parallel implementations in the source generators. We might keep support for the current attribute on fields, but I don't want to also add the complexity for custom accessors, and having to maintain that too for potentially a long period of time. Like I said, I want to just get this right from the start.

[DennisWelu]

@Sergio0694 Thanks for the pointer to the ongoing work! Hopefully since work has started it will make the final cut. Seems hopeful...

Also appreciate your efforts to keep the API clean and keep efforts focused/manageable. Good things for sure.

My understanding is you would not maintain the Attribute approach after this feature is available. Which means consumers are going to want to update code en masse anyway at some point? That's fine by me. Changing over might be tedious but it should be a clear-cut recipe.

Still...probably I don't understand the "weeks" of implementation needed but I imagine adding something in the meantime as simple as SetterVisibility=Visibility.Private as an option to ObservableProperty, and all that does is change the visibility keyword used in the generated code. FWIW.

This will continue to be important to our team because we are porting a bunch of Xamarin projects over to MAUI and the one big change is incorporating the MVVM toolkit where possible. The choice today for read-only properties is to make them read-write, or don't adopt generated properties. But there's SO much goodness in them it's hard not to. It will be hard to find all these properties later because nothing discretely points them out if you go looking for them.

[AignerGames]

Still...probably I don't understand the "weeks" of implementation needed but I imagine adding something in the meantime as simple as SetterVisibility=Visibility.Private as an option to ObservableProperty, and all that does is change the visibility keyword used in the generated code. FWIW.

I don't have experience with this code base, so my information could be wrong, but after a quick check I think that only a single place of the generator would have to be changed here to add the modifier type.

Well and the code to define the enum somewhere, modify the attribute to accept the enum and maybe unit tests.

[jhorv]

I will also say, as a side node and personal opinion, I don't really understand how not having support for private setters is a deal breaker. Yes, it's not as clean, but being pragmatic, it genuinely doesn't matter. Just ignore that, use the generators on fields, don't use the setters from the outside for now, and just go back and update the code once we get partial properties. This is exactly the same we're doing in the Microsoft Store app. The benefit of using the generators and saving hundreds and hundreds of lines of code vastly outweighs the fact that "oh this setter in this viewmodel is public but it should be private, even though literally nobody is setting from the outside anyway so it doesn't make any difference in practice". You're building an app, not a library, so you own the entire stack. Just don't use the setters for now, it will be fine.

For my organization and project, it matters a lot. The project I'm working on isn't just an app, but also a collection of libraries that other internal development teams will use. There is a client API library, a view model library, and a UI component library, and other devs may use all of it or just parts. As the provider of these libraries, it is a very big deal that these libraries be hard to use incorrectly. If Intellisense shows that something can be done/set/called then it must be a valid operation. That is the point of object-oriented encapsulation/data hiding, and all these public:public properties is the opposite of that: a nightmare for supportability, dev productivity, and our project's success.