Version 0.10.0 - Static Language Support by benjchristensen · Pull Request #304 · ReactiveX/RxJava · GitHub

Manual merge of pull #300 from @mattrjacobs

This will make RxJava completely static by removing all Object overloads (see #208 and #204).

I'm submitting this before it being 100% ready so people can review and provide feedback.

Open items to append to this pull request before merging:

1) subscribe with map is not handled yet

The following signature needs to be made static. Right now the lack of this combined with removal of Functions.from dynamic language functionality has broken this.

public Subscription subscribe(final Map<String, Object> callbacks)

2) Core artifact naming convention

Should rxjava-core-x.y.x.jar become rxjava-x.y.z.jar since the concept of core+language no longer applies?

I think I'd prefer this:

rxjava-x.y.z.jar
rxjava-groovy-x.y.z.jar
rxjava-clojure-x.y.z.jar
rxjava-scala-x.y.z.jar
rxjava-jruby-x.y.z.jar
rxjava-kotlin-x.y.z.jar
rxjava-dynamic-x.y.z.jar (object overload for any language)
rxjava-groovy-clojure-x.y.z.jar (multi-language jar)

Only one of those jars is needed hence the reason why I think the 'core' term is no longer needed as it communicated the fact it was always needed.

Any contrib modules would be: rxjava-contrib-module-name-x.y.x.jar

3) Dependencies from languages to core still exist

The build still will result in Maven Central POM files requires rxjava-core from the language version despite that not being the case. Need to eliminate this dependency in the artifact.

Implementation notes originally posted at #204 (comment):

After implementing and throwing away a few different approaches we have landed on a solution we feel will balance the various competing priorities.

Our goals are:

support static typing for Java/Scala/Kotlin etc by removing the Object overloads
support any JVM language, static or dynamically typed
allow all languages to use the same rx.Observable class so that we don't divide libraries with helpers such as GroovyObservable, ClojureObservable etc that then need to be converted back and forth when doing interop
do not require special classloaders or agents to enable runtime bytecode generation
do not remove static operators to enable proxying
small jars and limited or no dependencies

The solution we have arrived at will work as follows:

The rxjava-core source code will delete all Object overload methods and be pure static java.
- Any language that supports functional interfaces directly (such as Java 8 and XTend) can use the Java core version directly.
Languages needing specific lambda/clojure type mapping to the Func_/Action_ types will have language specific Jars created via build-time bytecode generation.
- Any method with a Func_/Action_ argument will be overloaded with a version supporting the language requirements.

For example:

The default Java version:

public static <T> Observable<T> filter(Observable<T> that, Func1<T, Boolean> predicate)

A Groovy version:

public static <T> Observable<T> filter(Observable<T> that, groovy.lang.Closure predicate)

A jar per language will be created as follows:
- rxjava-x.y.z.jar
- rxjava-groovy-x.y.z.jar
- rxjava-clojure-x.y.z.jar
- rxjava-scala-x.y.z.jar
- rxjava-jruby-x.y.z.jar
- rxjava-kotlin-x.y.z.jar

A project will include just the jar that meets their language needs, there will no longer be a "core" jar plus the language adaptor.

The drawback of this is that mixing two of these in a classpath will result in non-deterministic loading (whichever is loaded last wins) and that is the version that will be used. This means if a library depends on rxjava.jar but is using Groovy and needs rxjava-groovy.jar it is up to the developer of that project to make sure they have only the rxjava-groovy.jar version. This is not ideal but is a one-time pain setting up a build and is better than the constant pain of missing static typing or converting to/from different Observable implementations for different languages.

At this time we are optimizing for projects using a single language or Java + another language. If there are use cases where people are trying to mix multiple languages in a very polyglot manner we have two options:
- include an rxjava-dynamic.jar version that re-adds the Object overloads
- include build configs for common combinations of languages such as rxjava-groovy-clojure.jar
Language adaptations (such as clojure which has preferred idioms that necessitate wrapping) will still be possible through the language-adaptor projects and be included in the appropriate language jars.

This should not break any code but will require a slight change to the build dependencies in your project when we release this.

We hope that this enables the RxJava project to better achieve its goal of being polyglot and targeting the JVM in general and not any specific languages without sacrificing interop or idiomatic usage in each of the languages.

…age's native function support * Only hooked up to rxjava-groovy in this commit

* Adds an Object overload to Observable methods via code generation

…ode generation

…n body

…to version_0.10.0_multi-static Conflicts: rxjava-core/src/main/java/rx/Observable.java

I couldn't get these to work once we removed the Object overloads that was apparently being invoked instead of the staticly typed method so removing them. If someone else can get these generics working then please submit a new pull request.

