avalonia-dotnet-templates icon indicating copy to clipboard operation
avalonia-dotnet-templates copied to clipboard

Add `.UseReactiveUI()` call to xplat template

Open schnerring opened this issue 2 years ago • 11 comments

I was studying the templates and found that xplat is missing the .UseReactiveUI() call.

Related:

  • #19
  • #20

schnerring avatar Apr 27 '22 19:04 schnerring

My change shouldn't break the iOS build, however I don't have an environment where I can test this.

schnerring avatar Apr 27 '22 20:04 schnerring

PR makes sense. Will check later why does IOS fails

Takoooooo avatar Apr 29 '22 15:04 Takoooooo

I don't think ReactiveUI is compatible with iOS of current SDK. It definitely wasn't couple of months ago.

maxkatz6 avatar Apr 29 '22 16:04 maxkatz6

I vote against this change because it introduces a dependency that isn't really required

Gillibald avatar Apr 29 '22 16:04 Gillibald

In this case PackageReference should be removed as well. It's just unused now.

maxkatz6 avatar Apr 29 '22 16:04 maxkatz6

In this case PackageReference should be removed as well. It's just unused now.

It's not unused. The reference to ReactiveUI could be removed, but it would still be referenced implicitly through the AvaloniaTest project reference. AvaloniaTest uses the Avalonia.ReactiveUI package.

schnerring avatar Apr 29 '22 19:04 schnerring

My understanding from comparing the templates is that the project generated from the app-mvvm template and the AvaloniaTest project from the xplat template are functionally the same - both MVVM ReactiveUI projects. Except that platform-specific code was separated from AvaloniaTest into other projects.

My change just consolidates one of the small differences between these projects.

schnerring avatar Apr 29 '22 19:04 schnerring

Build should work with latest reactiveui (18.0.7+) https://github.com/reactiveui/ReactiveUI/issues/3184

maxkatz6 avatar Apr 30 '22 09:04 maxkatz6

Cool!

Do we wait until you bump the version in:

https://github.com/AvaloniaUI/Avalonia/blob/master/build/ReactiveUI.props#L3

I could also push a commit referencing 18.0.7+ explicitly.

schnerring avatar Apr 30 '22 15:04 schnerring

Try merging master to the branch now,i assume it was an issue on our side and i will later think about whether it is a good idea to merge this.

Takoooooo avatar May 04 '22 11:05 Takoooooo

Btw, this call was added only to Desktop call. But not to other projects. I.e. it should be something like this in AppDelegate for iOS:

protected override AppBuilder CustomizeAppBuilder(AppBuilder builder)
{
    return base
        .CustomizeAppBuilder(builder)
        .UseReactiveUI();   
}

It's not unused. The reference to ReactiveUI could be removed, but it would still be referenced implicitly through the AvaloniaTest project reference

It's unused in actual xplat template. Makes sense to add, but main repo should update reactiveUI to 18.x before that.

maxkatz6 avatar May 06 '22 19:05 maxkatz6