ReactiveX / RxJava

RxJava – Reactive Extensions for the JVM – a library for composing asynchronous and event-based programs using observable sequences for the Java VM.
Apache License 2.0
47.91k stars 7.6k forks source link

Proposal: Move to Java 8 #6317

Closed ZacSweers closed 5 years ago

ZacSweers commented 5 years ago

With Android Gradle Plugin 3.2.0 being stable now, it brings a new D8 tool that includes Bazel's desugaring of Java 8 code. This has enabled the android library ecosystem to start moving to Java 8 (language features first, possibly backporting of APIs like streams in the future).

As a result, the android ecosystem (as well as libraries) have started being able to migrate to Java 8. Since that was an original design goal of RxJava 2, but backported to Java 6 for android compatibility, I propose we reconsider that now that the ecosystem has moved forward.

If the proposal's accepted, we could do most of the bulk work fairly easily in an automated pass with IDE tooling.

Type annotations could be safely introduced solely for static analysis purposes (ref #6316)

Example community projects that have successfully moved to Java 8 (some android, some java, some both)

Thoughts?

akarnokd commented 5 years ago

RxJava is set out to support Android 2.3+, does this recent tooling enhancements still work with such old runtimes?

Run project-wide inspections from IntelliJ

I did the conversion back and forth the time and IntelliJ just gave up after a few files or created a non-compilable mess due to type-inference mismatches. Had to do it individually. Also this doesn't help the main library much as we moved to non-anonymous classes.

ZacSweers commented 5 years ago

Yep! It compiles down to Java-6 compatible bytecode as far as I know (the detailed docs say any minSdkVersion)

I did the conversion back and forth the time and IntelliJ just gave up after a few files or created a non-compilable mess due to type-inference mismatches. Had to do it individually.

Interesting. We did the same across our monorepo (several hundred thousand LoC) at the beginning of this year and found the tooling to be mostly seamless and quite reliable. Maybe it's improved since you tried?

Also this doesn't help the main library much as we moved to non-anonymous classes.

I figured, but at the same time it's a big step forward for the community, brings the project up to more modern tooling/language features, and potentially opens interesting doors for the project in the future

akarnokd commented 5 years ago

There are a few extra considerations:

ZacSweers commented 5 years ago

RxJava runs on desktop/server where we also pledged Java 6 as a baseline.

Since Java 8 is already reaching EoL, supporting it as a minimum seems like a fair thing. Ubiquitous major libraries like Guava are also Java 8 only now (including using Java 8 APIs).

Is it just the Android compiler or is there standard library support behind it, such as java.util.Objects and java.time.Duration?

Ish. The issue I linked is requesting for a newer version of Desugar which does support desugaring of apis like those. For now, D8 should be treated as only syntax backporting.

I'm not sure what consideration for reactor 3 is for? (unfamiliar with the project aside from knowing it exists)

slisaasquatch commented 5 years ago

This is just my two cents. While I acknowledge that Java 8 support is nice to have, I don't think it's going to make a huge difference. There's already RxJava2Jdk8Interop, and it's easy enough to convert between Duration and TimeUnit.

artem-zinnatullin commented 5 years ago

If this is applied, we'll need to add desugaring as CI check, otherwise nothing prevents contributors from referencing JDK APIs that can not be desugared

ZacSweers commented 5 years ago

If this is applied, we'll need to add desugaring as CI check, otherwise nothing prevents contributors from referencing JDK APIs that can not be desugared

Animal sniffer can cover that. Example: https://github.com/uber/AutoDispose/pull/264

Also another consideration - targeting java 6 is actually deprecated in gradle's java support at this point:

warning: [options] source value 1.6 is obsolete and will be removed in a future release
warning: [options] target value 1.6 is obsolete and will be removed in a future release

For me, it's not just a "nice to have". The combination of evolving tooling requirements and community direction risk could (in my opinion) make not adopting a risk.

ZacSweers commented 5 years ago

Any further thoughts on this? Happy to send a proposal PR if you'd rather

akarnokd commented 5 years ago

We can't just switch in a minor/patch version anyway and I'd prefer Java 9 so that we can be dependency-free.

catt-stefano commented 5 years ago

Just to make sure you guys are aware of this (probably are), to use a library written in Java 8 in an Android project, you would need to enable java 8 source compatibility in the app itself, just to support the Java 8 library. So this extra step would be necessary for making the library work:

// this goes in the app gradle file
compileOptions {
   sourceCompatibility JavaVersion.VERSION_1_8
   targetCompatibility JavaVersion.VERSION_1_8
}