These methods will need different names as they have the same argument count so break with dynamic overloads once we do static handling of dynamic languages.

- needed for Scala build to succeed

RxJava-pull-requests #174 SUCCESS
This pull request looks good

I don't think I follow your point about making the subscribe(Map<String, Object>) method static. Wouldn't that break all existing scripts that use this method? Could we add a subscribe(Map<String, Closure>) or subscribe(Map<String, IFn>) variant in the dynamic JARs? That would also suggest tightening up the core signature to subscribe(Map<String, Function>).
This naming convention makes sense to me. I will change rxjava-core to rxjava in the artifact name, at least. I think I will leave the directory rxjava-core alone, since it makes more sense there, and IMO, it would be confusing to have a dir like RxJava/rxjava/... I don't plan on bringing rxjava-kotlin along with this pull request, but I can review Kotlin Language Adaptor #292 to help fit that into the 0.10 world.
I think I understand this, though I'm already at the edge of my Gradle ability. If I'm understanding this correctly, we should not change the build steps for a project - we just need to change the generated Maven POM. That makes sense - I'll figure out a way to accomplish that.

Looks good, all in all. I'll try this out with my Scala sample as soon as possible. Thanks for the awesome work!

Thanks @jmhofer for stepping in to test out these changes.

@mattrjacobs Answers to your questions ...

subscribe(Map)

If we leave the Map<String, Object> then the Functions.from capability needs to keep working.

The other option is we change that to be Map<String, Function> as you suggest and then auto-generate the versions for each language.

Thanks, and agreed on the directory name, but that can change if we want in the future without impacting the artifact name once you set the name.
Maven POM

I think the simplest solution is just not have any "compile" dependencies, only "provided". I believe your Gradle scripts just need to adjust to include provided dependencies, not just compile runtime dependencies and then it will work. I would not try messing with POM files directly. If something is specified as "provided" then it is used for compilation but not included in the POM when pushed to Maven Central.

RxJava-pull-requests #175 SUCCESS
This pull request looks good

RxJava-pull-requests #176 FAILURE
Looks like there's a problem with this pull request

RxJava-pull-requests #177 SUCCESS
This pull request looks good

Currently, if you publish RxJava to your local Maven repo, you'll find that the rxjava-scala and rxjava-swing POMs include a dependency on "rxjava-core", which doesn't exist anymore as a module, as it was renamed to "rxjava".

Unfortunately, I don't know enough about Gradle to be able to fix this. My guess would be that it's not enough to set the baseName of the JAR to just "rxjava". There should be a way to tell gradle that the module is called "rxjava" even though the directory is called "rxjava-core".

Here's another problem: Excluding "*$UnitTest" from JARs that might be included by Scala will cause the Scala compiler to crash with a Typer error because it expects defined inner classes to exist. This means that at least in rxjava-swing, the exclude can't be done.

I've created a mini-PR against the unit test problem here: benjchristensen#1

