Skip to content

Commit

Permalink
Fix stop oaph omitting inappropriately (#3693)
Browse files Browse the repository at this point in the history
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 <[email protected]>
Co-authored-by: Chris Pulman <[email protected]>
  • Loading branch information
3 people authored Dec 16, 2023
1 parent 65377aa commit 0db5167
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -290,10 +290,62 @@ int GetInitialValue()
Assert.Equal(42, result);
}

/// <summary>
/// Tests that Observable As Property Helpers initial value should emit initial value.
/// </summary>
/// <param name="initialValue">The initial value.</param>
/// <summary>Test that Observable As Property Helpers defers subscription with initial function value doesn't call on changed when subscribed.</summary>
/// <param name = "initialValue" >The initial value.</param>
[Theory]
[InlineData(default(int))]
[InlineData(42)]
public void OAPHDeferSubscriptionWithInitialFuncValueNotCallOnChangedWhenSubscribed(int initialValue)
{
var observable = Observable.Empty<int>();

var wasOnChangingCalled = false;
Action<int> onChanging = v => wasOnChangingCalled = true;
var wasOnChangedCalled = false;
Action<int> onChanged = v => wasOnChangedCalled = true;

var fixture = new ObservableAsPropertyHelper<int>(observable, onChanged, onChanging, () => initialValue, true);

Assert.False(fixture.IsSubscribed);
Assert.False(wasOnChangingCalled);
Assert.False(wasOnChangedCalled);

var result = fixture.Value;

Assert.True(fixture.IsSubscribed);
Assert.False(wasOnChangingCalled);
Assert.False(wasOnChangedCalled);
Assert.Equal(initialValue, result);
}

/// <summary>Test that Observable As Property Helpers defers subscription with initial function value doesn't call on changed when source provides initial value after subscription.</summary>
/// <param name = "initialValue" >The initial value.</param>
[Theory]
[InlineData(default(int))]
[InlineData(42)]
public void OAPHDeferSubscriptionWithInitialFuncValueNotCallOnChangedWhenSourceProvidesInitialValue(int initialValue)
{
var observable = new Subject<int>();

var wasOnChangingCalled = false;
Action<int> onChanging = v => wasOnChangingCalled = true;
var wasOnChangedCalled = false;
Action<int> onChanged = v => wasOnChangedCalled = true;

var fixture = new ObservableAsPropertyHelper<int>(observable, onChanged, onChanging, () => initialValue, true);

var result = fixture.Value;

Assert.Equal(initialValue, result);

observable.OnNext(initialValue);

Assert.False(wasOnChangingCalled);
Assert.False(wasOnChangedCalled);
}

/// <summary>Tests that Observable As Property Helpers initial value should emit initial value.</summary>
/// <param name = "initialValue" >The initial value.</param>
[Theory]
[InlineData(default(int))]
[InlineData(42)]
Expand Down
17 changes: 11 additions & 6 deletions src/ReactiveUI/ObservableForProperty/ObservableAsPropertyHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -154,18 +154,23 @@ public ObservableAsPropertyHelper(
ex => _thrownExceptions.Value.OnNext(ex))
.DisposeWith(_disposable);

_getInitialValue = getInitialValue!;
_getInitialValue = getInitialValue ??= () => default(T?);

if (deferSubscription)
{
_lastValue = default;
Source = observable.DistinctUntilChanged();
// Although there are no subscribers yet, we should skip all the values that are equal getInitialValue() instead of equal default(T?) because
// default(T?) is never accessible anyway when subscriptions are deferred. We're going to assume that the current value is getInitialValue() even
// if it hasn't been evaluated yet
Source = observable.SkipWhile(x => EqualityComparer<T?>.Default.Equals(x, getInitialValue() /* Don't use field to avoid capturing this */))
.DistinctUntilChanged();
}
else
{
_lastValue = _getInitialValue();
Source = observable.StartWith(_lastValue).DistinctUntilChanged();
Source.Subscribe(_subject).DisposeWith(_disposable);
Source = observable.StartWith(_lastValue)
.DistinctUntilChanged();
Source.Subscribe(_subject)
.DisposeWith(_disposable);
_activated = 1;
}
}
Expand All @@ -184,7 +189,7 @@ public T Value
if (localReferenceInCaseDisposeIsCalled is not null)
{
_lastValue = _getInitialValue();
Source.StartWith(_lastValue).Subscribe(_subject).DisposeWith(localReferenceInCaseDisposeIsCalled);
Source.Subscribe(_subject).DisposeWith(localReferenceInCaseDisposeIsCalled);
}
}

Expand Down

0 comments on commit 0db5167

Please sign in to comment.