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

RxJava 3 Support #359

Closed
ZacSweers opened this issue Jun 27, 2019 · 7 comments · Fixed by #381
Closed

RxJava 3 Support #359

ZacSweers opened this issue Jun 27, 2019 · 7 comments · Fixed by #381

Comments

@ZacSweers
Copy link
Collaborator

ZacSweers commented Jun 27, 2019

RxJava 3 is planned for later this year, but will use the same maven coordinates and package name. There is one significant change that affects AutoDispose: the as() operator has been folded back into to(), where the original to() overloads accepting Functions has been replaced with as()'s use of custom converter interfaces.

For Java consumers - this basically means no change is required from the AutoDispose side. Consumers would need to switch to to(), but all the static autoDisposable() APIs would work out of the box with that in RxJava 3. All we need to do is update docs for usage in both.

This does present an interesting problem for the kotlin extensions though, which all call through to .as() under the hood and would no longer function in RxJava 3.

My current thoughts right now are to try to avoid requiring a V2 for AutoDispose, and instead make RxJava 3 support somehow opt-in. An idea for this is to wrap support behind an abstraction in an rx3-only module compiled against rxjava 3. Then you can opt in the kotlin extensions to this via new AutoDispose plugin, and under the hood it would switch to use the rx3 support.

The rx3 support module would have something like this

// Ugly name but we can't mark this as internal since it's another module, but maybe we can make it harder to discover
fun <T> Observable<T>.rx3toInteropForAutoDispose(converter: ObservableConverter<T, ObservableSubscribeProxy<T>>): ObservableSubscribeProxy<T> = to(converter)

Then in the main kotlin extensions we'd do something like this

fun <T> Observable<T>.autoDisposable(scope: ScopeProvider): ObservableSubscribeProxy<T> {
  val converter = AutoDispose.autoDisposable<T>(scope)
  return if (AutoDisposePlugins.useRx3()) {
    rx3toInteropForAutoDispose(converter)
  } else {
    `as`(converter)
  }
}

We'd need to add some sort of proguard warning suppression for consumers not using rx3 to safely ignore it missing from the classpath

@ZacSweers
Copy link
Collaborator Author

Alternatively - maybe we can use -Xfriend-paths to allow making the interop function internal but still accessible from autodispose artifacts

@ZacSweers
Copy link
Collaborator Author

We'd also need to update static analysis tools to handle both. I think that could be easy if we basically just check the signature of to() first

@ZacSweers
Copy link
Collaborator Author

we could also make the autodispose plugin more functional in nature, so we can skip the if-check. Main issue is that the plugins are written in java but the interop function would be kotlin

@ZacSweers
Copy link
Collaborator Author

Another incompatibility: defer factories have now changed to use Supplier instead of Callable: https://github.com/ReactiveX/RxJava/blob/3.x/src/main/java/io/reactivex/Completable.java#L358

That throws another wrench in trying to support both

@ShaishavGandhi
Copy link
Collaborator

Seems like we're going to get a new package and groupId
ReactiveX/RxJava#6606 (comment)

@ZacSweers
Copy link
Collaborator Author

Better than before but still annoying for us as we'll need to release a new artifact pointing to that package as well. I wonder if there's a way we can support both in builds somehow

@ZacSweers
Copy link
Collaborator Author

After we release 1.4.0, we'll start 2.x snapshots pointing to rxjava 3

@ZacSweers ZacSweers added this to the 2.0.0 milestone Sep 6, 2019
@ZacSweers ZacSweers pinned this issue Sep 15, 2019
@ZacSweers ZacSweers unpinned this issue May 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants