NonSerializable/XMLIgnore Attributes with ICommand/ObservableProperty attributes

[Aurumaker72]

Describe the problem

A way/special attribute to be used in conjunction with ICommand or ObservableProperty which signalizes the backing field not to be serialized

Describe the solution

  [ICommand]
  [SpecialXMLIgnore]
  private void DoSomething()
  {
     MessageBox.Show("Hello");
  }

Example which should compile to

[XmlIgnore]
private IRelayCommand? doSomethingCommand;
public IRelayCommand DoSomethingCommand => doSomethingCommand ??= new RelayCommand(DoSomething);

Alternatives

No response

Additional info

No response

Help us help you

No response

[ghost]

Hello, 'Aurumaker72! Thanks for submitting a new feature request. I've automatically added a vote 👍 reaction to help get things started. Other community members can vote to help us prioritize this feature in the future!

[michael-hawker]

FYI @Sergio0694 this was in the WCT repo, @Arlodotexe we may want to touch base on Monday quick and look at our GitHub Issue categories or something and call out MVVM Toolkit better to redirect here, as I've been noticing a few issues still getting filed over for WCT.

Also linking back to this comment here by @skint007 which was the same request:

Would it be possible to have Attributes carried over to the generated public properties?

Example:

[ObservableProperty]
[NotMapped]
private Person? _selectedPerson;

Currently it doesn't look like they are applied.

[timunie]

Forwarded post /idea

https://github.com/CommunityToolkit/dotnet/issues/8#issuecomment-1093268742

[michael-hawker]

Bubbling up the solution proposed by @timunie:

I think other Attributes can be added like AlsoNotifyChangeFor. Would be

[AttributeDecorator(nameof(MyAttribute))]
private string myField;

That wouldn't account for if extra constructor arguments are needed though.

Would we be able to use params or something to take-in any constructor arguments? i.e.

[AttributeDecorator(nameof(SerializeThing), "SomeOption = true", "SomeOtherOption = 3")]
private string _myField

// To Generate:
[SerializeThing(SomeOption = true, SomeOtherOption = 3)]
public string MyField { get; set; }

Not sure if anything better than a raw string could be used...

Then once https://github.com/dotnet/csharplang/discussions/3412 is implemented, this could just all go away in a future version? (but at least still be relatively simple to migrate back over?)

[Sergio0694]

As a related work item, I think we should try to get more traction on that proposal and talk to the Roslyn folks.

cc. @333fred you can see not having access to partial properties is causing a significant amount of issues for us here. I'd be happy to help with a proposal if you'd be willing to champion this, if you think that could be helpful to get this going 😄

[Sergio0694]

So, there is one big elephant in the room here: not all attributes are valid on both fields and properties.

We need to remind ourselves that what we're doing today in the MVVM Toolkit is just a "best effort", but we'll just never be able to actually land on a "perfect" solution until C# actually gets support for partial properties (see https://github.com/dotnet/csharplang/discussions/3412). That said, we can try to make this at least a bit better. What we could do is: for all attributes on a field marked with [ObservableProperty] that is not one of ours, check whether the attribute type also supports properties as targets. If it does, forward it. If it does not, just ignore it. This would help with some scenarios (eg. [JsonIgnore] would work), but not all. I could implement this if we all agree it'd be at least better than what we have today.

Thoughts? 🙂

cc. @michael-hawker @Arlodotexe @Insire @timunie @powerdude, @sonnemaf?

[powerdude]

I like the idea above. Also, to complete the picture and for those scenarios where an attribute can't be on a field, but can be on a property, I think consideration should be made for the AttributeDecorator mentioned above. Maybe call it AlsoAddAttribute

[timunie]

So, there is one big elephant in the room here: not all attributes are valid on both fields and properties.

I am happy with whatever you choose. But tbh, I think "forwarding" an attribute may lead to unpredictable situations:

• You want the attribute for the field, but it can also be added for properties ► wrong forwarded
• I want an attribute to be forwarded, but it failed. Will I ever know this?
• The attribute should only apply to the property, but not for the field itself. Will this be possible?

Happy coding Tim

[Sergio0694]

Yes, that is what made me not do this in the first place. I do have many issues with this. As I said, I really don't see a "perfect" solution here, the fact remains that this approach is just fundamentally troublesome. This is why for now I've only special cases the validation attributes and only had them being forwarded to properties.

Honestly I'm kinda leaning towards only doing so (especially since we're close to 8.0 stable), just saying that special atributes need a manual property for now, and then potentially revisit this post 8.0, or just push for getting partial properties in C# 12.

[powerdude]

Great points. I think these would lead to being explicit then and something like the AttributeDecorator would be the way to go.

[Sergio0694]

I really don't like AttributeDecorator though, as it would completely kill the build-time safety of using other attributes and passing parametrs to it. As in, we'd have to essentially reimplement all the logic to verify that some arguments are valid for any of the constructors for the target attribute type, which is not a trivial problem (it's a very, very hard problem, and something best left to the compiler). I don't like the alternative of just YOLOing it and forwarding with no checks, as then invalid arguments would cause hard to understand errors about invalid parameters coming from generated files, so I could see users just scratching their head at that. To clerify: I do see how we have some kind of limitation here, I'm just not seeing a good enough solution 🤔

[Insire]

I agree with Sergio. Let's postpone this feature until there is a solid solution, instead of implementing a error prone one.

[powerdude]

Ok, fair enough. What about the use of a proxy class? Something like this:

public partial class MyViewModel : ObservableObject
{

[ObservableProperty]
[AttributeProxy(typeof(Proxy)
private string prop1;

// this class only exists to provide example properties for the generator to use and pull attributes from
private class Proxy
{
  [SomeAttribute]
   public string Prop1 { get;set;}
}
}

[Sergio0694]

Honestly I'm thinking the only proper way would be to have opt-in forwarded attributes, like:

[ObservableProperty]
[SomeAttribute]
[SomeOtherAttribute]
[AlsoForwardAttribute(typeof(SomeAttribute))] // opt-in
private string name;

This would instruct the generator to only forward SomeAttribute and not the other. Then we could also check that that attribute is valid on a property as well. But, downsides: verbose. Also I don't think this is something we should rush into 8.0.

What we might do is add this as a preview feature (ie. with [PreviewFeature] on it). This would have the compiler check this, and it'd emit an error unless you explicitly added <EnablePreviewFeatures>true to your .csproj. But then... Eh, not sure 🤔

[timunie]

Tbh I think postpone is the best we can do here. But just my two cents.

[michael-hawker]

@Sergio0694 in either case for now regardless of when we do this, should we have an info/warning diagnostic that if an "unrecognized"/un-auto-forwarded (i.e. non-validation) property is found that it calls that out? At least then there wouldn't be confusion about why something isn't working as expected. Something like:

"MVVMTK0024: Warning: The attribute name attribute is decorating the private field, but is not forwarded to generated property property name."

---

Would it make sense to make it a negative case of [ExcludeAttribute(typeof(SomeAttribute))]?

[Sergio0694]

This is now superseded by #413, closing this one.