ObservableProperty Attribute no longer sending oldValue in On{PropertyName}Changing and On{PropertyName}Changed

[failwyn]

Describe the bug

In CommunityToolkit.Mvvm 8.2.0.0 the ObservableProperty Attribute SourceGenerator is generating code that sends default as the oldValue toOn{PropertyName}Changing(oldValue, newValue) and On{PropertyName}Changed(oldValue, newValue).

Steps to reproduce

1. Create a class that inherits from ObservableObject
2. Create a property with the [ObservableProperty] Attribute
3. look at the code generated by the SourceGenerator
4. default is sent to OnNameChanging(oldValue, newValue) and OnNameChanged(oldValue, newValue)

The correct code is displayed in the documentation at https://learn.microsoft.com/en-us/dotnet/communitytoolkit/mvvm/generators/observableproperty#running-code-upon-changes

When compiling the following code

partial class Customer : ObservableObject
{
    [ObservableProperty]
    //[Required]
    private string name;
}

This code is generated

/// <inheritdoc/>
partial class Customer
{
    /// <inheritdoc cref="name"/>
    [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator", "8.2.0.0")]
    [global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage]
    public string Name
    {
        get => name;
        [global::System.Diagnostics.CodeAnalysis.MemberNotNull("name")]
        set
        {
            if (!global::System.Collections.Generic.EqualityComparer<string>.Default.Equals(name, value))
            {
                OnNameChanging(value);
                OnNameChanging(default, value);
                OnPropertyChanging(global::CommunityToolkit.Mvvm.ComponentModel.__Internals.__KnownINotifyPropertyChangingArgs.Name);
                name = value;
                OnNameChanged(value);
                OnNameChanged(default, value);
                OnPropertyChanged(global::CommunityToolkit.Mvvm.ComponentModel.__Internals.__KnownINotifyPropertyChangedArgs.Name);
            }
        }
    }

    partial void OnNameChanging(string value);
    /// <summary>Executes the logic for when <see cref="Name"/> is changing.</summary>
    /// <param name="oldValue">The previous property value that is being replaced.</param>
    /// <param name="newValue">The new property value being set.</param>
    /// <remarks>This method is invoked right before the value of <see cref="Name"/> is changed.</remarks>
    [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator", "8.2.0.0")]
    partial void OnNameChanging(string? oldValue, string newValue);
    /// <summary>Executes the logic for when <see cref="Name"/> just changed.</summary>
    /// <param name="value">The new property value that was set.</param>
    /// <remarks>This method is invoked right after the value of <see cref="Name"/> is changed.</remarks>
    [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator", "8.2.0.0")]
    partial void OnNameChanged(string value);
    /// <summary>Executes the logic for when <see cref="Name"/> just changed.</summary>
    /// <param name="oldValue">The previous property value that was replaced.</param>
    /// <param name="newValue">The new property value that was set.</param>
    /// <remarks>This method is invoked right after the value of <see cref="Name"/> is changed.</remarks>
    [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator", "8.2.0.0")]
    partial void OnNameChanged(string? oldValue, string newValue);
}

Expected behavior

The oldValue should be sent to OnNameChanging and OnNameChanged instead of the default value for the type.

IDE and version

VS 2022

IDE version

17.5.3

Nuget packages

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

Nuget package version(s)

8.2.0.0

Help us help you

No, just wanted to report this

[Sergio0694]

This is by design, and it's just an optimization. You're seeing default being passed because you're not implementing any of those partial methods that need oldValue, so this way the generated code is avoiding one unnecessary field read. You can try implementing any of those methods and if you check the generated code you'll see it'll automatically change to pass the previous property value as expected. The docs show that code because the method is implemented there 🙂

[holmes-mike]

@Sergio0694 sorry realised this has been closed, I was reviewing the generated code and was also confused by this. If you are recognising whether the partial method has implementation why not remove the call altogether? Not sure if this is more clear?

i.e. if partial method has implementation

    public string Name
    {
        get => name;
        [global::System.Diagnostics.CodeAnalysis.MemberNotNull("name")]
        set
        {
            if (!global::System.Collections.Generic.EqualityComparer<string>.Default.Equals(name, value))
            {
                OnNameChanging(value);
                OnNameChanging(name, value);
                OnPropertyChanging(global::CommunityToolkit.Mvvm.ComponentModel.__Internals.__KnownINotifyPropertyChangingArgs.Name);
                name = value;
                OnNameChanged(value);
                OnNameChanged(name, value);
                OnPropertyChanged(global::CommunityToolkit.Mvvm.ComponentModel.__Internals.__KnownINotifyPropertyChangedArgs.Name);
            }
        }
    }

if no partial methods implemented

    public string Name
    {
        get => name;
        [global::System.Diagnostics.CodeAnalysis.MemberNotNull("name")]
        set
        {
            if (!global::System.Collections.Generic.EqualityComparer<string>.Default.Equals(name, value))
            {
                OnPropertyChanging(global::CommunityToolkit.Mvvm.ComponentModel.__Internals.__KnownINotifyPropertyChangingArgs.Name);
                name = value;
                OnPropertyChanged(global::CommunityToolkit.Mvvm.ComponentModel.__Internals.__KnownINotifyPropertyChangedArgs.Name);
            }
        }
    }

I'm not thinking from a performance perspective. Rather making it obvious what partial methods actually get called with some possible side effect?