I got my little sample (https://github.com/jmhofer/rxjava-samples) working with this PR. The code itself was very easy to adapt. The implicits seem to be working fine.

One small thing I noticed: The very long names of the implicits are a bit distracting, imho. Should I want to use one of these functions explicitly, I'd prefer shorter names like simply func1 instead of scalaFunction1ToRxFunc1, for example.

Responses to @jmhofer:

POM references to rxjava-core: In the Scala case, I prefer including all of the core Rx classes directly in the rxjava-scala.jar, so that there is no dependency at all. This avoids the problem entirely. For rxjava-swing, I'll work on the Gradle side to make the reference correct. This might take the form of changing the reference or actually changing the name of rxjava-core to rxjava - we'll see how friendly Gradle is.
Excluding inner classes: I've also come across the Scala compiler error on Classfile parsing with excluded inner classes. At the moment, unit tests exist in rxjava-core.jar and rxjava-scala.jar only. Going forward:
- rxjava-core: These unit tests only exist so that rxjava-scala can be built. I will exclude them, add a new unfilteredJar Gradle task, and build rxjava-scala off of unfilteredJar.
- rxjava-scala: I will continue to include UnitTests, since it's pretty likely that Scala projects are consuming this
- rxjava-swing: I hadn't considered the case of Scala consuming this, but you're obviously right. For now, I'll err on the side of caution and unilaterally add the UnitTests back to the jar. At some later point, we might want to re-examine (rxjava-swing-scala.jar/rxjava-swing-filtered.jar). But adding them back is the safe thing to do. Your PR is exactly what I will do, thanks for that.
Implicit names: In my experience, it's rare that I call implicit conversions by name, but I can see your point. My only concern is ambiguous namings. Would changing 'scalaFunction1ToRxFunc1' to 'toRxFunc1' (and other similar naming changes) be satisfying?

@mattrjacobs: All your suggestions seem sensible to me.

Concerning implicit naming: It may be relatively rare, however using the implicits often leads to your having to specify the parameter types of the closures being implicitely converted, which can be more verbose and confusing than making the conversion explicit and then being able to shorten the closure syntax.

Names like toRxFunc1 are ok with me, not too long and still unambiguous, sounds like a good compromise.

RxJava-pull-requests #181 SUCCESS
This pull request looks good

RxJava-pull-requests #182 SUCCESS
This pull request looks good

RxJava-pull-requests #183 SUCCESS
This pull request looks good

RxJava-pull-requests #188 FAILURE
Looks like there's a problem with this pull request

Replaced by #319

mattrjacobs and others added 14 commits July 3, 2013 23:11

Make rxjava-core typesafe

7f1a989

Add Code generator that adapts the core Observable to a dynamic langu…

c93ba88

…age's native function support * Only hooked up to rxjava-groovy in this commit

Clean up rxjava-groovy Gradle build

bbedef2

Get rxjava-clojure build working in Gradle

698508f

Get rxjava-jruby build working in Gradle

5ac401d

Added rxjava-dynamic language-adaptor

d4efc29

* Adds an Object overload to Observable methods via code generation

Reworked Scala adaptor to use implicits in RxImplicits, rather than c…

4d45c10

…ode generation

Added license to files I touched and moved Gradle config around

5d10dbf

Bugfix to codegeneration - explicitly call a static/instance method i…

73c8631

…n body

Merge branch 'multi-static' of git://github.com/mattrjacobs/RxJava in…

8beb820

…to version_0.10.0_multi-static Conflicts: rxjava-core/src/main/java/rx/Observable.java

Remove '? super' generic definitions

227c442

I couldn't get these to work once we removed the Object overloads that was apparently being invoked instead of the staticly typed method so removing them. If someone else can get these generics working then please submit a new pull request.

Removing zip FuncN overloads - they conflict with dynamic languages

b831bf8

These methods will need different names as they have the same argument count so break with dynamic overloads once we do static handling of dynamic languages.

Upgrade to Gradle 1.6

add859f

- needed for Scala build to succeed

Version 0.10.0

e8373fa

benjchristensen mentioned this pull request Jul 5, 2013

Type-safety in rxjava-core, dynamic language support via compile-time code generation, and Scala support via implicits #300

Closed

benjchristensen mentioned this pull request Aug 14, 2013

Initial support for scheduling on Android Handler threads #318

Merged

4 tasks

mattrjacobs mentioned this pull request Aug 19, 2013

typesafe-core #319

Closed

benjchristensen closed this Aug 20, 2013

Version 0.10.0 - Static Language Support by benjchristensen · Pull Request #304 · ReactiveX/RxJava · GitHub

Uh oh!

Version 0.10.0 - Static Language Support - #304

Version 0.10.0 - Static Language Support#304benjchristensen wants to merge 14 commits intoReactiveX:masterReactiveX/RxJava:masterfrom benjchristensen:version_0.10.0_multi-staticbenjchristensen/RxJava:version_0.10.0_multi-staticCopy head branch name to clipboard

Conversation

benjchristensen commented Jul 5, 2013

Uh oh!

cloudbees-pull-request-builder commented Jul 5, 2013

Uh oh!

mattrjacobs commented Jul 6, 2013

Uh oh!

jmhofer commented Jul 6, 2013

Uh oh!

benjchristensen commented Jul 6, 2013

Uh oh!

cloudbees-pull-request-builder commented Jul 6, 2013

Uh oh!

cloudbees-pull-request-builder commented Jul 6, 2013

Uh oh!

cloudbees-pull-request-builder commented Jul 6, 2013

Uh oh!

jmhofer commented Jul 8, 2013

Uh oh!

jmhofer commented Jul 8, 2013

Uh oh!

jmhofer commented Jul 8, 2013

Uh oh!

jmhofer commented Jul 8, 2013

Uh oh!

mattrjacobs commented Jul 8, 2013

Uh oh!

jmhofer commented Jul 9, 2013

Uh oh!

cloudbees-pull-request-builder commented Jul 12, 2013

Uh oh!

cloudbees-pull-request-builder commented Jul 12, 2013

Uh oh!

cloudbees-pull-request-builder commented Jul 12, 2013

Uh oh!

cloudbees-pull-request-builder commented Jul 31, 2013

Uh oh!

benjchristensen commented Aug 20, 2013

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Version 0.10.0 - Static Language Support#304
benjchristensen wants to merge 14 commits into
ReactiveX:masterfrom
benjchristensen:version_0.10.0_multi-static