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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consolidate core Rx libs into single library/package #199

Closed
clairernovotny opened this issue Jun 21, 2016 · 18 comments
Closed

Consolidate core Rx libs into single library/package #199

clairernovotny opened this issue Jun 21, 2016 · 18 comments
Assignees
Milestone

Comments

@clairernovotny
Copy link
Member

Right now the core of Rx is split into many smaller libraries. There were many reasons why this was done for 2.0 but today it seems like extra complexity and provides a negative perception of Rx as "big and heavy".

Further, most people don't really download "just the interfaces" or just bits and pieces as evidenced by these recent download stats from NuGet.org:

Package Downloads
Rx-Main 494,225
Rx-Interfaces 566,329
Rx-Core 569,340
Rx-Linq 565,726
Rx-PlatformServices 518,551

The above table show that most people download all of the core libraries together with a few people skipping the meta-package (Rx-Main).

This proposal is to take the core libraries and combine them into a single dll and package (System.Reactive). With xproj and whatever that morphs into with MSBuild, cross-compiling many libraries is much easier than it was in the past, so it's easy to light-up as much as possible per platform.

@anaisbetts
Copy link
Contributor

I've heard this complaint constantly in my libraries that use Rx, +:100: to having a single library. There's no non-bananas use-case to have version X of Rx-Interfaces and version Y of Rx-Core

@clairernovotny clairernovotny modified the milestones: vNext, Future Jun 21, 2016
@LeeCampbell
Copy link
Contributor

Agree with @paulcbetts that Rx-Interfaces and Rx-Core should probably be merged (as System.Reactive?). Personally I think I always pull in Rx-Linq too (renamed to System.Reactive.Linq?), but libraries authors may not need that dependency?

Would Platform Services disappear in the .NET Standard world? I have thought before that the packages could be stripped back to System.Reactive, System.Reactive.PlatformServices (if even required anymore) and then the UI Specific ones as totally options System.Reactive.Xaml/System.Reactive.WinForms (or what ever they may be called).

I obviously don't want to have to pull Xaml/WPF/UWP/SL/Winform dependencies for Console/Web/Server applications or libraries.

@RolandPheasant
Copy link

Like @LeeCampbell I always pull in Rx-Linq and have never seen any benefit with to separating Rx-Linq, Rx-Core and Rx-Interfaces so I am in favour of these being merged.

My instinct would be to keep the UI specific libraries separate as I only use these when necessary.

@raffaeler
Copy link

What's the name of the WPF package now?
Nuget sadly misses a decent filter ability by author.

TIA

@shiftkey
Copy link
Contributor

shiftkey commented Aug 1, 2016

@raffaeler
Copy link

Looks like the right one, thank you.

I would suggest to put a verbose description on all the available packages or at least in the readme.md of this repo.

@shiftkey
Copy link
Contributor

shiftkey commented Aug 1, 2016

@raffaeler would you be interested in submitting a PR to do this? There's a list of the nuspec files for Rx.NET and Ix.NET in the repository already...

@danielcweber
Copy link
Collaborator

When writing these tests I thought that it would be cool to factor out all the various xDisposable classes. They are definitely useful even outside the Rx context.

@danielcweber
Copy link
Collaborator

danielcweber commented Aug 5, 2016

There's a possible case where we might want to keep the Interfaces library separated. Some operators in Ix.Async should have an overload that takes an IScheduler, e.g. AsyncEnumerable.ToObservable. It's synchronous counterpart in Rx (to convert IEnumerable to IObservable) has an overload that schedules calls to MoveNext.

I agree that the use of scheduling a synchronous operation (IEnumerator.MoveNext) is more obvious than the use of scheduling an asynchronous operation (IAsyncEnumerator.MoveNext), however, it might still be useful.

@RxDave
Copy link
Contributor

RxDave commented Aug 5, 2016

I'm still on the fence about this one. I can't seem to convince myself that the separation is necessary, nor that it makes sense to change it at this point.

What's the benefit of separate libraries?

