Добавление событий жизненного цикла для AndroidMVi #24

Maksim2355 · 2022-04-20T18:51:24Z

Как ты помог развить студийку?

Предложил возможность добавить событий ЖЦ для андроида
(компоуз опустим)

UPDATE вынес из MviView код, связанный с привязкой lifecycleEvent. Мне кажется, таким образом можно сделать код более гибким

Есть подводные камни?

Сейчас, для связывание нужно сделать 4 действия

В вашем эвенте создать LifecycleEvent
Реализовать нужный интерфейс для viewModel для маппинга его
Заимплементить LifecycleMiddleware
Вызвать в вашей view bindLifecycleEvent

Я глубоко уверен, что последние 2 проблемы отойдут в базовые классы на проде, но вот с первыми двумя сложнее. Тяжело так просто взять и скормить событие, которые не связано с событием экрана(из-за сильной типизации и к вопросу о ~~лидерах протеста~~ этого ишью #9)

Т.е для компоуза ты ничего не сделал? Не стыдно?

Нет. Я пока глубоко убежден, что подход для него будет отличаться. Думаю если мы планируем сделать такую штуку для компоуза, то пр будет сильно больше(или как минимум его стоит смотреть отдельно)

Мы же уходим от стандартного андроида

Да, но если спросите мое мнение(вдруг🥺👉🏻👈🏻) то я скажу, что выглядит будто для компоуза нужно еще достаточно времени, чтобы доработать mvi-flow + написать достаточно кода и определить проблемы архитектуры(и по возможности устранить) + Еще замотивировать наших разрабах для более плотного изучения компоуза

А вот для варианта без компоуза выглядит так, что еще немного доработать и в прод пускать. Подход почти не изменился, но зато новые проекты могут стартануть без рисков и к ребятам будут более низкие требовании, на случай если они еще плохо знакомы с технологией. Таким образом, переход будет постепенный. Оставим привычных подход, но улучшим его и сделаем более современным(Плюс если будем использовать ComposeView вместо виджетов, то таким образом и композировать будем постепенно)

Уже второй пр делаешь без прямых указаний и отвлекаешь от основной работы на писанину многочисленных комментариев. Записывай подобные вещи в ишью и потом на тебя или на кого-то другого назначат

не удержался 👈🏻😨👉🏻

А если наша либа станет очень популярной и кто-то долистает до первых ПРов и увидит как ты их оформляешь?

В будущем более взросло буду оформлять, хотя я итак удерживаюсь и мемы не кидаю в скриншотах...

Есть еще предложения?

Скриншоты(если есть)

…cleScope

…android' into add_lifecycle_event_for_default_android

margarita-v

спасибо за такое активное участие в развитии либы, очень полезно 🔥

mvi-core/src/main/kotlin/ru/surfstudio/mvi/core/event/LifecycleMviEvent.kt

mvi-core/src/main/kotlin/ru/surfstudio/mvi/lifecycle/LifecycleBinds.kt

mvi-core/src/main/kotlin/ru/surfstudio/mvi/flow/LifecycleMiddleware.kt

margarita-v · 2022-04-22T17:48:45Z

mvi-core/src/main/kotlin/ru/surfstudio/mvi/lifecycle/LifecycleBinds.kt

+/**
+ * Sends lifecycle events to the shared event bus
+ */
+fun <E : Event> MviView<E>.bindsLifecycleEvent() {


можно в MviView

Тут, насколько это возможно для меня ¯_(ツ)_/¯ , осознаное решение. Хотелось бы сделать код наименее связным. Так, я старался следовать принципу единой ответственности. MviView создает правила, которые должны соблюдать наша вьюха, чтобы реализовать MVI-архитектуру. А вот поддержку обработки событий жизненного цикла я хотел вынести в отдельное расширение. Так наш код будет мене связным и мы можем при необходимости изменить/доработать реализацию

Но если ты считаешь, что лучше в MviView вынести, то я, конечно, так и сделаю

мне кажется, в этом случае можно сразу в MviView, тк она уже является LifecycleOwner, то есть мы не особо разделили ответственность в итоге, а просто вместо расширения сразу будет функция интерфейса, и кому нужно будут ее использовать (если бы действительно расширяли тем, чего не было в MviView, тогда оправдано было бы)

margarita-v · 2022-04-22T17:50:00Z

mvi-core/src/main/kotlin/ru/surfstudio/mvi/lifecycle/MapperLifecycleEvent.kt

+import ru.surfstudio.mvi.vm.android.MviView
+
+/**
+ * Map [Lifecycle.Event] from lifecycleScope to [LifecycleMviEvent]


судя по реализации, в E, надо добавить where для более точного определения события

Немного не понял 😕

комент про LifecycleMviEvent, но по факту <E : Event>, потому предлагаю ограничить E с помощью where для соответствия описанию

margarita-v · 2022-04-22T17:51:44Z

sample/src/main/kotlin/ru/surfstudio/mvi/flow/app/simple/SimpleMiddleware.kt

 import java.io.IOException

 class SimpleMiddleware(
    private val state: FlowState<SimpleState>
-) : DslFlowMiddleware<SimpleEvent> {
+) : DslFlowMiddleware<SimpleEvent>, LifecycleMiddleware<SimpleEvent> {


Suggested change

) : DslFlowMiddleware<SimpleEvent>, LifecycleMiddleware<SimpleEvent> {

) : LifecycleMiddleware<SimpleEvent> {

Написал BaseFlowMiddleware. По сути обычный middleware c полной поддержкой LifecycleEvent

Почему я не реализовал MapperLifecycleEvent в LifecycleMiddleware?

Хотелось бы сделать код менее связным. Так, при необходимости мы можем изменить подход не меняя основные сущности т.к они не оч связаны

mvi-core/src/main/kotlin/ru/surfstudio/mvi/lifecycle/MapperLifecycleEvent.kt

mvi-core/src/main/kotlin/ru/surfstudio/mvi/flow/LifecycleMiddleware.kt

margarita-v · 2022-04-22T18:00:55Z

mvi-core/src/main/kotlin/ru/surfstudio/mvi/lifecycle/MapperLifecycleEvent.kt

+/**
+ * Map [Lifecycle.Event] from lifecycleScope to [LifecycleMviEvent]
+ */
+interface MapperLifecycleEvent<E : Event> {


все круто придумано! предлагаю перенести ответственность в middleware, тк у него весь мапинг. LifecycleMiddleware наследует MapperLifecycleEvent, затем поправить доку LifecycleMiddleware и bindsLifecycleEvent (принцип останется тот же, просто теперь viewModel.middleware кастуем к MapperLifecycleEvent)

Мне только не нравится, что мы пишем каждый раз lifecycle. мне такое не нравилось для события ЖЦ и навигации в прошлом подходе. ~~Я тогда постараюсь поколдовать с хабом и если будут успехи, то сообщу~~ идея не удалась(

…android' into add_lifecycle_event_for_default_android

mvi-core/src/main/kotlin/ru/surfstudio/mvi/flow/BaseFlowMiddleware.kt

margarita-v · 2022-04-25T19:34:54Z

mvi-core/src/main/kotlin/ru/surfstudio/mvi/lifecycle/LifecycleBinds.kt

+/**
+ * Sends lifecycle events to the shared event bus
+ */
+fun <E : Event> MviView<E>.bindsLifecycleEvent() {


мне кажется, в этом случае можно сразу в MviView, тк она уже является LifecycleOwner, то есть мы не особо разделили ответственность в итоге, а просто вместо расширения сразу будет функция интерфейса, и кому нужно будут ее использовать (если бы действительно расширяли тем, чего не было в MviView, тогда оправдано было бы)

margarita-v · 2022-04-25T19:37:17Z

mvi-core/src/main/kotlin/ru/surfstudio/mvi/lifecycle/MapperLifecycleEvent.kt

+/**
+ * Map [Lifecycle.Event] from lifecycleScope to [MviLifecycleEvent]
+ */
+interface MapperLifecycleEvent<E> {


Suggested change

interface MapperLifecycleEvent<E> {

interface MapperLifecycleEvent<E> where E: Event, E: MviLifecycleEvent<E> {

это в идеале, чтобы описание соответствовало содержанию, а так как минимум E: Event

margarita-v · 2022-04-25T19:39:21Z

mvi-core/src/main/kotlin/ru/surfstudio/mvi/flow/LifecycleMiddleware.kt

+ * Middleware, that reacts on android lifecycle.
+ *
+ * To receive events, you need to add event that implements [MviLifecycleEvent]
+ * and for your viewModel implements [MapperLifecycleEvent]


с появлением BaseFlowMiddleware дока не актуальна

mvi-core/src/main/kotlin/ru/surfstudio/mvi/core/event/MviLifecycleEvent.kt

mvi-core/src/main/kotlin/ru/surfstudio/mvi/flow/LifecycleMiddleware.kt

…ware.kt Co-authored-by: Margarita Volodina <[email protected]>

…eware.kt Co-authored-by: Margarita Volodina <[email protected]>

…ycleEvent.kt Co-authored-by: Margarita Volodina <[email protected]>

margarita-v · 2022-04-26T08:44:09Z

mvi-core/src/main/kotlin/ru/surfstudio/mvi/core/event/MviLifecycleEvent.kt

+import androidx.lifecycle.Lifecycle
+
+/** Screen lifecycle event */
+interface MviLifecycleEvent<E: Event> {


типизация не нужна?

мб вернуть наследование от Event?

margarita-v · 2022-04-26T08:49:24Z

mvi-core/src/main/kotlin/ru/surfstudio/mvi/flow/BaseFlowMiddleware.kt

+ * Base middleware with support of lifecycle events
+ *
+ * To receive events, you need to add event that implements [MviLifecycleEvent]
+ * and to call the [bindLifecycleEventfecycleEvent] for the entity implementing the MviView interface


To receive events, you need to add event which implements [MviLifecycleEvent]
and to call [bindsLifecycleEvent] for [MviView] implementation

Maksim2355 added 2 commits April 20, 2022 20:44

add_lifecycler_event_for_default_android

976d4f0

add_lifecycler_event_for_default_an добавил логику для событий lifecy…

410de3a

…cleScope

Maksim2355 requested a review from margarita-v April 20, 2022 18:51

Maksim2355 self-assigned this Apr 20, 2022

Maksim2355 and others added 9 commits April 20, 2022 18:52

Code formatting [skip ci] [version]

8539cce

add_lifecycler_event_for_default_anправки пра

8084616

Merge remote-tracking branch 'origin/add_lifecycle_event_for_default_…

988426e

…android' into add_lifecycle_event_for_default_android

add_lifecycler_event_for_default_anправки пра

7b082d0

add_lifecycler_event_for_default_anправки пра

18da450

add_lifecycler_event_for_default_anправки пра

3eff065

Code formatting [skip ci] [version]

87eaa2e

add_lifecycler_event_for_default_android update filterLifecycleEvent

bc7aeb0

Merge remote-tracking branch 'origin/add_lifecycle_event_for_default_…

1371a9b

…android' into add_lifecycle_event_for_default_android

margarita-v reviewed Apr 22, 2022

View reviewed changes

Maksim2355 and others added 5 commits April 23, 2022 19:57

add_lifecycler_event_for_default_android edit Pr

e109b91

add_lifecycler_event_for_default_android edit Pr

0bae567

Code formatting [skip ci] [version]

b227b16

add_lifecycler_event_for_default_android edit Pr

6b9f616

Merge remote-tracking branch 'origin/add_lifecycle_event_for_default_…

967c63f

…android' into add_lifecycle_event_for_default_android

margarita-v reviewed Apr 25, 2022

View reviewed changes

mvi-core/src/main/kotlin/ru/surfstudio/mvi/flow/LifecycleMiddleware.kt Outdated Show resolved Hide resolved

Maksim2355 and others added 4 commits April 25, 2022 22:53

Update mvi-core/src/main/kotlin/ru/surfstudio/mvi/flow/BaseFlowMiddle…

1c78995

…ware.kt Co-authored-by: Margarita Volodina <[email protected]>

Update mvi-core/src/main/kotlin/ru/surfstudio/mvi/flow/LifecycleMiddl…

eee9659

…eware.kt Co-authored-by: Margarita Volodina <[email protected]>

Update mvi-core/src/main/kotlin/ru/surfstudio/mvi/core/event/MviLifec…

3cfb79a

…ycleEvent.kt Co-authored-by: Margarita Volodina <[email protected]>

add_lifecycler_event_for_default_android edit Pr

32f8f76

margarita-v reviewed Apr 26, 2022

View reviewed changes

add_lifecycler_event_for_default_android edit Pr

286f787

margarita-v approved these changes Apr 26, 2022

View reviewed changes

margarita-v merged commit 48c9d1d into master Apr 26, 2022

margarita-v deleted the add_lifecycle_event_for_default_android branch April 26, 2022 18:58

margarita-v mentioned this pull request Jun 24, 2022

Lifecycle-aware middleware for composable #31

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Добавление событий жизненного цикла для AndroidMVi #24

Добавление событий жизненного цикла для AndroidMVi #24

Maksim2355 commented Apr 20, 2022 •

edited

Loading

margarita-v left a comment

margarita-v Apr 22, 2022

Maksim2355 Apr 23, 2022 •

edited

Loading

Maksim2355 Apr 23, 2022

margarita-v Apr 25, 2022

margarita-v Apr 22, 2022

Maksim2355 Apr 23, 2022

margarita-v Apr 23, 2022 •

edited

Loading

margarita-v Apr 22, 2022

Maksim2355 Apr 23, 2022

margarita-v Apr 22, 2022

Maksim2355 Apr 23, 2022 •

edited

Loading

margarita-v Apr 25, 2022

margarita-v Apr 25, 2022

margarita-v Apr 25, 2022

margarita-v Apr 25, 2022

margarita-v Apr 26, 2022

margarita-v Apr 26, 2022

margarita-v Apr 26, 2022

	) : DslFlowMiddleware<SimpleEvent>, LifecycleMiddleware<SimpleEvent> {
	) : LifecycleMiddleware<SimpleEvent> {

	interface MapperLifecycleEvent<E> {
	interface MapperLifecycleEvent<E> where E: Event, E: MviLifecycleEvent<E> {

Добавление событий жизненного цикла для AndroidMVi #24

Добавление событий жизненного цикла для AndroidMVi #24

Conversation

Maksim2355 commented Apr 20, 2022 • edited Loading

Как ты помог развить студийку?

Есть подводные камни?

Т.е для компоуза ты ничего не сделал? Не стыдно?

Мы же уходим от стандартного андроида

А если наша либа станет очень популярной и кто-то долистает до первых ПРов и увидит как ты их оформляешь?

Есть еще предложения?

Скриншоты(если есть)

margarita-v left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Maksim2355 Apr 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

margarita-v Apr 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Maksim2355 Apr 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Maksim2355 commented Apr 20, 2022 •

edited

Loading

Maksim2355 Apr 23, 2022 •

edited

Loading

margarita-v Apr 23, 2022 •

edited

Loading

Maksim2355 Apr 23, 2022 •

edited

Loading