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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: ObservableAsPropertyHelper emits property changed for lazy initial value after first-time reading and doesn't respect DistinctUntilChanged #3682

Closed
AmrAlSayed0 opened this issue Nov 28, 2023 · 1 comment 路 Fixed by #3693
Labels

Comments

@AmrAlSayed0
Copy link
Contributor

AmrAlSayed0 commented Nov 28, 2023

Describe the bug 馃悶

When reading a calculated property backed by a lazy ObservableAsPropertyHelper an INPC event is emitted while the value of the property is still the initial value which is not the case for normal properties.

There is also another bug that if the emitted non-initial value is the same as the initial value it is still emmitted.

Step to reproduce

public sealed class SuperDuperReactiveObject : ReactiveObject
{
	private readonly ObservableAsPropertyHelper<string?> _calculatedPropertyLazyOaph;
	private readonly ObservableAsPropertyHelper<string?> _calculatedPropertyEagerOaph;

	public string? CalculatedPropertyLazy => this._calculatedPropertyLazyOaph.Value;
	public string? CalculatedPropertyEager => this._calculatedPropertyEagerOaph.Value;

	[Reactive]
	public string? ReactiveStringObject { get; set; }

	public SuperDuperReactiveObject(IObservable<string?> calculatedPropertyObs, bool useInternalProperty)
	{
		if (useInternalProperty)
		{
			IObservable<string?> reactiveStringObjectObs = this.WhenAnyValue(vm => vm.ReactiveStringObject);
			this._calculatedPropertyLazyOaph = reactiveStringObjectObs.ToProperty(this, nameof(SuperDuperReactiveObject.CalculatedPropertyLazy), (string?)null, true);
			this._calculatedPropertyEagerOaph = reactiveStringObjectObs.ToProperty(this, nameof(SuperDuperReactiveObject.CalculatedPropertyEager), (string?)null, false);
		}
		else
		{
			this._calculatedPropertyLazyOaph = calculatedPropertyObs.ToProperty(this, nameof(SuperDuperReactiveObject.CalculatedPropertyLazy), (string?)null, true);
			this._calculatedPropertyEagerOaph = calculatedPropertyObs.ToProperty(this, nameof(SuperDuperReactiveObject.CalculatedPropertyEager), (string?)null, false);
		}
	}
}

Case 1

Code

Subject<string?> subject = new Subject<string?>();

SuperDuperReactiveObject ro = new SuperDuperReactiveObject(subject, false);

ro.Changed.Subscribe(e => Debug.WriteLine($"{e.PropertyName} Changed to {e.Sender?.GetType()?.GetProperty(e.PropertyName!)?.GetMethod?.Invoke(e.Sender, Array.Empty<object>())}"));

Debug.WriteLine($"Before Sending a source change");

subject.OnNext("First");

Debug.WriteLine($"After Sending a source change");
Debug.WriteLine(string.Empty);

Debug.WriteLine($"Before Reading Properties");

string? calculatedPropertyLazy = ro.CalculatedPropertyLazy;
string? calculatedPropertyEager = ro.CalculatedPropertyEager;

Debug.WriteLine($"After Reading Properties");
Debug.WriteLine(string.Empty);

Debug.WriteLine($"Before Sending a 2nd source change");

subject.OnNext("Second");

Debug.WriteLine($"After Sending a 2nd source change");

Result

Before Sending a source change
CalculatedPropertyEager Changed to First            //As expected
After Sending a source change

Before Reading Properties
CalculatedPropertyLazy Changed to             //Expected nothing here
After Reading Properties

Before Sending a 2nd source change
CalculatedPropertyEager Changed to Second            //As expected
CalculatedPropertyLazy Changed to Second            //As expected
After Sending a 2nd source change

Case 2

Code

Subject<string?> subject = new Subject<string?>();

SuperDuperReactiveObject ro = new SuperDuperReactiveObject(subject, false);

ro.Changed.Subscribe(e => Debug.WriteLine($"{e.PropertyName} Changed to {e.Sender?.GetType()?.GetProperty(e.PropertyName!)?.GetMethod?.Invoke(e.Sender, Array.Empty<object>())}"));

Debug.WriteLine($"Before Sending a source change");

subject.OnNext(null);

Debug.WriteLine($"After Sending a source change");
Debug.WriteLine(string.Empty);

Debug.WriteLine($"Before Reading Properties");

string? calculatedPropertyLazy = ro.CalculatedPropertyLazy;
string? calculatedPropertyEager = ro.CalculatedPropertyEager;

Debug.WriteLine($"After Reading Properties");
Debug.WriteLine(string.Empty);

Debug.WriteLine($"Before Sending a 2nd source change");

subject.OnNext(null);

