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

Kotlin Multiplatform #67

Open
sveltema opened this issue Nov 17, 2023 · 9 comments
Open

Kotlin Multiplatform #67

sveltema opened this issue Nov 17, 2023 · 9 comments
Assignees
Milestone

Comments

@sveltema
Copy link

I have rewritten a version of this library for Kotlin Multiplatform that is in use in a production app. My version has some of the same items you are currently iterating on, Effects using Flows, Effect cancellation (by id and by scope) among other changes. You can find a version of it here.

Since there is very little in this framework that is JVM specific, in the name of avoiding duplication of effort, I would be more than happy to contribute to this library and use this framework if it supported KMP.

Is there any chance you would consider supporting KMP in this library? I'd be happy to contribute to or do the implementation.

@heytherewill
Copy link
Contributor

Hey there, Steven :)

I was discussing with @semanticer a couple of days ago that once 1.0 was out we should start working on Multiplatform! I was gonna do it, namely because I have some Android projects that I'm migrating to Multiplatform 😂, but if you want to take it over I see it as an absolute win :)

@sveltema
Copy link
Author

Lovely! I would love to work on a Multiplatform version. There will need to be discussions about certain aspects of this work. Some that come to mind ATM are which platforms to support (I would expect to eventually support jvm, macOS, iOS, watchOS, tvOS, js, linuxX64, mingwX64 and wasm), and if the library is not generally for use directly from native code (at least to begin with) but for use in multiplatform or jvm projects.

@semanticer
Copy link
Collaborator

Your Repo looks great Steven! I'll leave the multiplatform stuff to you and @heytherewill but I'd like to take a closer look at the ScopedActions, buffering, and batching actions you've implemented there.

I'd merge our PR first. Then, we can kick off new issues and tweak what we've got.

Also, noticed you axed the subscriptions from your version. I get why they seem less crucial now with flow-based effects on the horizon, but they're still unique, especially since they work with State and respond to its changes. Maybe the smart move is to spin off subscriptions into an optional, standalone module? 🤔

@sveltema
Copy link
Author

When I axed them, I thought that with long lasting effects added into the mix, the use case for subscriptions was a little indistinct. However, I can see situations where they could be useful. When using my library, I did end up needing something like subscriptions but I created them outside the framework by collecting the store StateFlow and sending actions when I needed to do so. This allowed me the flexibility of independent cancelling and scoping of the subscriptions separate from the framework. That said, I don't really have a strong opinion about keeping Subscriptions or separating them out. I chose to ax them to remain similar to TCA, and the general feeling that they wouldn't be needed (in the framework) when long running effects are possible.

@sveltema
Copy link
Author

sveltema commented Dec 4, 2023

Spent a little time converting the recent release. Converting the library itself has been fairly easy but the tests are using mockk which does not support Kotlin Native. There are multiplatform libraries ( MockKMP, mockative ) but tests will have to be rewritten (which I will start working on next).

An alternative would be to keep the existing tests, but they would only run from jvm targets, so not great for testing across all library supported platforms.

@heytherewill
Copy link
Contributor

IMO the best course of action is to drop mocks altogether and use Fakes instead, since everything crucial is already an interface that can be implemented easily

@semanticer
Copy link
Collaborator

I'd love to have a checklist of what we need to do to go fully KMP. I'm now trying to get more familiar with KMP but don't feel confident creating that checklist yet myself, so if anyone wants to step in, please feel free.

@sveltema
Copy link
Author

I have a fork of the current toggl/komposable-architecture:main branch that is a complete KMP version. When only considered against the main branch, the only thing left to tackle is publishing as I am unfamiliar with Toggl's publishing processes.

I have not created a PR for this as I have been watching the progress of the TestStore and either merging my fork or the TestStore changes will likely cause many conflicts which will need to be sorted out. My gut feeling is that the TestStore changes should probably be merged in first, and the KMP implementation updated again to resolve the many resulting conflicts.

@semanticer semanticer added this to the Release 1.0.0 milestone Jan 29, 2024
@daividssilverio
Copy link
Member

We need to break down this issue into smaller steps.

@semanticer semanticer self-assigned this Oct 17, 2024
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

No branches or pull requests

4 participants