-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Allow to use Compose on multiple Kotlin versions #2366
Changes from all commits
647ea81
f37f1e6
0daecf1
9b3b769
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
#!/bin/bash | ||
|
||
# Script to build most of the examples, to verify if they can compile. | ||
# Don't add examples, which don't depend on maven.pkg.jetbrains.space, because they won't be able to compile. | ||
|
||
set -euo pipefail | ||
|
||
if [ "$#" -ne 2 ]; then | ||
echo "Specify Compose and Kotlin version. For example: ./validateExamplesWithJs.sh 1.1.1 1.6.10" | ||
exit 1 | ||
fi | ||
COMPOSE_VERSION=$1 | ||
KOTLIN_VERSION=$2 | ||
|
||
|
||
runGradle() { | ||
pushd $1 | ||
./gradlew $2 -Pcompose.version=$COMPOSE_VERSION -Pkotlin.version=$KOTLIN_VERSION | ||
popd | ||
} | ||
|
||
runGradle web-compose-bird build | ||
runGradle web-landing build | ||
runGradle web-with-react build |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
package org.jetbrains.compose | ||
|
||
import org.jetbrains.kotlin.gradle.plugin.KotlinPlatformType | ||
|
||
internal object ComposeCompilerCompatability { | ||
fun compilerVersionFor(kotlinVersion: String): ComposeCompilerVersion? = when (kotlinVersion) { | ||
"1.7.10" -> ComposeCompilerVersion("1.3.0-alpha01") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder I we can use one table/json/xml file to generate both table in the docs and compatibility code There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably it is overkill. If we will have difficulties of supporting docs, we can investigate. Currently, I don't see a solution yet, but already see multiple issues with this code generation :) Besides this place, we need to keep all other docs in sync too. |
||
"1.7.20" -> ComposeCompilerVersion( | ||
"1.3.2-alpha01", | ||
unsupportedPlatforms = setOf(KotlinPlatformType.js) | ||
) | ||
else -> null | ||
} | ||
} | ||
|
||
internal data class ComposeCompilerVersion( | ||
val version: String, | ||
val unsupportedPlatforms: Set<KotlinPlatformType> = emptySet() | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,13 +13,18 @@ import org.jetbrains.kotlin.gradle.plugin.* | |
import org.jetbrains.kotlin.gradle.targets.js.ir.KotlinJsIrTarget | ||
|
||
class ComposeCompilerKotlinSupportPlugin : KotlinCompilerPluginSupportPlugin { | ||
private var composeCompilerArtifactProvider = ComposeCompilerArtifactProvider { null } | ||
private lateinit var composeCompilerArtifactProvider: ComposeCompilerArtifactProvider | ||
|
||
override fun apply(target: Project) { | ||
super.apply(target) | ||
target.plugins.withType(ComposePlugin::class.java) { | ||
val composeExt = target.extensions.getByType(ComposeExtension::class.java) | ||
composeCompilerArtifactProvider = ComposeCompilerArtifactProvider { composeExt.kotlinCompilerPlugin.orNull } | ||
|
||
composeCompilerArtifactProvider = ComposeCompilerArtifactProvider( | ||
kotlinVersion = target.getKotlinPluginVersion() | ||
) { | ||
composeExt.kotlinCompilerPlugin.orNull | ||
} | ||
} | ||
} | ||
|
||
|
@@ -52,6 +57,7 @@ class ComposeCompilerKotlinSupportPlugin : KotlinCompilerPluginSupportPlugin { | |
|
||
override fun applyToCompilation(kotlinCompilation: KotlinCompilation<*>): Provider<List<SubpluginOption>> { | ||
val target = kotlinCompilation.target | ||
composeCompilerArtifactProvider.checkTargetSupported(target) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The overall code design became confusing.
will not trigger the check at all, despite that we know they are incompatible. I suggest to separate compiler inference and compiler compatibility:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
See my comment in the other discussion. We can't make compatibility check for an arbitrary compiler in the Gradle plugin. We only can make such check, if the compiler was chosen from our list. As for code quality, we expand responsibility of |
||
return target.project.provider { | ||
platformPluginOptions[target.platformType] ?: emptyList() | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,33 +5,67 @@ | |
|
||
package org.jetbrains.compose.internal | ||
|
||
import org.jetbrains.compose.ComposeBuildConfig | ||
import org.jetbrains.compose.ComposeCompilerCompatability | ||
import org.jetbrains.kotlin.gradle.plugin.KotlinPlatformType | ||
import org.jetbrains.kotlin.gradle.plugin.KotlinTarget | ||
import org.jetbrains.kotlin.gradle.plugin.SubpluginArtifact | ||
|
||
internal class ComposeCompilerArtifactProvider(private val customPluginString: () -> String?) { | ||
private const val KOTLIN_COMPATABILITY_LINK = | ||
"https://github.com/JetBrains/compose-jb/blob/master/VERSIONING.md#kotlin-compatibility" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using Kotlin team about There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How can I create such link? |
||
|
||
internal class ComposeCompilerArtifactProvider( | ||
private val kotlinVersion: String, | ||
customPluginString: () -> String? | ||
) { | ||
fun checkTargetSupported(target: KotlinTarget) { | ||
require(!unsupportedPlatforms.contains(target.platformType)) { | ||
"This version of Compose Multiplatform doesn't support Kotlin " + | ||
"$kotlinVersion for ${target.platformType} target. " + | ||
"Please see $KOTLIN_COMPATABILITY_LINK " + | ||
"to know the latest supported version of Kotlin." | ||
} | ||
} | ||
|
||
private var unsupportedPlatforms: Set<KotlinPlatformType> = emptySet() | ||
|
||
val compilerArtifact: SubpluginArtifact | ||
get() { | ||
val customPlugin = customPluginString() | ||
val customCoordinates = customPlugin?.split(":") | ||
return when (customCoordinates?.size) { | ||
null -> DefaultCompiler.pluginArtifact | ||
1 -> { | ||
val customVersion = customCoordinates[0] | ||
check(customVersion.isNotBlank()) { "'compose.kotlinCompilerPlugin' cannot be blank!" } | ||
DefaultCompiler.pluginArtifact.copy(version = customVersion) | ||
|
||
init { | ||
val customPlugin = customPluginString() | ||
val customCoordinates = customPlugin?.split(":") | ||
when (customCoordinates?.size) { | ||
null -> { | ||
val version = requireNotNull( | ||
ComposeCompilerCompatability.compilerVersionFor(kotlinVersion) | ||
) { | ||
"This version of Compose Multiplatform doesn't support Kotlin " + | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The message is a bit confusing. As I understand it, here we simply don't know, what version of Compose Compiler to choose. Instead the message says "this version of Compose doesn't support Kotlin $kotlinVersion", which may or may not be true. For example, if there is a version of a Compose Compiler, that works with current plugin and the desired version of Kotlin, then in my understanding the Gradle plugin & libraries are compatible, we simply don't know the right version of the compiler. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've been thinking about the best way to do this, and I couldn't invent the better than the current way. The idea is that if the user chooses Compiler themselves, the responsibility is on them. But if it is chosen by automatic selection, we can display a meaningful message. If we would add a mapping Compiler version -> supported platforms, then it always be incomplete. There won't be new or dev versions in this mapping. We can solve this issue changing Compose Compiler, but for just showing a message it seems overkill. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
And that is why we tell Compose Multiplatform instead of Compose Compiler :). Compose Multiplatform as a whole, and in a default state doesn't support it, but a custom compiler can theoretically support it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, there is an option to completely prohibit using Kotlin 1.7.20, even if the user sets compatible Compose Compiler. But I don't think it is a good idea. |
||
"$kotlinVersion. " + | ||
"Please see $KOTLIN_COMPATABILITY_LINK " + | ||
"to know the latest supported version of Kotlin." | ||
} | ||
3 -> DefaultCompiler.pluginArtifact.copy( | ||
groupId = customCoordinates[0], | ||
artifactId = customCoordinates[1], | ||
version = customCoordinates[2] | ||
|
||
compilerArtifact = DefaultCompiler.pluginArtifact( | ||
version = version.version | ||
) | ||
else -> error(""" | ||
unsupportedPlatforms = version.unsupportedPlatforms | ||
} | ||
1 -> { | ||
val customVersion = customCoordinates[0] | ||
check(customVersion.isNotBlank()) { "'compose.kotlinCompilerPlugin' cannot be blank!" } | ||
compilerArtifact = DefaultCompiler.pluginArtifact(version = customVersion) | ||
} | ||
3 -> compilerArtifact = DefaultCompiler.pluginArtifact( | ||
version = customCoordinates[2], | ||
groupId = customCoordinates[0], | ||
artifactId = customCoordinates[1], | ||
) | ||
else -> error(""" | ||
Illegal format of 'compose.kotlinCompilerPlugin' property. | ||
Expected format: either '<VERSION>' or '<GROUP_ID>:<ARTIFACT_ID>:<VERSION>' | ||
Actual value: '$customPlugin' | ||
""".trimIndent()) | ||
} | ||
} | ||
} | ||
|
||
val compilerHostedArtifact: SubpluginArtifact | ||
get() = compilerArtifact.run { | ||
|
@@ -47,10 +81,13 @@ internal class ComposeCompilerArtifactProvider(private val customPluginString: ( | |
const val GROUP_ID = "org.jetbrains.compose.compiler" | ||
const val ARTIFACT_ID = "compiler" | ||
const val HOSTED_ARTIFACT_ID = "compiler-hosted" | ||
const val VERSION = ComposeBuildConfig.composeCompilerVersion | ||
|
||
val pluginArtifact: SubpluginArtifact | ||
get() = SubpluginArtifact(groupId = GROUP_ID, artifactId = ARTIFACT_ID, version = VERSION) | ||
fun pluginArtifact( | ||
version: String, | ||
groupId: String = GROUP_ID, | ||
artifactId: String = ARTIFACT_ID, | ||
): SubpluginArtifact = | ||
SubpluginArtifact(groupId = groupId, artifactId = artifactId, version = version) | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
/* | ||
* Copyright 2020-2022 JetBrains s.r.o. and respective authors and developers. | ||
* Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE.txt file. | ||
*/ | ||
|
||
package org.jetbrains.compose.test.tests.integration | ||
|
||
import org.gradle.testkit.runner.TaskOutcome | ||
import org.gradle.testkit.runner.UnexpectedBuildFailure | ||
import org.jetbrains.compose.test.utils.GradlePluginTestBase | ||
import org.jetbrains.compose.test.utils.TestProjects | ||
import org.jetbrains.compose.test.utils.checks | ||
import org.junit.jupiter.api.Test | ||
import org.junit.jupiter.api.assertThrows | ||
|
||
class KotlinCompatabilityTest : GradlePluginTestBase() { | ||
@Test | ||
fun testKotlinMpp_1_7_10() = testMpp("1.7.10") | ||
|
||
@Test | ||
fun testKotlinJsMpp_1_7_10() = testJsMpp("1.7.10") | ||
|
||
@Test | ||
fun testKotlinMpp_1_7_20() = testMpp("1.7.20") | ||
|
||
@Test | ||
fun testKotlinJsMpp_1_7_20() { | ||
assertThrows<UnexpectedBuildFailure> { | ||
testJsMpp("1.7.20") | ||
} | ||
} | ||
|
||
private fun testMpp(kotlinVersion: String) = with( | ||
testProject( | ||
TestProjects.mpp, | ||
testEnvironment = defaultTestEnvironment.copy(kotlinVersion = kotlinVersion) | ||
) | ||
) { | ||
val logLine = "Kotlin MPP app is running!" | ||
gradle("run").build().checks { check -> | ||
check.taskOutcome(":run", TaskOutcome.SUCCESS) | ||
check.logContains(logLine) | ||
} | ||
} | ||
|
||
private fun testJsMpp(kotlinVersion: String) = with( | ||
testProject( | ||
TestProjects.jsMpp, | ||
testEnvironment = defaultTestEnvironment.copy(kotlinVersion = kotlinVersion) | ||
) | ||
) { | ||
gradle(":compileKotlinJs").build().checks { check -> | ||
check.taskOutcome(":compileKotlinJs", TaskOutcome.SUCCESS) | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the Compose Multiplatform Gradle plugin could be more precise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is details, and only obvious for people who knows how our Gradle plugin works. Compose Multiplatform is considered as a whole product. And one of the features of this product is supporting specific Kotlin version. This support can be achieved multiple ways - via Gradle plugin, via Kotlin compiler plugin, via IDE, via runtime dependencies, etc.