Debug.WriteLine($"After Sending a 2nd source change");

Result

Before Sending a source change
After Sending a source change

Before Reading Properties
CalculatedPropertyLazy Changed to             //Expected nothing here
After Reading Properties

Before Sending a 2nd source change
CalculatedPropertyLazy Changed to             //Expected nothing here
After Sending a 2nd source change

Case 3

Code

Subject<string?> subject = new Subject<string?>();

SuperDuperReactiveObject ro = new SuperDuperReactiveObject(subject, true);

ro.Changed.Subscribe(e => Debug.WriteLine($"{e.PropertyName} Changed to {e.Sender?.GetType()?.GetProperty(e.PropertyName!)?.GetMethod?.Invoke(e.Sender, Array.Empty<object>())}"));

Debug.WriteLine($"Before Sending a source change");

ro.ReactiveStringObject = "First";

Debug.WriteLine($"After Sending a source change");
Debug.WriteLine(string.Empty);

Debug.WriteLine($"Before Reading Properties");

string? calculatedPropertyLazy = ro.CalculatedPropertyLazy;
string? calculatedPropertyEager = ro.CalculatedPropertyEager;

Debug.WriteLine($"After Reading Properties");
Debug.WriteLine(string.Empty);

Debug.WriteLine($"Before Sending a 2nd source change");

ro.ReactiveStringObject = "Second";

Debug.WriteLine($"After Sending a 2nd source change");

Result

Before Sending a source change
CalculatedPropertyEager Changed to First            //As expected
ReactiveStringObject Changed to First            //As expected
After Sending a source change

Before Reading Properties
CalculatedPropertyLazy Changed to             //Expected nothing here
CalculatedPropertyLazy Changed to First            //As expected
After Reading Properties

Before Sending a 2nd source change
CalculatedPropertyEager Changed to Second            //As expected
ReactiveStringObject Changed to Second            //As expected
CalculatedPropertyLazy Changed to Second            //As expected
After Sending a 2nd source change

Case 4

Code

Subject<string?> subject = new Subject<string?>();

SuperDuperReactiveObject ro = new SuperDuperReactiveObject(subject, true);

ro.Changed.Subscribe(e => Debug.WriteLine($"{e.PropertyName} Changed to {e.Sender?.GetType()?.GetProperty(e.PropertyName!)?.GetMethod?.Invoke(e.Sender, Array.Empty<object>())}"));

Debug.WriteLine($"Before Sending a source change");

ro.ReactiveStringObject = null;

Debug.WriteLine($"After Sending a source change");
Debug.WriteLine(string.Empty);

Debug.WriteLine($"Before Reading Properties");

string? calculatedPropertyLazy = ro.CalculatedPropertyLazy;
string? calculatedPropertyEager = ro.CalculatedPropertyEager;

Debug.WriteLine($"After Reading Properties");
Debug.WriteLine(string.Empty);

Debug.WriteLine($"Before Sending a 2nd source change");

ro.ReactiveStringObject = null;

Debug.WriteLine($"After Sending a 2nd source change");

Result

Before Sending a source change
After Sending a source change

Before Reading Properties
CalculatedPropertyLazy Changed to             //Expected nothing here
CalculatedPropertyLazy Changed to             //Expected nothing here
After Reading Properties

Before Sending a 2nd source change
After Sending a 2nd source change

Reproduction repository

Code is inline

Expected behavior

Included in the comments in the "Steps to reproduce"

IDE

Visual Studio 2022

Operating system

Windows

Device

Windows PC

ReactiveUI Version

19.5.1

glennawatson pushed a commit that referenced this issue Dec 16, 2023
Fixes #3682

<!-- Please be sure to read the
[Contribute](https://github.com/reactiveui/reactiveui#contribute)
section of the README -->

**What kind of change does this PR introduce?**
<!-- Bug fix, feature, docs update, ... -->



**What is the current behavior?**
<!-- You can also link to an open issue here. -->

**What is the new behavior?**
<!-- If this is a feature change -->

For deferred subscriptions in OAPH 2 changes where made:

- On first `Value` access INPC is no longer emitted while the `Value` is
still the initial value.

- After accessing `Value` for the first time, INPC is no longer emitted
if the value produced from source is the same as the initial value


**What might this PR break?**
I don't think anything will break. It doesn't make sense for any UI
framework to subscribe to INPC without reading the initial value anyway.


**Please check if the PR fulfills these requirements**
- [x] Tests for the changes have been added (for bug fixes / features)
- [x] Docs have been added / updated (for bug fixes / features)

**Other information**:

---------

Co-authored-by: amr <amr.al.sayed@cmaster-eg.com>
Co-authored-by: Chris Pulman <chris.pulman@yahoo.com>
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant