Skip to content

Commit

Permalink
Introduce compose.ui dependency and add Modifier to Ui.Content() (#422)
Browse files Browse the repository at this point in the history
This is a breaking API change to `Ui.Content()` to add a `Modifier`
parameter. There are breaking changes, but if using code gen it's fairly
minimal.

```diff
public interface Ui<UiState : CircuitUiState> {
-  @composable public fun Content(state: UiState)
+  @composable public fun Content(state: UiState, modifier: Modifier)
}
```

We've been getting stuck on this for awhile now when trying to bridge to
UI land and finally pulling the trigger on this to ease a bunch of
things.

At a high level, this replaces the toe-hold `ScreenUi` we've been
keeping around as modifiers give us a means of plumbing down extra UI
metadata, so I've removed it from the API.

We can also offer a default onUnavailableContent API now, so I've made
it non-nullable in `CircuitConfig` using that default. If people want
the previous erroring behavior, they can still achieve this with a
custom function of their own.

One place this hits friction is UI libraries that don't use Modifiers
(i.e. Mosaic), but it's easy enough to just ignore that parameter coming
from `ui { state, _ -> ... }`.

Other things
- Realized our code gen artifact was sorta halfway multiplatform and
resulted in confusing KSP configuration errors. Cleaned that up and made
it just a standard JVM artifact.
- Reworked how we generate UI factories a bit in code gen to make it
simpler to maintain. Put a thorough diagram in for good measure.
- Removed redundant wrapper `Box`es that were shims for this
functionality missing.
- It's not possible in abstract composable functions to add a default
value. i.e. can't write `modifier: Modifier = Modifier` in the
`Ui.Content(...)` signature.

This does _add_ the compose UI dependency to circuit core, but after a
lot of thought I think this is ok. Our android artifact already imposed
it for all intents and purposes, this just drops that facade and
embraces the fact that this is a UI architecture and not just a business
logic layer using compose runtime only.

Supersedes #406.
Helps #50 too.
Eases state sharing via modifiers. It would be easy to build modifiers
that build upon each other with this and we no longer have a missing UI
link.
  • Loading branch information
ZacSweers authored Feb 2, 2023
1 parent b0aeda1 commit d5a2190
Show file tree
Hide file tree
Showing 31 changed files with 304 additions and 212 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ com.squareup.anvil:annotations
javax.inject:javax.inject
org.jetbrains.compose.runtime:runtime-saveable
org.jetbrains.compose.runtime:runtime
org.jetbrains.compose.ui:ui
org.jetbrains.kotlin:kotlin-bom
org.jetbrains.kotlin:kotlin-stdlib-jdk7
org.jetbrains.kotlin:kotlin-stdlib-jdk8
Expand Down
22 changes: 22 additions & 0 deletions circuit-codegen-annotations/dependencies/jvmRuntimeClasspath.txt
Original file line number Diff line number Diff line change
@@ -1,10 +1,30 @@
com.google.dagger:dagger
com.squareup.anvil:annotations
javax.inject:javax.inject
org.jetbrains.compose.animation:animation-core-desktop
org.jetbrains.compose.animation:animation-core
org.jetbrains.compose.animation:animation-desktop
org.jetbrains.compose.animation:animation
org.jetbrains.compose.foundation:foundation-desktop
org.jetbrains.compose.foundation:foundation-layout-desktop
org.jetbrains.compose.foundation:foundation-layout
org.jetbrains.compose.foundation:foundation
org.jetbrains.compose.runtime:runtime-desktop
org.jetbrains.compose.runtime:runtime-saveable-desktop
org.jetbrains.compose.runtime:runtime-saveable
org.jetbrains.compose.runtime:runtime
org.jetbrains.compose.ui:ui-desktop
org.jetbrains.compose.ui:ui-geometry-desktop
org.jetbrains.compose.ui:ui-geometry
org.jetbrains.compose.ui:ui-graphics-desktop
org.jetbrains.compose.ui:ui-graphics
org.jetbrains.compose.ui:ui-text-desktop
org.jetbrains.compose.ui:ui-text
org.jetbrains.compose.ui:ui-unit-desktop
org.jetbrains.compose.ui:ui-unit
org.jetbrains.compose.ui:ui-util-desktop
org.jetbrains.compose.ui:ui-util
org.jetbrains.compose.ui:ui
org.jetbrains.kotlin:kotlin-bom
org.jetbrains.kotlin:kotlin-stdlib-jdk7
org.jetbrains.kotlin:kotlin-stdlib-jdk8
Expand All @@ -14,4 +34,6 @@ org.jetbrains.kotlinx:atomicfu
org.jetbrains.kotlinx:kotlinx-coroutines-bom
org.jetbrains.kotlinx:kotlinx-coroutines-core-jvm
org.jetbrains.kotlinx:kotlinx-coroutines-core
org.jetbrains.skiko:skiko-awt
org.jetbrains.skiko:skiko
org.jetbrains:annotations
2 changes: 0 additions & 2 deletions circuit-codegen/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,5 @@ dependencies {
implementation(libs.dagger)
implementation(libs.kotlinpoet)
implementation(libs.kotlinpoet.ksp)
implementation(projects.circuit)
implementation(projects.circuitCodegenAnnotations)
implementation(libs.anvil.annotations)
}
9 changes: 0 additions & 9 deletions circuit-codegen/dependencies/runtimeClasspath.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,10 @@ com.squareup.anvil:annotations
com.squareup:kotlinpoet-ksp
com.squareup:kotlinpoet
javax.inject:javax.inject
org.jetbrains.compose.runtime:runtime-desktop
org.jetbrains.compose.runtime:runtime-saveable-desktop
org.jetbrains.compose.runtime:runtime-saveable
org.jetbrains.compose.runtime:runtime
org.jetbrains.kotlin:kotlin-bom
org.jetbrains.kotlin:kotlin-reflect
org.jetbrains.kotlin:kotlin-stdlib-common
org.jetbrains.kotlin:kotlin-stdlib-jdk7
org.jetbrains.kotlin:kotlin-stdlib-jdk8
org.jetbrains.kotlin:kotlin-stdlib
org.jetbrains.kotlinx:atomicfu-jvm
org.jetbrains.kotlinx:atomicfu
org.jetbrains.kotlinx:kotlinx-coroutines-bom
org.jetbrains.kotlinx:kotlinx-coroutines-core-jvm
org.jetbrains.kotlinx:kotlinx-coroutines-core
org.jetbrains:annotations
1 change: 1 addition & 0 deletions circuit-codegen/gradle.properties
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
POM_ARTIFACT_ID=circuit-codegen
POM_NAME=Circuit (Codegen)
POM_DESCRIPTION=Circuit (Codegen)
circuit.noCompose=true
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,9 @@ import com.google.devtools.ksp.symbol.KSDeclaration
import com.google.devtools.ksp.symbol.KSFunctionDeclaration
import com.google.devtools.ksp.symbol.KSType
import com.google.devtools.ksp.symbol.Visibility
import com.slack.circuit.CircuitContext
import com.slack.circuit.CircuitUiState
import com.slack.circuit.Navigator
import com.slack.circuit.Presenter
import com.slack.circuit.Screen
import com.slack.circuit.ScreenUi
import com.slack.circuit.Ui
import com.slack.circuit.codegen.annotations.CircuitInject
import com.squareup.anvil.annotations.ContributesMultibinding
import com.squareup.kotlinpoet.AnnotationSpec
import com.squareup.kotlinpoet.ClassName
import com.squareup.kotlinpoet.CodeBlock
import com.squareup.kotlinpoet.FileSpec
import com.squareup.kotlinpoet.FunSpec
Expand All @@ -45,7 +38,6 @@ import com.squareup.kotlinpoet.STAR
import com.squareup.kotlinpoet.TypeName
import com.squareup.kotlinpoet.TypeSpec
import com.squareup.kotlinpoet.asClassName
import com.squareup.kotlinpoet.asTypeName
import com.squareup.kotlinpoet.joinToCode
import com.squareup.kotlinpoet.ksp.addOriginatingKSFile
import com.squareup.kotlinpoet.ksp.toClassName
Expand All @@ -55,9 +47,18 @@ import dagger.assisted.AssistedFactory
import javax.inject.Inject
import javax.inject.Provider

private val CIRCUIT_INJECT_ANNOTATION = CircuitInject::class.java.canonicalName
private val CIRCUIT_PRESENTER = Presenter::class.java.canonicalName
private val CIRCUIT_UI = Ui::class.java.canonicalName
private const val CIRCUIT_BASE_PACKAGE = "com.slack.circuit"
private val MODIFIER = ClassName("androidx.compose.ui", "Modifier")
private val CIRCUIT_INJECT_ANNOTATION =
ClassName("$CIRCUIT_BASE_PACKAGE.codegen.annotations", "CircuitInject")
private val CIRCUIT_PRESENTER = ClassName(CIRCUIT_BASE_PACKAGE, "Presenter")
private val CIRCUIT_PRESENTER_FACTORY = CIRCUIT_PRESENTER.nestedClass("Factory")
private val CIRCUIT_UI = ClassName(CIRCUIT_BASE_PACKAGE, "Ui")
private val CIRCUIT_UI_FACTORY = CIRCUIT_UI.nestedClass("Factory")
private val CIRCUIT_UI_STATE = ClassName(CIRCUIT_BASE_PACKAGE, "CircuitUiState")
private val SCREEN = ClassName(CIRCUIT_BASE_PACKAGE, "Screen")
private val NAVIGATOR = ClassName(CIRCUIT_BASE_PACKAGE, "Navigator")
private val CIRCUIT_CONTEXT = ClassName(CIRCUIT_BASE_PACKAGE, "CircuitContext")
private const val FACTORY = "Factory"

@AutoService(SymbolProcessorProvider::class)
Expand All @@ -68,9 +69,11 @@ public class CircuitSymbolProcessorProvider : SymbolProcessorProvider {
}

private class CircuitSymbols private constructor(resolver: Resolver) {
val circuitUiState = resolver.loadKSType<CircuitUiState>()
val screen = resolver.loadKSType<Screen>()
val navigator = resolver.loadKSType<Navigator>()
val modifier = resolver.loadKSType(MODIFIER.canonicalName)
val circuitUiState = resolver.loadKSType(CIRCUIT_UI_STATE.canonicalName)
val screen = resolver.loadKSType(SCREEN.canonicalName)
val navigator = resolver.loadKSType(NAVIGATOR.canonicalName)

companion object {
fun create(resolver: Resolver): CircuitSymbols? {
@Suppress("SwallowedException")
Expand All @@ -83,24 +86,23 @@ private class CircuitSymbols private constructor(resolver: Resolver) {
}
}

private inline fun <reified T> Resolver.loadKSType(): KSType {
return loadOptionalKSType<T>()
?: error("Could not find ${T::class.java.canonicalName} in classpath")
}
private fun Resolver.loadKSType(name: String): KSType =
loadOptionalKSType(name) ?: error("Could not find $name in classpath")

private inline fun <reified T> Resolver.loadOptionalKSType(): KSType? {
return getClassDeclarationByName(getKSNameFromString(T::class.java.canonicalName))
?.asType(emptyList())
private fun Resolver.loadOptionalKSType(name: String?): KSType? {
if (name == null) return null
return getClassDeclarationByName(getKSNameFromString(name))?.asType(emptyList())
}

private class CircuitSymbolProcessor(
private val logger: KSPLogger,
private val codeGenerator: CodeGenerator
private val codeGenerator: CodeGenerator,
) : SymbolProcessor {

override fun process(resolver: Resolver): List<KSAnnotated> {
val symbols = CircuitSymbols.create(resolver) ?: return emptyList()
resolver.getSymbolsWithAnnotation(CIRCUIT_INJECT_ANNOTATION).forEach { annotatedElement ->
resolver.getSymbolsWithAnnotation(CIRCUIT_INJECT_ANNOTATION.canonicalName).forEach {
annotatedElement ->
when (annotatedElement) {
is KSClassDeclaration -> {
generateFactory(annotatedElement, InstantiationType.CLASS, symbols)
Expand All @@ -121,12 +123,12 @@ private class CircuitSymbolProcessor(
private fun generateFactory(
annotatedElement: KSAnnotated,
instantiationType: InstantiationType,
symbols: CircuitSymbols
symbols: CircuitSymbols,
) {
val circuitInjectAnnotation =
annotatedElement.annotations.first {
it.annotationType.resolve().declaration.qualifiedName?.asString() ==
CIRCUIT_INJECT_ANNOTATION
CIRCUIT_INJECT_ANNOTATION.canonicalName
}
val screenKSType = circuitInjectAnnotation.arguments[0].value as KSType
val screenIsObject =
Expand Down Expand Up @@ -185,12 +187,12 @@ private class CircuitSymbolProcessor(
val packageName: String,
val factoryType: FactoryType,
val constructorParams: List<ParameterSpec>,
val codeBlock: CodeBlock
val codeBlock: CodeBlock,
)

/** Computes the data needed to generate a factory. */
// Detekt and ktfmt don't agree on whether or not the rectangle rule makes for readable code.
@Suppress("ComplexMethod", "LongMethod")
@Suppress("ComplexMethod", "LongMethod", "ReturnCount")
@OptIn(KspExperimental::class)
private fun computeFactoryData(
annotatedElement: KSAnnotated,
Expand Down Expand Up @@ -232,34 +234,72 @@ private class CircuitSymbolProcessor(
assistedParams
)
FactoryType.UI -> {
// State param is optional
val stateParam =
fd.parameters.singleOrNull { parameter ->
symbols.circuitUiState.isAssignableFrom(parameter.type.resolve())
}
if (stateParam == null) {
CodeBlock.of(
"%M<%T>·{·%M(%L)·}",
MemberName("com.slack.circuit", "ui"),
CircuitUiState::class.java,
MemberName(packageName, name),
assistedParams
)
} else {
val block =
if (assistedParams.isEmpty()) {
CodeBlock.of("")
} else {
CodeBlock.of(",·%L", assistedParams)

// Modifier param is required
val modifierParam =
fd.parameters.singleOrNull { parameter ->
symbols.modifier.isAssignableFrom(parameter.type.resolve())
}
?: run {
logger.error("UI composable functions must have a Modifier parameter!", fd)
return null
}
CodeBlock.of(
"%M<%T>·{·state·->·%M(%L·=·state%L)·}",
MemberName("com.slack.circuit", "ui"),
stateParam.type.resolve().toTypeName(),
MemberName(packageName, name),
stateParam.name!!.getShortName(),
block
)
}

/*
Diagram of what goes into generating a function!
- State parameter is _optional_ and can be omitted if it's static state.
- When omitted, the argument becomes _ and the param is omitted entirely.
- <StateType> is either the State or CircuitUiState if no state param is used.
- Modifier parameter is required.
- Assisted parameters can be 0 or more extra supported assisted parameters.
Optional state param
Optional state arg │
│ │ Required modifier param
│ Req modifier arg │ │
┌─── ui function │ │ │ │ Any assisted params
│ │ │ Composable │ │ │
│ State type │ │ │ │ │ │
│ │ │ │ │ │ │ │
│ │ │ │ │ │ │ │
└──────┴───── ──┴── ───┴──── ──┴───── ───────┴───── ────────┴────────── ────────┴────────
ui<StateType> { state, modifier -> Function(state = state, modifier = modifier, <assisted params>) }
────────────────────────────────────────────────────────────────────────────────────────────────────
Diagram generated with asciiflow. You can make new ones or edit with this link.
https://asciiflow.com/#/share/eJzVVM1KxDAQfpVhTgr1IizLlt2CCF4F9ZhLdGclkKbd%2FMCW0rfwcXwan8SsWW27sd0qXixzmDTffN83bSY1Kp4TpspJmaDkFWlMsWa4Y5guZvOEYeWzy%2FnCZ5Z21i8Ywg%2Be29KKQnEJxnJLUHLNc8bUKIjr55jo7eXVR1R62Dplo4Xc0dYJTWvIi7XYCNIDnnpVvqjF9%2BzF2sFlsBvCCdg49bTvsVcx4vtb2u7ySlXAjRHG%2BlY%2BOjBBdYSqza6LvCwMf5Q0VS%2FqDjq%2FzVYlDfV1zPMrpWHSf6w1JeDz4HfejGCH9ybrTQT%2BUan%2FEk4s7%2Fdj%2F%2BAPUQZ1uAOSdtwuMrg5TM9ZuB9WEWb1lSawPBqL7Bwahg027%2Byjz8s%3D)
*/

@Suppress("IfThenToElvis") // The elvis is less readable here
val stateType =
if (stateParam == null) CIRCUIT_UI_STATE else stateParam.type.resolve().toTypeName()
val stateArg = if (stateParam == null) "_" else "state"
val stateParamBlock =
if (stateParam == null) CodeBlock.of("")
else CodeBlock.of("%L·=·state,·", stateParam.name!!.getShortName())
val modifierParamBlock =
CodeBlock.of("%L·=·modifier", modifierParam.name!!.getShortName())
val assistedParamsBlock =
if (assistedParams.isEmpty()) {
CodeBlock.of("")
} else {
CodeBlock.of(",·%L", assistedParams)
}
CodeBlock.of(
"%M<%T>·{·%L,·modifier·->·%M(%L%L%L)·}",
MemberName("com.slack.circuit", "ui"),
stateType,
stateArg,
MemberName(packageName, name),
stateParamBlock,
modifierParamBlock,
assistedParamsBlock
)
}
}
}
Expand Down Expand Up @@ -291,8 +331,8 @@ private class CircuitSymbolProcessor(
.getAllSuperTypes()
.mapNotNull {
when (it.declaration.qualifiedName?.asString()) {
CIRCUIT_UI -> FactoryType.UI
CIRCUIT_PRESENTER -> FactoryType.PRESENTER
CIRCUIT_UI.canonicalName -> FactoryType.UI
CIRCUIT_PRESENTER.canonicalName -> FactoryType.PRESENTER
else -> null
}
}
Expand Down Expand Up @@ -361,7 +401,7 @@ private fun KSFunctionDeclaration.assistedParameters(
symbols: CircuitSymbols,
logger: KSPLogger,
screenType: KSType,
allowNavigator: Boolean
allowNavigator: Boolean,
): CodeBlock {
return buildSet {
for (param in parameters) {
Expand Down Expand Up @@ -410,22 +450,17 @@ private fun KSType.isInstanceOf(type: KSType): Boolean {
private fun TypeSpec.Builder.buildUiFactory(
originatingSymbol: KSAnnotated,
screenBranch: CodeBlock,
instantiationCodeBlock: CodeBlock
instantiationCodeBlock: CodeBlock,
): TypeSpec {
return addSuperinterface(Ui.Factory::class)
return addSuperinterface(CIRCUIT_UI_FACTORY)
.addFunction(
FunSpec.builder("create")
.addModifiers(KModifier.OVERRIDE)
.addParameter("screen", Screen::class)
.addParameter("context", CircuitContext::class)
.returns(ScreenUi::class.asClassName().copy(nullable = true))
.addParameter("screen", SCREEN)
.addParameter("context", CIRCUIT_CONTEXT)
.returns(CIRCUIT_UI.parameterizedBy(STAR).copy(nullable = true))
.beginControlFlow("return·when·(screen)")
.addStatement(
"%L·->·%T(%L)",
screenBranch,
ScreenUi::class.asTypeName(),
instantiationCodeBlock
)
.addStatement("%L·->·%L", screenBranch, instantiationCodeBlock)
.addStatement("else·->·null")
.endControlFlow()
.build()
Expand All @@ -437,7 +472,7 @@ private fun TypeSpec.Builder.buildUiFactory(
private fun TypeSpec.Builder.buildPresenterFactory(
originatingSymbol: KSAnnotated,
screenBranch: CodeBlock,
instantiationCodeBlock: CodeBlock
instantiationCodeBlock: CodeBlock,
): TypeSpec {
// The TypeSpec below will generate something similar to the following.
// public class AboutPresenterFactory : Presenter.Factory {
Expand All @@ -452,14 +487,14 @@ private fun TypeSpec.Builder.buildPresenterFactory(
// }
// }

return addSuperinterface(Presenter.Factory::class)
return addSuperinterface(CIRCUIT_PRESENTER_FACTORY)
.addFunction(
FunSpec.builder("create")
.addModifiers(KModifier.OVERRIDE)
.addParameter("screen", Screen::class)
.addParameter("navigator", Navigator::class)
.addParameter("context", CircuitContext::class)
.returns(Presenter::class.asClassName().parameterizedBy(STAR).copy(nullable = true))
.addParameter("screen", SCREEN)
.addParameter("navigator", NAVIGATOR)
.addParameter("context", CIRCUIT_CONTEXT)
.returns(CIRCUIT_PRESENTER.parameterizedBy(STAR).copy(nullable = true))
.beginControlFlow("return when (screen)")
.addStatement("%L·->·%L", screenBranch, instantiationCodeBlock)
.addStatement("else·->·null")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ app.cash.turbine:turbine
com.google.guava:listenablefuture
org.jetbrains.compose.runtime:runtime-saveable
org.jetbrains.compose.runtime:runtime
org.jetbrains.compose.ui:ui
org.jetbrains.kotlin:kotlin-bom
org.jetbrains.kotlin:kotlin-stdlib-jdk7
org.jetbrains.kotlin:kotlin-stdlib-jdk8
Expand Down
Loading

0 comments on commit d5a2190

Please sign in to comment.