Core and Linq seem like a nice organizational separation, and in fact Core is 113 KB while Linq is 693 KB. Core provides types that implement the Rx contracts over the native IObservable<T> and IObserver<T> interfaces; e.g., ObservableBase<T>, ObserverBase<T>, schedulers and disposables, including the core implementation for platform enhancements. Linq contains all of the useful operators. However, I can't see why anyone would choose to use Core without Linq. If you're using Core for the implementations and Rx contracts, then Linq seems like the obvious best way to consume it. I could see how it would be useful to separate out Disposables though; e.g., System.Dispsoables (there's nothing inherently "reactive" about those.)

The Interfaces library seems like a nice separation too, until you notice that it's only 24 KB. But more importantly, these interfaces have the Rx contracts on them implicitly; it's not like some program can interoperate over those interfaces with a third-party API, where one uses them with Rx's strict contracts and the other does whatever it wants. Therefore, perhaps Interfaces should be merged with Core, even if still separated from Linq and PlatformServices.

So what's the benefit of creating one library out of the Interfaces, Core, Linq and PlatformServices?

It's to ease some people's fears that Rx is "big and heavy". And who are these people? I haven't run into that problem yet. And so what if they think that? They're wrong, unless they actually think that ~1 MB is large and heavy for a small set of libraries that implements a stable, mathematically sound, reactive programming paradigm with complete implementations for LINQ Standard Operators and expression trees.

-- I figured somebody had to try and at least defend the separation, otherwise we're not being totally honest :-)

@danielcweber
Copy link
Collaborator

danielcweber commented Aug 5, 2016

I myself also used to add Rx-Linq to everything and it was just fine, so I'm all for unification of Core and Linq with one or two exceptions:

  1. Extracting the disposables has IMO a very valid justification, which is at least 150 lines of duplicated stuff in Ix.Async. In the past, I have imported Rx-Core into projects in the past just to get the Disposables.
  2. As to whether the scheduler implementations should be factored out, I don't have a strong opinion yet. In Readme.md it says "Rx = Observables + LINQ + Schedulers" - why not reflect this in the packages?

If anyone can see the necessity of using the Rx scheduler interfaces in Ix or elsewhere, these should probably be left separated. I can see the point about Rx's strict contracts but don't think that package layout will keep anybody from using any interfaces as he or she pleases. After all, isn't that good design, separating interfaces from implementations?

So I would vote for: Interfaces+Disposables+(Reactive.Core&Linq) or Interfaces+Disposables+Schedulers+(Reactive.Core&Linq).

@shiftkey
Copy link
Contributor

shiftkey commented Aug 8, 2016

We've talked a bunch here about how to structure things, and I worry about things going more side-tracked without resolving this. Things like "Disposables should be a standalone thing" and "Schedulers should be available in Ix.NET" are very interesting proposals, on top of what we were originally discussing.

If I help organize some sort of RFC-esque process not unlike the swift-evolution repository, to document how we go about achieving these changes, would that interest others?

I mostly want to do this so:

  • we have some sort of documentation so we can all get on the same page
  • have a set of proposals to drive the roadmap for the project, and
  • communicate more clearly outside this repository what's happening here

Thoughts?

@clairernovotny
Copy link
Member Author

clairernovotny commented Dec 9, 2016

Here's a justification for doing this... Implemeting #205, while solving the issue caused by loading an earlier assembly version, has caused lots of confusion and pain.

Here's sample of issues related here:
#305
#299
#296
#264

And those are just the people who bothered to report issues. Consolidating into a single System.Reactive assembly lets us version appropriate while avoiding a requirement for binding redirects. I'd like to do that for version 4.0.

As to the other ideas about a separate disposables or schedulers, library, we can do that for the next version. The goal here is a fairly quick release that solves the pain above.

The downside here is that it's a breaking change (hence the major version increment) because you can't use a binding redirect out of this. We'll be introducing a new dll, so you can use v3 and v4 SxS, but a common library that needs v3 won't "just work" with v4...at least not easily. If this is a show-stopper, we might be able to create a compatibility package that includes TypeForwarders.

Extracting schedulers and disposables in the future could be made non-breaking by way of TypeForwarders.

@shiftkey
Copy link
Contributor

shiftkey commented Dec 9, 2016

Unless we have significant reasons to avoid it, I'm also 👍 for doing this with v4

@clairernovotny
Copy link
Member Author

Looks like there's a few options for tooling for creating a set of TypeForwarders. This would ensure compat between v3 and v4:

https://github.com/mono/mono/blob/3baad19c6c34891fa04c59d9673cbbd7c91e5ac2/mcs/class/Facades/netstandard/TypeForwarders.cs#L5

https://github.com/dotnet/buildtools/tree/master/src/GenFacades

There'd be a System.Reactive.Compatibility package that would bring in a set of type forwarders. I imagine it should not be a System.Reactive dependency by default but certainly anyone using v4 that wants to use libs compiled against v3 would add that extra package to make things work.

@clairernovotny clairernovotny self-assigned this Dec 16, 2016
@clairernovotny
Copy link
Member Author

A quick update -

I have an early cut at this here in the v4.0-proto branch:
https://github.com/Reactive-Extensions/Rx.NET/tree/v4.0-proto

It combines most of the libraries into System.Reactive and targets netstandard 1.3, net45, net46 and uap10.0.

There’s very few differences between the platforms these days. The main difference with uap over netstandard is support for the winrt bits and no thread/threadpool.

Xamarin gets netstandard 1.3. net45 and net46 are mostly the same as netstandard 1.3 with the addition of the platform-specific bits there (forms/wpf/remoting). The only diff between 4.5 and 4.6 is support for TaskCompletionSource.TrySetCanceled(CancellationToken). Net45 doesn’t support that; the others do.

The two libs that are separate are:

  • Microsoft.Reactive.Testing: Testing shouldn't be merged
  • System.Reactive.Observable.Aliases: These have other names for Rx terminology. The main issue is that it contains a namespace System.Reactive.Observable.Aliases and that conflicts with having a type System.Reactive.Observable in a couple of uses. I don't see the harm in leaving these as a separate lib as I suspect they're rarely used.

I still need to re-wire packaging, signing and the test projects. I also have not yet actually moved the files to minimize merge/rebasing issues until we're ready.

@clairernovotny
Copy link
Member Author

This is now complete and available on the MyGet feed

@RehanSaeed
Copy link

RehanSaeed commented Aug 13, 2019

I'm trying to use System.Reactive in a UWP app but it adds 9MB to a 30MB app which is a problem as we are highly space constrained. Is my best bet to stick with v3?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants