Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert STAR sample to KMP #1103

Merged
merged 61 commits into from
Jan 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
a0254a3
Abstract android bits out of TokenStorage
ZacSweers Oct 7, 2023
1510f37
Extract out android APIs in PetRepository
ZacSweers Oct 7, 2023
41af3e6
Use compose key events in PetPhotoCarousel
ZacSweers Oct 7, 2023
f105cfd
Move preview out + use entries
ZacSweers Oct 7, 2023
364cd27
Fix typo
ZacSweers Oct 7, 2023
b2a95f4
Switch out deps
ZacSweers Oct 7, 2023
fc91c6a
Merge branch 'main' into z/kmpStar
ZacSweers Oct 19, 2023
7c2aca1
Merge branch 'main' into z/kmpStar
ZacSweers Jan 6, 2024
fd6951f
Add coil3 coordinates
ZacSweers Jan 6, 2024
1bc471c
More dep organization
ZacSweers Jan 6, 2024
6bac40c
Fix coil3 group
ZacSweers Jan 6, 2024
3b62dfc
Finish build file setup
ZacSweers Jan 6, 2024
5896b7d
Rename dirs to new structure
ZacSweers Jan 6, 2024
e7eaee9
Initial pass at aligning sources
ZacSweers Jan 6, 2024
c1b9af5
KMP SqlDriverFactory
ZacSweers Jan 6, 2024
986a5eb
KMP BackPressNavIcon
ZacSweers Jan 6, 2024
c182c2d
KMP ConditionalSystemUiColors
ZacSweers Jan 6, 2024
a2ac08c
Introduce common parcelizing APIs
ZacSweers Jan 7, 2024
908a873
Misc work and prep
ZacSweers Jan 7, 2024
be41cf6
Spotless pass
ZacSweers Jan 7, 2024
9135336
Android builds now
ZacSweers Jan 7, 2024
27cdde7
Update CoilRule to c3
ZacSweers Jan 7, 2024
7bc1b5e
STAR apk builds
ZacSweers Jan 7, 2024
4f3dda9
Extract more things down to commonMain
ZacSweers Jan 7, 2024
27496b0
Extract ListBenchmarksScreen
ZacSweers Jan 7, 2024
3c994bd
Push whole impls down into common using typealias trick
ZacSweers Jan 7, 2024
3e81bf9
More extractions
ZacSweers Jan 7, 2024
ca26de7
Desktop is working!
ZacSweers Jan 7, 2024
461f53f
Source jvmTarget from libs
ZacSweers Jan 7, 2024
d608b9f
Verify star desktop on CI
ZacSweers Jan 7, 2024
7360d2e
Formatting
ZacSweers Jan 7, 2024
c0c7b32
More misc
ZacSweers Jan 7, 2024
e8a136b
Spotless and disable another noisy warning + comment
ZacSweers Jan 7, 2024
1efe41e
More misc
ZacSweers Jan 7, 2024
396e1b2
Improve landscape detection on desktop
ZacSweers Jan 7, 2024
655fbf4
More cleanups and tweaks
ZacSweers Jan 7, 2024
6849346
Implement filters support on desktop
ZacSweers Jan 7, 2024
5096784
Add dark mode to desktop
ZacSweers Jan 7, 2024
46747dd
Gate photo full screen overlay on android
ZacSweers Jan 7, 2024
c8cdad0
Fall back to a warning vector if there's no image url
ZacSweers Jan 8, 2024
4d85c60
Icon cleanup
ZacSweers Jan 8, 2024
a4bb63a
Handle url clicks
ZacSweers Jan 8, 2024
f527798
Get CI check working
ZacSweers Jan 8, 2024
9ec3e30
Remove hamcrest
ZacSweers Jan 8, 2024
ebe86c1
Run star instrumentation tests on CI too
ZacSweers Jan 8, 2024
2426c77
Use kotlinx-datetime for multiplatform
ZacSweers Jan 8, 2024
adbda8c
More time APIs
ZacSweers Jan 8, 2024
b148272
Use in-memory FS for token storage on desktop
ZacSweers Jan 8, 2024
a2ca059
Merge branch 'main' into z/kmpStar
ZacSweers Jan 8, 2024
cd63a5f
Try an explicitly ATD model
ZacSweers Jan 8, 2024
eb4566f
Fix leakcanary config
ZacSweers Jan 8, 2024
b3f1354
Add missing rules
ZacSweers Jan 8, 2024
da0d732
Update profile generation for AGP 8.2 + enable startup-prof
ZacSweers Jan 8, 2024
4c2a768
Fix AboutScreen on android
ZacSweers Jan 8, 2024
3a2dbe8
Fix wrong platform type on android
ZacSweers Jan 8, 2024
30733a6
Update EW config
ZacSweers Jan 8, 2024
1215b4c
Spotless
ZacSweers Jan 8, 2024
d9a0bbc
Regenerate snapshots to make lfs happy?
ZacSweers Jan 8, 2024
fa28fe3
Fix test
ZacSweers Jan 8, 2024
c3b34a7
More fixes
ZacSweers Jan 8, 2024
b949d2a
Update samples/star/README.md
ZacSweers Jan 11, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ jobs:
detektTest \
assembleAndroidTest

- name: Verify STAR desktop builds
id: gradle-build-star-desktop
run: |
./gradlew :samples:star:jvmJar -Pcircuit.buildDesktop

# Defer these until after the above run, no need to waste resources running them if there are other failures first
- name: Run instrumentation tests via emulator.wtf (main repo only)
if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository
Expand Down
33 changes: 27 additions & 6 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import io.gitlab.arturbosch.detekt.extensions.DetektExtension
import java.net.URI
import org.jetbrains.compose.ComposeExtension
import org.jetbrains.dokka.gradle.DokkaTaskPartial
import org.jetbrains.kotlin.gradle.dsl.JvmTarget.JVM_11
import org.jetbrains.kotlin.gradle.dsl.JvmTarget
import org.jetbrains.kotlin.gradle.dsl.KotlinJvmCompilerOptions
import org.jetbrains.kotlin.gradle.dsl.KotlinMultiplatformExtension
import org.jetbrains.kotlin.gradle.dsl.KotlinProjectExtension
Expand Down Expand Up @@ -95,7 +95,13 @@ allprojects {
format("license") {
licenseHeaderFile(rootProject.file("spotless/spotless.kt"), "(package|@file:)")
target("src/**/*.kt")
targetExclude("**/circuit/backstack/**/*.kt")
targetExclude(
"**/circuit/backstack/**/*.kt",
"**/HorizontalPagerIndicator.kt",
"**/FilterList.kt",
"**/Remove.kt",
"**/Pets.kt",
)
}
}
configure<SpotlessExtension> {
Expand Down Expand Up @@ -131,6 +137,8 @@ fun Project.configureComposeBom(dependencyHandler: DependencyHandler) {
}
}

val jvmTargetVersion = libs.versions.jvmTarget

