komposable-architecture icon indicating copy to clipboard operation
komposable-architecture copied to clipboard

Code generation proposal

Open heytherewill opened this issue 1 year ago • 6 comments

Proposal

Since the inception of this library I've been meaning to write a compiler plugin which could generate a lot of the boilerplate needed to overcome the lack of Keypaths in Kotlin. I have a Proof of concept plugin but I need to discuss the details to make sure I'm covering all details. Since I don't have access to the Toggl codebase anymore, I will need internal help to find edge cases

My idea is simple:

  • Introduce annotations that allow you to indicate State and action mappings
  • Introduce a KSP SymbolProcessor that will generate the mapping and pullback functions for you
  • We delete inline fun <From, reified To> From.unwrap(): To? = everywhere so I can finally have peace of mind

This is the first draft I have of the wishful thinking outcome of this proposal. I'd love to hear feedback on naming, but also on possible ways of improving the proposal, edge cases I didn't consider and boilerplate we could automate that I'm not touching here yet.

/*
    Newly added annotations
 */
@Target(AnnotationTarget.CLASS)
@Retention(AnnotationRetention.SOURCE)
annotation class WrapsAction

@Target(AnnotationTarget.CLASS)
@Retention(AnnotationRetention.SOURCE)
annotation class ChildStates(vararg val childStates: KClass<*>)

/*
    Code the users write
 */
data class SettingsState(
    val booleanSetting: Boolean = true
)

sealed interface SettingsAction {
    data class ChangeSomeSetting(val newValue: Boolean) : SettingsAction
}

// We need to annotate the parent because the
// child might not have access to the parent due to modularization.
@ChildStates(SettingsState::class)
data class AppState(
    val listOfItems: List<String> = emptyList(),
    val booleanSetting: Boolean = true,
)

sealed interface AppAction {
    object ClearList : AppAction

    @WrapsAction
    data class Settings(val settingsAction: SettingsAction) : AppAction
}

class SettingsReducer : Reducer<SettingsState, SettingsAction> {
    override fun reduce(state: Mutable<SettingsState>, action: SettingsAction): List<Effect<SettingsAction>> =
        when (action) {
            is SettingsAction.ChangeSomeSetting -> state.mutateWithoutEffects {
                copy(booleanSetting = action.newValue)
            }
        }
}

fun mainReducer(settingsReducer: SettingsReducer): Reducer<AppState, AppAction> =
    settingsReducer.pullback()

/*
    Auto-generated code
 */

// This we can infer by checking that `SettingsReducer` implements `Reducer<SettingsState, SettingsAction>`
// and also by checking that both `SettingsState` and `SettingsAction` have generated action mappings.
fun Reducer<SettingsState, SettingsAction>.pullback() = pullback(
    ::mapAppStateToSettingsState,
    ::mapAppActionToSettingsAction,
    ::mapSettingsStateToAppState,
    ::mapSettingsActionToAppAction,
)

// This function is super simple to generate. We will throw a compiler error if
// the wrapper action has more than one property (meaning it's not truly wrapping a child action).
fun mapSettingsActionToAppAction(settingsAction: SettingsAction): AppAction =
    AppAction.Settings(settingsAction)

// This one we can generate easily if we are doing name matching BUT we need to allow overrides.
// We can add something like `ParentPropertyPath` so one can annotate the child state. While we
// can output compiler errors to ensure refactors don't break mapping, you are still just
// using a string to tell where to find the parent property.
fun mapSettingsStateToAppState(appState: AppState, settingsState: SettingsState): AppState =
    appState.copy(
        booleanSetting = settingsState.booleanSetting
    )

// Same as the other action mapping function.
fun mapAppActionToSettingsAction(appAction: AppAction): SettingsAction? =
    if (appAction is AppAction.Settings) appAction.settingsAction else null

// Same as the other state mapping function.
fun mapAppStateToSettingsState(appState: AppState): SettingsState =
    SettingsState(
        booleanSetting = appState.booleanSetting
    )

