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

Fix extensions to fail in compile time on nullable types #201

Merged
merged 1 commit into from Oct 30, 2018
Merged

Fix extensions to fail in compile time on nullable types #201

merged 1 commit into from Oct 30, 2018

Conversation

krossovochkin
Copy link
Contributor

  1. Kotlin is Null-safe. That means that each possible NPE should be found during compilation, not in runtime
  2. RxJava2 doesn't allow nulls
  3. In order to make generics in Kotlin to allow only non-null types it is required to state it explicitly (like <T : Any>)
  4. Each file maybe.kt, flowable.kt etc. already has this

This PR is about patching observables.kt, singles.kt etc. to not allow nullable types.

Thanks.

@thomasnield
Copy link
Collaborator

It should be noted that all these extensions are likely to be deprecated anyway, with experimental new type inference in Kotlin 1.3

https://youtube.com/watch?t=1447&v=MyljSWm0Y_k

Ill merge later.

@krossovochkin
Copy link
Contributor Author

Hi @thomasnield, I got your point, but have small question.

It is clear that this repo is mostly kotlin-wrapper around rxjava2.
Though in the future I hope it will take some advantage of kotlin native to have pure-kotlin implementation.

In this case imho I think having more stable kotlin-public-API should be more important than relying on kotlin-java SAM interface.
I know that all observable.kt, maybe.kt etc. are not really kotlin-specific, but why we can't define these extension files (observables.kt, et.c) as public API?.
And if we remove these extension files later on (using kotlin 1.3) it sounds like we'll move closer to JVM instead of kotlin stuff.

Just my humble opinion. Feel free to decide when/whether to merge it or not.

Thanks!

@thomasnield thomasnield merged commit 1a5baf8 into ReactiveX:2.x Oct 30, 2018
@thomasnield
Copy link
Collaborator

thomasnield commented Oct 30, 2018

Just merged. I think the reality is if Kotlin ever gets a reactive API, it is going to be completely from scratch and will not rely on the signatures of RxJava API.

Also, it will be a lot of work to create a stable multiplatform API. My hope is JetBrains will get involved in doing that.

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

Successfully merging this pull request may close these issues.

None yet

4 participants