https://kotlinlang.org logo
#orbit-mvi
Title
# orbit-mvi
g

Guilherme Delgado

06/22/2022, 1:25 PM
Hi guys! I know you must be busy and with other priorities at hand, but do you have any idea when will you have spare time to look into this PR? Thanks 🙂
❤️ 1
m

Mikolaj Leszczynski

06/22/2022, 1:28 PM
I’ll take a look Friday!
🙌 1
g

Guilherme Delgado

06/22/2022, 1:29 PM
I’m available if you need any help 👍
👍 1
a

appmattus

06/22/2022, 7:15 PM
Should be around too. Ping me when looking @Mikolaj Leszczynski
👍 1
m

Mikolaj Leszczynski

06/24/2022, 12:37 PM
Probably not the outcome you were looking for @Guilherme Delgado, but we have to make hard choices sometimes
We set out to create this project with a certain philosophy in mind and we have to stand by it.
g

Guilherme Delgado

06/24/2022, 12:38 PM
hey no problem 😊 it was well explained. it’s better that you keep your philosophy, at least you spend your time on it which i thank you for it
👍 1
m

Mikolaj Leszczynski

06/24/2022, 12:39 PM
Still, thank you for the contribution - it did push us to review the encapsulation and please do keep it up!
👍 1
g

Guilherme Delgado

06/24/2022, 12:40 PM
just one small comment:
it increases the API surface for little reason other than making it easier for a small subset of ppl to hide the container
could be a small subset of ppl because they probably don’t mind, but actually having the container “public” kind of scratches my OCD 😅 so yeah I think it’s better not to be able to do something like: viewmodel.container.intent … in the view
but then again:
the functions feel like they are better placed as part of something like androidx.compose.runtime or a separate library, especially given there is no orbit code in the new functions
completely agree 👍
talking about contributions, i made another one in the gradle-plugin, pretty small though, i which i had the knowledge to make it work with latest versions of kotlin 😞 😬
m

Mikolaj Leszczynski

06/24/2022, 12:42 PM
We discussed ways of hiding it and none felt attractive: • Base class is out of the question because we went for composition for a reason • Having a pair of private/public fields to hide away container particulars also feels boilerplatey and would require the
ContainerHost
to call
_container.intent
instead of simply
intent
g

Guilherme Delgado

06/24/2022, 12:44 PM
yes i don’t think the “hiding” logic is the lib responsibility we can achieve it very well in our end simply by:
Copy code
/**
 * Just to keep it private without public access.
 * @param initialState The initial state of the container.
 */
internal fun <STATE : Any, SIDE_EFFECT : Any> CoroutineScope.containerHostVisibilityWrapper(initialState: STATE) =
    object : ContainerHost<STATE, SIDE_EFFECT> {
        override val container = this@containerHostVisibilityWrapper.container<STATE, SIDE_EFFECT>(initialState)
    }
kmm world ^
Copy code
/**
 * Just to keep it private without public access.
 */
fun <STATE : Parcelable, SIDE_EFFECT : Any> ViewModel.containerHostVisibilityWrapper(
    initialState: STATE,
    savedStateHandle: SavedStateHandle,
    onCreate: ((state: STATE) -> Unit)? = null
) =
    object : ContainerHost<STATE, SIDE_EFFECT> {
        override val container =
            this@containerHostVisibilityWrapper.container<STATE, SIDE_EFFECT>(initialState, savedStateHandle = savedStateHandle, onCreate = onCreate)
    }
android world ^
and that brings us to the point that, when androidx gives us that “flow extensions”, this “issue” it’s solved 🙂
will land soon
👍 1
a

appmattus

06/24/2022, 12:55 PM
I thought I'd seen them somewhere!
7 Views