Nullability issue with DisposeWith at Bind, OneWayBind

[imckl]

Describe the bug

The compiler warns about "[CS8631] Nullability of type argument doesn't match constraint type." when building application with c# 8.0 Nullable reference types enabled in .netcore3.1

Steps To Reproduce

Code behind xaml:

this.Bind(ViewModel,
    vm => vm.PropertyA,
    v => v.SomeButton)
    .DisposeWith(disposable);

When build the application, the complier warns about [CS8631]

Expected behavior

Should be without any warning.

It seems that System.Reactive.Disposables.DisposeWith does not except a Nullable value as parameter(which writes as IDisposable Extensions in class DisposableMixins)

I've write a new DisposeWith which accepts nullable value

public static T? DisposeWith<T>(this T? item, CompositeDisposable? compositeDisposable) where T : class?, IDisposable
{
    if (compositeDisposable == null)
    {
        throw new ArgumentNullException(nameof(compositeDisposable));
    }

    if (item == null)
    {
        return null;
    }
            
    compositeDisposable.Add((IDisposable)item);
            
    return item;
}

But it conflicts with original DisposeWith Extension because they share same method signature.
Hence renaming to DisposeWithNullable or some other might be a workaround, tills lots of code should be modified to fix this wanring.

So what's the proper way to resolve this Nullability issue?

Screenshots

Environment

• OS: Windows 10 x64, .net core 3.1
• Version 11.5.17
• Device: WPF

Additional context

[imckl]

Possible duplicate issue with #2467

[glennawatson]

I think the problem here is the fact that Bind returns a disposable value when it probably shouldn't.

[imckl]

I think the problem here is the fact that Bind returns a disposable value when it probably shouldn't.

With recent update, bind returns nullable disposable. And the null value itself does not need to dispose, of course.
But how to handle this "null disposable"?
Hence more, why bind accepts a nullable ViewModel, which in fact bind action on a null value doesn't make any sense.

Hmmm, I'm confused with the nullable parameter ViewModel in bind method now.
If meets c# Nullable Reference is a must, should we just leave ViewModel (eg: in bind method) as non-nullable parameter?

[glennawatson]

The view model isn't the issue. I will work on a fix tomorrow.

[Noggog]

This is not a duplicate of #2467.
I was about to open an issue/PR about this myself, haha. I think either:

1. Bind needs to not return a nullable object
2. DisposeWith needs to take nullable, like you suggest
3. Glenn's suggestion of having Bind not return anything if that's proper. Would definitely be the cleanest

If meets c# Nullable Reference is a must, should we just leave ViewModel (eg: in bind method) as non-nullable parameter?

This is because the ViewModel is being passed in not because it's actually being used within the function. Rather, it's there to provide typing information on what would otherwise be a massive generic you would need to write out. By passing it in, the typing can be inferred, and so you don't have to write it. And since it's not actually being used, it's made nullable so that it lines up with ReactiveControl's member, which is also nullable.

[glennawatson]

Hold off on any PRs for the moment just doing some stuff at the moment related.

[glennawatson]

Fixed in ac9e712

[github-actions]

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.