toggl / komposable-architecture

🏗️ Kotlin implementation of Point-Free's composable architecture
Apache License 2.0
280 stars 20 forks source link

Code generation proposal #60

Closed heytherewill closed 1 week ago

heytherewill commented 1 year ago

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:

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

semanticer commented 1 year ago

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 commented 1 year ago

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

WrapsAction

ChildStates

semanticer commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

heytherewill commented 1 year ago

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.

semanticer commented 1 week ago

The last bit, "Create a SymbolProcessor for generating Pullback boilerplate," will probably be harder than we thought.

I'm talking about this part:

// 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,
)

There are a couple of things making this hard. The first one is that getting generic types is not so easy. Also, reducers can have m2m relationships, meaning a "parent" reducer can have multiple "child" reducers. However, a "child" reducer might be "pullback-able" to different parent reducers, which makes the generation logic much harder to write. We might need some additional annotations.

semanticer commented 1 week ago

I'm closing this issue, and creating a new one https://github.com/toggl/komposable-architecture/issues/84 just for the case mentioned above ☝