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

100% Java? #40

Open
bootstraponline opened this issue Aug 17, 2018 · 9 comments
Open

100% Java? #40

bootstraponline opened this issue Aug 17, 2018 · 9 comments
Labels
enhancement New feature or request

Comments

@bootstraponline
Copy link
Contributor

Thoughts about adopting Kotlin for mobius? I understand Kotlin can be used with mobius. It'd be nice if mobius was written in Kotlin.

@togi
Copy link
Collaborator

togi commented Aug 17, 2018

There might be a point of a pure kotlin implementation to allow compiling to javascript or native, but it's not something we've discussed at this point.

There are however kotlin extension planned, most likely including coroutine support. We don't really have a timeline for when those will be out, but for the time being you can find some snippets that might end up being included by looking in the kotlin branch of the sample app repository

@pettermahlen pettermahlen added the enhancement New feature or request label Aug 23, 2018
@ATizik
Copy link

ATizik commented Sep 21, 2018

@togi
Would you be interested in a PR containing mobius core converted to pure Kotlin impl without java dependencies? I've done conversion already, every test except logger-related pass. If you're interested I'll fix remaining tests and make a separate package for mobius kotlin core, and another for tests to allow them to be used from jvm to avoid converting tests too.
There are, of course, some places where it can potentially break on other platforms, so it's a work in progress.

@DrewCarlson
Copy link

DrewCarlson commented Sep 21, 2018

@ATizik I've done similar work here if you want to leverage any of it. Note the internal Mobius design will only work on a single thread with K/N, I have a Worker based implementation that I'm experimenting with but much of the core-common code needs to be reworked to support it.

Side note: The Todo sample only implements an iOS client but this project is being used internally on Android, iOS, and Javascript.

@ATizik
Copy link

ATizik commented Sep 22, 2018

@DrewCarlson
That could certainly be helpful, thanks!
I wonder if you experimented with coroutines on K/N instead of threads? I feel like K/N threads are so foreign to traditional JVM threads that it might need a lot of work to get it up and running.

@togi
Copy link
Collaborator

togi commented Sep 22, 2018

I'd certainly be interested in seeing a kotlin branch if you have it, but I think a PR converting everything would be a bit hard to review properly. The main issue isn't really to just convert everything to kotlin, the main concern would rather be maintaining API stability towards previous versions, as well as creating an API that makes sense for kotlin (including kotlin-specific extensions), and supporting kotlin native. Basically, we'd like to make sure we get it right, and I believe the best approach would be to do it iteratively (eg. each pull request focusing on just one part of the public API) and using tools like japicmp to make sure we stay binary-compatible.

I've been meaning to add japicmp-gradle-plugin to the project but just haven't had time lately, maybe someone in the community would be interested in taking that on?

@DrewCarlson We used WorkRunners internally specifically to make mobius agnostic of threading models. So if there are limitations from how we've implemented it, we'd definitely be interested
if you can explain a bit more in detail what the problem is (preferably reported as a serparate issue).

@DrewCarlson
Copy link

@ATizik I did consider coroutines, currently they are not a practical solution here.

Kotlin Native does not provide Thread-like APIs, which does introduce some issues but binary compatibility in a core-jvm module can be maintained. As it exists now, the current design could certainly be supported by K/N by adding the necessary threading primitives. This would require wrapping native target platform APIs which to me feels like a hack and requires more work to support all native targets.

I felt the best solution for K/N was to restructure around Workers. WorkRunner's cannot simply pass a Runnable to a Worker and execute it, extra effort is required to ensure that any data passed to the Worker cannot be mutated by another Worker. Happy to explore possible solutions in a separate issue.

On topic: My port has some breaking changes that are almost all because of sealing off the API, which is a good change IMO (except for any cases I overlooked). Outside of the few remaining issues, compatibility on the Java side is good though existing Kotlin projects require a few changes (as expected).

@togi
Copy link
Collaborator

togi commented Sep 23, 2018

@DrewCarlson ah, I see. If you look at mobius internals you'll see that the only thing the WorkRunners are used for is to create MessageDispatchers, and those are just a building block for something that works a bit like actors in kotlin coroutines. Maybe we should change so that instead of just configuring WorkRunners in the loop factory, you'd be able to configure MessageDispatchers directly? It'd require some changes internally in mobius but it wouldn't require the public API to change. I'll try prototyping the idea a bit and see what it looks like.

@ATizik
Copy link

ATizik commented Sep 24, 2018

Coroutine version of WorkRunner. Because there are no actors in common module I did this primitive solution. Tests pass with this worker, but I'm not sure if they are extensive enough to test corner cases

@togi
So, to make it clear, if I made iterative PR with kotlin conversion it would be helpful? Would you like to eventually convert mobius-core completely to kotlin, or have a separate artifact, at least until conversion is complete?

@bootstraponline
Copy link
Contributor Author

I think it'd be amazing if mobius was fully written in Kotlin.

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

No branches or pull requests

5 participants