subprojects {
pluginManager.withPlugin("java") {
configure<JavaPluginExtension> {
Expand All @@ -141,7 +149,9 @@ subprojects {
}
}

tasks.withType<JavaCompile>().configureEach { options.release.set(11) }
tasks.withType<JavaCompile>().configureEach {
options.release.set(jvmTargetVersion.map(String::toInt))
}

// This is the default base plugin applied on all projects, so safe to add this hook here
configureComposeBom(dependencies)
Expand All @@ -154,7 +164,7 @@ subprojects {
compilerOptions {
allWarningsAsErrors.set(true)
if (this is KotlinJvmCompilerOptions) {
jvmTarget.set(JVM_11)
jvmTarget.set(jvmTargetVersion.map(JvmTarget::fromTarget))
// Stub gen copies args from the parent compilation
if (this@configureEach !is KaptGenerateStubsTask) {
freeCompilerArgs.addAll(
Expand Down Expand Up @@ -448,8 +458,19 @@ subprojects {

pluginManager.withPlugin("wtf.emulator.gradle") {
val emulatorWtfToken = providers.gradleProperty("emulatorWtfToken")
if (emulatorWtfToken.isPresent) {
configure<EwExtension> { token.set(emulatorWtfToken) }
configure<EwExtension> {
devices.set(
listOf(
mapOf(
"model" to "Pixel2Atd",
"version" to "30",
"atd" to "true",
)
)
)
if (emulatorWtfToken.isPresent) {
token.set(emulatorWtfToken)
}
}
// We don't always run emulator.wtf on CI (forks can't access it), so we add this helper
// lifecycle task that depends on connectedCheck as an alternative. We do this only on projects
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ private class CircuitSymbolProcessor(
private val platforms: List<PlatformInfo>,
) : SymbolProcessor {

private val lenient = options["circuit.codegen.lenient"]?.toBoolean() ?: false

override fun process(resolver: Resolver): List<KSAnnotated> {
val symbols = CircuitSymbols.create(resolver) ?: return emptyList()
val codegenMode =
Expand Down Expand Up @@ -375,23 +377,28 @@ private class CircuitSymbolProcessor(
}
}
InstantiationType.CLASS -> {
val cd = annotatedElement as KSClassDeclaration
cd.checkVisibility(logger) {
val declaration = annotatedElement as KSClassDeclaration
declaration.checkVisibility(logger) {
return null
}
val isAssisted = cd.isAnnotationPresent(AssistedFactory::class)
val isAssisted =
if (lenient) {
declaration.annotations.any { it.shortName.asString().contains("AssistedFactory") }
} else {
declaration.isAnnotationPresent(AssistedFactory::class)
}
val creatorOrConstructor: KSFunctionDeclaration?
val targetClass: KSClassDeclaration
if (isAssisted) {
val creatorFunction = cd.getAllFunctions().filter { it.isAbstract }.single()
val creatorFunction = declaration.getAllFunctions().filter { it.isAbstract }.single()
creatorOrConstructor = creatorFunction
targetClass = creatorFunction.returnType!!.resolve().declaration as KSClassDeclaration
targetClass.checkVisibility(logger) {
return null
}
} else {
creatorOrConstructor = cd.primaryConstructor
targetClass = cd
creatorOrConstructor = declaration.primaryConstructor
targetClass = declaration
}
val useProvider =
!isAssisted && creatorOrConstructor?.isAnnotationPresent(Inject::class) == true
Expand All @@ -409,10 +416,16 @@ private class CircuitSymbolProcessor(
}
.firstOrNull()
?: run {
val annotationsString =
declaration.annotations.toList().joinToString {
it.annotationType.resolve().toTypeName().toString()
}
logger.error(
"Factory must be for a UI or Presenter class, but was " +
"${targetClass.qualifiedName?.asString()}. " +
"Supertypes: ${targetClass.getAllSuperTypes().toList()}",
"${targetClass.qualifiedName?.asString()}.\n" +
"Supertypes: ${targetClass.getAllSuperTypes().toList()}.\n" +
"isAssisted? ${isAssisted}\n" +
"Annotations: $annotationsString}",
Comment on lines +425 to +428
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This extra logging was helpful to figure out that KSP couldn't see through the actualized typealias so opted to leave it in

targetClass
)
return null
Expand Down Expand Up @@ -442,7 +455,9 @@ private class CircuitSymbolProcessor(
CodeBlock.of("provider.get()")
} else if (isAssisted) {
// Inject the target class's assisted factory that we'll call its create() on.
constructorParams.add(ParameterSpec.builder("factory", cd.toClassName()).build())
constructorParams.add(
ParameterSpec.builder("factory", declaration.toClassName()).build()
)
CodeBlock.of(
"factory.%L(%L)",
creatorOrConstructor!!.simpleName.getShortName(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1091,9 +1091,7 @@ class CircuitSymbolProcessorTest {
)
) { messages ->
assertThat(messages)
.contains(
"Factory must be for a UI or Presenter class, but was test.Favorites. Supertypes: [Any]"
)
.contains("Factory must be for a UI or Presenter class, but was test.Favorites.")
}
}

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import androidx.compose.ui.layout.layout
* A composable which optionally lays out the given [content], depending on what [shouldLayout]
* returns, whilst still keeping [content] attached to composition.
*
* This is useful when used in conjuction with `movableContentOf`, as that will destroy the
* This is useful when used in conjunction with `movableContentOf`, as that will destroy the
* content's state 'very soon' after the content is detached from composition. By using this
* composable, the content can remain attached to composition, but can avoid being measured, laid
* out or drawn, allowing us to reduce wasted work.
Expand Down
10 changes: 10 additions & 0 deletions docs/code-gen.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,16 @@ ksp {
}
```

If using Kotlin multiplatform with typealias annotations for Dagger annotations (i.e. expect
annotations in common with actual typealias declarations in JVM source sets), you can match on just
annotation short names alone to support this case via `circuit.codegen.lenient` mode.

```kotlin
ksp {
arg("circuit.codegen.lenient", "true")
}
```

## Usage

The primary entry point is the `CircuitInject` annotation.
Expand Down
6 changes: 6 additions & 0 deletions gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,17 @@ org.gradle.parallel=true
org.gradle.configureondemand=true
org.gradle.caching=true

# Disable noisy DAGP stability warning
dependency.analysis.compatibility=NONE
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opportunistic noise cleanup


# Disable noisy stability warning
kotlin.mpp.stability.nowarn=true
kotlin.mpp.androidSourceSetLayoutVersion=2
kotlin.mpp.androidGradlePluginCompatibility.nowarn=true

# https://kotlinlang.org/docs/ksp-multiplatform.html#avoid-the-ksp-configuration-on-ksp-1-0-1
systemProp.allowAllTargetConfiguration=false

# Enable for Compose iOS
org.jetbrains.compose.experimental.uikit.enabled=true
# Enable for Compose Web
Expand Down
Loading
Loading