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

ObservableProperty and access modifiers #291

Closed
pekspro opened this issue Jun 4, 2022 · 4 comments
Closed

ObservableProperty and access modifiers #291

pekspro opened this issue Jun 4, 2022 · 4 comments
Labels
feature request 📬 A request for new changes to improve functionality mvvm-toolkit 🧰 Issues/PRs for the MVVM Toolkit

Comments

@pekspro
Copy link

pekspro commented Jun 4, 2022

Overview

If I do this:

[ObservableProperty]
private string _title

A Title property is auto generated where both get and set is public. What if you want to limit the access on set? It would be nice of you could define the access modifier in the ObservableProperty constructor.

Usage example

Maybe like this:

[ObservableProperty(AccessModifier.Internal)]
private string _title

This would be Title property is auto generated where set is internal.

Breaking change?

No

Additional context

No response

Help us help you

No, just wanted to propose this

@pekspro pekspro added the feature request 📬 A request for new changes to improve functionality label Jun 4, 2022
@Sergio0694
Copy link
Member

This is not supported and not planned. We want to support this properly in C# 12 via partial properties. For now the plan is to work on a proposal for that to add to the working set, and I don't think it'd be a good idea to hack a clunkier alternative for this just for the short term until C# 12 was out (which would be november 2023). That would also solve a whole lot of other problems related to the fact we're currently using fields in the first place 🙂

@ewerspej
Copy link

@Sergio0694 That's too bad, because right now, we cannot use the [ObservableProperty] Source Generator for properties that need private, protected or internal modifiers. C# 12 might not be usable in legacy projects, either. Other Source Generators, such as [RelayCommand], also allow to modify the resulting code.

@Sergio0694
Copy link
Member

C# 12 should work on downlevel projects just fine (eg. see https://github.com/Sergio0694/PolySharp).
For now, I'd recommend just keeping the setters public, and just be careful not to set them externally. I'd argue the benefit of using [ObservableProperty] and removing all the extra boilerplate is still very much worth it. After all it's not unlike when you'd expose an ObservableCollection<T>, even though you'd only want the owning viewmodel to ever modify it 🙂

@ewerspej
Copy link

ewerspej commented Apr 15, 2023

Well, you never know who will use or touch the code. Making the setter explicitly private is being done on purpose for good reason. Making the setter public comes with a risk and incurs technical debt, because making the pending change once a proper solution is available must not be forgotten.

You're right that compiler features should be backwards compatible with existing projects as long as no runtime changes are required. Still, I don't think that this should be dependent on a C# language version if it could be solved with Source Generators and Analyzers already now.

Don't get me wrong, I understand your reasoning and it has its merit. I'm just unhappy about the fact that I cannot convert certain properties to use Source Generators unless I make changes that affect the behavior and the API of the existing code (if we consider a public setter of a public property an API).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request 📬 A request for new changes to improve functionality mvvm-toolkit 🧰 Issues/PRs for the MVVM Toolkit
Projects
None yet
Development

No branches or pull requests

3 participants