Tasks

  • [X] Introduce new komposable-architecture-compiler artifact
  • [X] Create a SymbolProcessor for generating Action boilerplate
  • [ ] Create a SymbolProcessor for generating State boilerplate
  • [ ] Create a SymbolProcessor for generating Pullback boilerplate

heytherewill avatar Oct 26 '23 05:10 heytherewill

What are we gonna do about optionalPullback? It would probably need some extra condition parameter like:

fun mainReducer(settingsReducer: SettingsReducer): Reducer<AppState, AppAction> =
    settingsReducer.optionalPullback { appState -> listOfItems.isNotEmpty() }

🤔

semanticer avatar Oct 26 '23 17:10 semanticer

I don't hate the naming, I'll just leave some possible alternatives here for consideration:

WrapsAction

  • PullbackStates
  • ComposedAction
  • WrapperAction

ChildStates

  • PullbackStates
  • SubStates
  • NestedStates
  • ParentOf

semanticer avatar Oct 26 '23 17:10 semanticer

It'd be super cool to have an option to replace some of the generated functions with your own implementation if needed.

Would something like this be possible?

fun Reducer<SettingsState, SettingsAction>.pullback(
    mapToLocalState: (AppState) -> SettingsState = ::mapAppStateToSettingsState,
    mapToLocalAction: (AppAction) -> SettingsAction? = ::mapAppActionToSettingsAction,
    mapToGlobalState: (AppState, SettingsState) -> AppState = ::mapSettingsStateToAppState,
    mapToGlobalAction: (SettingsAction) -> AppAction = ::mapSettingsActionToAppAction,
) = pullback(
    mapToLocalState,
    mapToLocalAction,
    mapToGlobalState,
    mapToGlobalAction,
)

semanticer avatar Oct 26 '23 17:10 semanticer

This is probably an edge case so I don't think we need to address this now but what if SettingsReducer is pulled in more than one context. We'd have to generate multiple pullback methods with different names 🤔

semanticer avatar Oct 26 '23 17:10 semanticer

The biggest obstacle in Toggl App would be the fact that we have a ton of custom mappings where we often map types to different types like her for example:

fun mapAppStateToCalendarState(appState: AppState): CalendarState =
    CalendarState(
        user = appState.user,
        workspaces = appState.workspaces.getOrEmptyMap(),
        organizations = appState.organizations.getOrEmptyMap(),
        timeEntries = appState.timeEntries.getOrEmptyMap(),
        ....
    )

But I guess that's our problem :-D

semanticer avatar Oct 26 '23 17:10 semanticer

What are we gonna do about optionalPullback? It would probably need some extra condition parameter like:

Good point. I think mapping should JustWork™ and be able to figure out optional state based on nullability, but if you want something like having optional state based on a property then yeah, we need to add an extra parameter. I'll cook something

I don't hate the naming, I'll just leave some possible alternatives here for consideration:

I started with @WrapsAction because my idea was to pass the type of the action being wrapped. This is not needed since we'll enforce the type to only have a single property, which means WrapperAction is a much better name. I like ParentOf and ChildStates, both could work.

We'd have to generate multiple pullback methods with different names

That was my first idea, but I opted for making it simpler because I didn't see the need for disambiguation. Doesn't hurt to be explicit 🤷

we have a ton of custom mappings where we often map types to different types

I think the solution of having overridable functions in the generated pullback method could be a way of solving that. We can go as far as making the whole thing augmentable rather than replaceable, e.g.:

interface ReducerMapperInterfaceThatSeriouslyNeedABetterName<ParentState, ParentAction, ChildState, ChildAction> {
    fun mapToParentAction(childAction: ChildAction): ParentAction
    fun mapToParentState(parentState: ParentState, childState: ChildState): ParentState
    fun mapToChildAction(parentaction: ParentAction): ChildAction?
    fun mapToChildState(parentState: ParentState): ChildState
}

The compiler plugin could generate an implementation of this interface with virtual methods that you could override if you want to provide specific behavior without losing the generated bits. I like the solution of just using functions though, so please advise if you think this is needed.

heytherewill avatar Oct 27 '23 19:10 heytherewill