dotnet icon indicating copy to clipboard operation
dotnet copied to clipboard

ObservableObject annotation with record not generating OnPropertyChanged and OnPropertyChanging invokers

Open EvilAngel00 opened this issue 1 year ago • 2 comments

Describe the bug

When using the annotations [ObservableObject] and [ObservableProperty] in a class, it correctly generates (at least) two files. One containing the property which when set calls the functions OnPropertyChanging/OnPropertyChanged and another one containing those functions and the handlers for those events.

If we change the class to a record, the first file is correctly generated but not the second, causing the OnPropertyChanging/OnPropertyChanged calls not to compile.

Here's an example project: ObservablePropertyInRecord.zip

Changing the RecordViewModel from a record to a class behaves as expected.

Regression

No response

Steps to reproduce

Example steps to reproduce:

1. Create a `public partial record` with the `[ObservableObject]` annotation
2. Create a variable with the `[ObservableProperty]` annotation
3. Code should not compile and give the errors:

CS0103	The name 'OnPropertyChanging' does not exist in the current context
CS0103	The name 'OnPropertyChanged' does not exist in the current context

Expected behavior

The code generator successfully creates the handlers for a record as it does for a class.

Screenshots

ViewModel as a class:

Screenshot 2023-10-31 093049

Generated code:

Screenshot 2023-10-31 093134 Screenshot 2023-10-31 093146

ViewModel as a record:

Screenshot 2023-10-31 093205

Generated code (the second file is missing):

Screenshot 2023-10-31 093214 Screenshot 2023-10-31 093228

IDE and version

VS 2022

IDE version

17.7.6

Nuget packages

  • [ ] CommunityToolkit.Common
  • [ ] CommunityToolkit.Diagnostics
  • [ ] CommunityToolkit.HighPerformance
  • [X] CommunityToolkit.Mvvm (aka MVVM Toolkit)

Nuget package version(s)

8.2.2

Additional context

No response

Help us help you

No, just wanted to report this

EvilAngel00 avatar Oct 31 '23 08:10 EvilAngel00

Hi there

I checked this one out.

In TransitiveMembersGenerator.cs:67 is a type check with inline declaration:

node is ClassDeclarationSyntax

A record is not recognized as ClassDeclarationSyntax, so the generation skips here.

Changing the code to the following will do the work:

static (node, _) => (node is ClassDeclarationSyntax classDeclaration && classDeclaration.HasOrPotentiallyHasAttributes())
    || (node is RecordDeclarationSyntax recordDeclaration && recordDeclaration.HasOrPotentiallyHasAttributes()),

But do we really want record view models?

best regards

JochenMader avatar Nov 07 '23 18:11 JochenMader

Records should not be view models. The mere nature of the record is to be immutable. The viewmodels, au contraire, is a construct with bindings and designed and tought to be mutable.

DrkWzrd avatar Nov 19 '23 18:11 DrkWzrd