Allow to use Compose on multiple Kotlin versions #2366

igordmn · 2022-10-06T20:45:48Z

JS target supports a lower version (1.7.10), because we have a bug in Koltin 1.7.20

Compose 1.2.0 will support:

1.7.20 and 1.7.10 for Android and Desktop
1.7.10 for JS

We will release the new patchset (1.2.1) with 1.7.2X support for JS later

JS target supports a lower version (1.7.10), because we have a bug in Koltin 1.7.20 Compose 1.2.0 will support: - 1.7.20 and 1.7.10 for Android and Desktop - 1.7.10 for JS We will release the new patchset (1.2.1) with 1.7.2X support for JS later

dima-avdeev-jb · 2022-10-07T10:55:05Z

gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/ComposeCompilerCompatability.kt

+        "1.7.10" -> ComposeCompilerVersion("1.3.0-alpha01")
+        "1.7.20" -> ComposeCompilerVersion("1.3.2-alpha01", isJsSupported = false)


1.3.0-alpha01 and 1.3.2-alpha01
Maybe we can use stable versions?
https://mvnrepository.com/artifact/androidx.compose.compiler/compiler

Yes, we will release stable versions (1.3.0, 1.3.2) and use them here, when we be sure that everything works.

dima-avdeev-jb · 2022-10-07T10:56:04Z

gradle-plugins/gradle.properties

-# Default version of Kotlin compatible with compose.compiler.version
-compose.compiler.compatible.kotlin.version=1.7.20
+# The latest version of Compose Compiler used by Gradle plugin. Used only in tests.
+compose.tests.compiler.version=1.3.2-alpha01


maybe we can you stable version? 1.3.2

AlexeyTsvetkov

All issues are not critical, but ideally we should look at them

AlexeyTsvetkov · 2022-10-07T12:30:15Z

README.md

@@ -72,4 +72,6 @@ Note that when you use Compose Multiplatform, you setup your project differently

 ## Getting latest version of Compose Multiplatform ##

-See https://github.com/JetBrains/compose-jb/releases/latest for the latest stable release or https://github.com/JetBrains/compose-jb/releases for all stable and dev releases.
+See [this page](https://github.com/JetBrains/compose-jb/releases/latest) to know the latest stable release or [this](https://github.com/JetBrains/compose-jb/releases) to know all stable and dev releases.


I think this could be simplified to a list:

The latest stable release;

The latest dev release;

Compatability and versioning overview;

Changelog.

Thanks! This is much better.

AlexeyTsvetkov · 2022-10-07T12:33:26Z

VERSIONING.md

-1.0.0+ | 1.5.31
-1.0.1-rc2+ | 1.6.10
-
+When the new version of Kotlin is released, the latest Compose Multiplatform version isn't supported yet.


A new version of Kotlin may be not supported immediately after release.

That is what I tried to write, but something went wrong :D.

I took your variant and slightly changed it.

AlexeyTsvetkov · 2022-10-07T14:13:03Z

VERSIONING.md


-### Gradle plugin compatibility
+But after some time we will release a version which supports the latest Kotlin.
+Starting from 1.2.0, Compose Multiplatform supports multiple versions of Kotlin.


the Compose Multiplatform Gradle plugin could be more precise

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.

AlexeyTsvetkov · 2022-10-07T14:22:30Z

VERSIONING.md

-* 1.0.0 works with Gradle 6.7 or later (7.2 is the latest tested version).
+Compose version | Kotlin version
+--- | ---
+1.0.0 | 1.5.31


Should the ordering be a reverse? I suppose the main use case for this table is to answer: "If I want Kotlin x.y.z, what version of Compose should I use?"

Kotlin Compose Notes

1.5.31 1.0.0

1.6.20 1.1.1

1.7.10 1.2.0

1.7.20 1.2.0 Compose Web is broken

Should the ordering be a reverse

You are right. Now, when we support multiple versions of Kotlin, users don't have to upgrade Kotlin when they upgrade Compose. And when they upgrade Kotlin, they need to know which version of Compose they should use.

Changed.

AlexeyTsvetkov · 2022-10-07T17:12:49Z

gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/ComposeCompilerCompatability.kt

+
+internal data class ComposeCompilerVersion(
+    val version: String,
+    val isJsSupported: Boolean = true


Why make the specific check for JS only? IMO using enum set of KotlinPlatformType would be cleaner and more flexible

I just kept the principle - don't add support if it is not needed yet :). And when I wanted to add supportedPlatforms, I felt into analytic paralysis :D (where can I find the list?). But now I realized that we can add unsupportedPlatforms. Added

AlexeyTsvetkov · 2022-10-07T18:29:42Z

...ns/compose/src/main/kotlin/org/jetbrains/compose/internal/ComposeCompilerArtifactProvider.kt

 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"


Consider using Kotlin team about kotl.in URL shortener or more general jb.gg URL shortener.
E.g. kotl.in/compose-compatibility would look nicer.
That's obviously not urgent, just nice to have

How can I create such link?

AlexeyTsvetkov · 2022-10-07T18:59:35Z

gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/ComposeCompilerCompatability.kt

+
+internal object ComposeCompilerCompatability {
+    fun compilerVersionFor(kotlinVersion: String): ComposeCompilerVersion? = when (kotlinVersion) {
+        "1.7.10" -> ComposeCompilerVersion("1.3.0-alpha01")


I wonder I we can use one table/json/xml file to generate both table in the docs and compatibility code

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.

AlexeyTsvetkov · 2022-10-07T19:12:24Z

...ns/compose/src/main/kotlin/org/jetbrains/compose/internal/ComposeCompilerArtifactProvider.kt

+                val version = requireNotNull(
+                    ComposeCompilerCompatability.compilerVersionFor(kotlinVersion)
+                ) {
+                    "This version of Compose Multiplatform doesn't support Kotlin " +


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.

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.

This version of Compose doesn't support Kotlin $kotlinVersion

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.

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.

AlexeyTsvetkov · 2022-10-07T19:51:15Z

...-plugins/compose/src/main/kotlin/org/jetbrains/compose/ComposeCompilerKotlinSupportPlugin.kt

@@ -52,6 +57,7 @@ class ComposeCompilerKotlinSupportPlugin : KotlinCompilerPluginSupportPlugin {

    override fun applyToCompilation(kotlinCompilation: KotlinCompilation<*>): Provider<List<SubpluginOption>> {
        val target = kotlinCompilation.target
+        composeCompilerArtifactProvider.checkTargetSupported(target)


The overall code design became confusing.
Previously, ComposeCompilerArtifactProvider had a single responsibility of providing a version of ComposeCompilerArtifact. Now ComposeCompilerArtifactProvider also knows how to check if a target is compatible with a Compose Compiler, but only if the default compiler is used.
So the following configuration

plugins { id("org.jetbrains.kotlin.multiplatform") version "1.7.20" id("org.jetbrains.compose") version "1.2.0" } kotlin { js(IR) { browser() binaries.executable() } } compose { kotlinCompilerPlugin.set("1.3.2-alpha01") web {} }

will not trigger the check at all, despite that we know they are incompatible.

I suggest to separate compiler inference and compiler compatibility:

The inference should only try to choose a version. If inference fails, it should tell user to try updating the Compose Compiler (if a compatible version exists).

Then we run compiler compatibility checks, regardless of whether the version was inferred or set manually.

I suggest to separate compiler inference and compiler compatibility:

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 ComposeCompilerArtifactProvider from providing a version of ComposeCompilerArtifact to providing info about compatible compiler. It still can be considered as a single responsibility.

[Support multiple versions of Kotlin PR](#2366) breaks `kotlinCompilerPlugin` feature. `customPluginString` isn't set at the moment of plugin applying (or Provider's initialization), so we need to read it only when the artifact is requested.

* Fix `kotlinCompilerPlugin` property [Support multiple versions of Kotlin PR](#2366) breaks `kotlinCompilerPlugin` feature. `customPluginString` isn't set at the moment of plugin applying (or Provider's initialization), so we need to read it only when the artifact is requested. * Refactoring

igordmn requested a review from AlexeyTsvetkov October 6, 2022 20:45

Merge remote-tracking branch 'origin/master' into multipleKotlinVersions

f37f1e6

dima-avdeev-jb reviewed Oct 7, 2022

View reviewed changes

dima-avdeev-jb approved these changes Oct 7, 2022

View reviewed changes

Ability to pass a different kotlin version for checking JS tutorials

0daecf1

AlexeyTsvetkov approved these changes Oct 7, 2022

View reviewed changes

Discussions

9b3b769

igordmn merged commit 3996233 into master Oct 8, 2022

igordmn deleted the multipleKotlinVersions branch October 8, 2022 13:50

igordmn mentioned this pull request Oct 12, 2022

Fix kotlinCompilerPlugin property #2396

Merged

igordmn mentioned this pull request Nov 11, 2022

Kotlin 1.7.21 compatibility and adding an opt-out for compatibility checks #2459

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow to use Compose on multiple Kotlin versions #2366

Allow to use Compose on multiple Kotlin versions #2366

igordmn commented Oct 6, 2022

dima-avdeev-jb Oct 7, 2022 •

edited

Loading

igordmn Oct 7, 2022 •

edited

Loading

dima-avdeev-jb Oct 7, 2022

AlexeyTsvetkov left a comment

AlexeyTsvetkov Oct 7, 2022

igordmn Oct 7, 2022

AlexeyTsvetkov Oct 7, 2022

igordmn Oct 7, 2022

AlexeyTsvetkov Oct 7, 2022

igordmn Oct 7, 2022

AlexeyTsvetkov Oct 7, 2022

igordmn Oct 7, 2022

AlexeyTsvetkov Oct 7, 2022

igordmn Oct 7, 2022

AlexeyTsvetkov Oct 7, 2022

igordmn Oct 7, 2022

AlexeyTsvetkov Oct 7, 2022

igordmn Oct 7, 2022

AlexeyTsvetkov Oct 7, 2022

igordmn Oct 7, 2022

igordmn Oct 7, 2022

igordmn Oct 7, 2022

AlexeyTsvetkov Oct 7, 2022

igordmn Oct 7, 2022

		"1.7.10" -> ComposeCompilerVersion("1.3.0-alpha01")
		"1.7.20" -> ComposeCompilerVersion("1.3.2-alpha01", isJsSupported = false)

Kotlin	Compose	Notes
1.5.31	1.0.0
1.6.20	1.1.1
1.7.10	1.2.0
1.7.20	1.2.0	Compose Web is broken

Allow to use Compose on multiple Kotlin versions #2366

Allow to use Compose on multiple Kotlin versions #2366

Conversation

igordmn commented Oct 6, 2022

dima-avdeev-jb Oct 7, 2022 • edited Loading

Choose a reason for hiding this comment

igordmn Oct 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexeyTsvetkov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dima-avdeev-jb Oct 7, 2022 •

edited

Loading

igordmn Oct 7, 2022 •

edited

Loading