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

Update README about using custom Compose Compiler #2526

Merged
merged 2 commits into from Dec 15, 2022
Merged

Conversation

igordmn
Copy link
Collaborator

@igordmn igordmn commented Dec 2, 2022

No description provided.

igordmn added a commit that referenced this pull request Dec 2, 2022
Fixes #2459

Readme: #2526

1. Add `dependencies: Dependencies` extension that is accessible in `compose { }` block
2. Add `Dependencies.compiler` property that can return versions of Compose compiler used by the plugin:
```
compose {
    kotlinCompilerPlugin.set(dependencies.compiler.forKotlin("1.7.20"))
    //kotlinCompilerPlugin.set(dependencies.compiler.auto) // determined by applied version of Kotlin. It is a default.
}
```

3. Add ability to set arguments for Compose Compiler. Now we can write:
```
compose {
    kotlinCompilerPlugin.set(dependencies.compiler.forKotlin("1.7.20"))
    kotlinCompilerPluginArgs.add("suppressKotlinVersionCompatibilityCheck=1.7.21")
}
```

4. Remove checks for different targets

We had a separate check for JS, when we released 1.2.0. It doesn't support Kotlin 1.7.20 at that moment.

It is hard to refactor this feature in the new code, so I removed it. It is not needed now and it had an ugly code. When we will need it again, we'll write it again.

5. Remove the `compose.tests.androidx.compiler.version` property from gradle.properties and remove `defaultAndroidxCompilerEnvironment`

Because they are used only in one test, and it seems there is no reason to use it in another place in the future
@igordmn igordmn changed the base branch from master to release/1.2.2 December 2, 2022 19:13
@igordmn igordmn changed the base branch from release/1.2.2 to master December 2, 2022 19:15
@@ -30,9 +30,40 @@ Kotlin version | Minimal Compose version | 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 | JS is not supported (will be fixed in the next versions)
1.7.20 | 1.2.0 | JS is not supported (fixed in the next version)
1.7.20 | 1.2.1
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 PR should be merged only after 1.2.2 is released

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's better to use the exact version instead of "the next version"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

@igordmn igordmn marked this pull request as ready for review December 2, 2022 19:16
@@ -30,9 +30,40 @@ Kotlin version | Minimal Compose version | 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 | JS is not supported (will be fixed in the next versions)
1.7.20 | 1.2.0 | JS is not supported (fixed in the next version)
1.7.20 | 1.2.1
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's better to use the exact version instead of "the next version"

VERSIONING.md Outdated
1.7.20 | 1.2.1

### Using the latest Kotlin version

When a new version of Kotlin is released, the corresponding Compose Multiplatform release may not yet have been published. There are still ways to use it, although stability is not garantied.
Copy link
Collaborator

Choose a reason for hiding this comment

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

garantied should be guaranteed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

VERSIONING.md Outdated

#### Using Jetpack Compose Compiler

Compose Multiplatform uses Compose Compiler to compile Compose code, and its version is strictly bound to the version of Kotlin. By default it chooses appropriate `org.jetbrains.compose.compiler:compiler` version by the Kotlin version applied to the Gradle project. But there is a way to choose another version of Compose Compiler. For example, you can use Jetpack Compose Compiler published by Google
Copy link
Collaborator

Choose a reason for hiding this comment

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

Compose Multiplatform uses Compose Compiler to compile Compose code, and its version is strictly bound to the version of Kotlin.

This sentence repeats the word "Compose" a bit too often. At the same time, the whole paragraph might be a bit confusing, when someone does not understand how the Gradle plugin and the compiler plugin relate to each other.

By default it chooses

By default needs a comma , in the beginning of a sentence (By default,). Also, it might be confusing, what it refers to.

chooses appropriate .. version by the Kotlin version applied to the Gradle project`.

I'm not sure about articles here. appropriate definitely needs one. Also, I'm not sure, why the Kotlin version and the Gradle project?

My suggestion:

The compilation process of composable functions is handled by the Compose compiler plugin. Each release of the compiler plugin is strictly bound to a single version of the Kotlin compiler. Normally, the Gradle plugin chooses an appropriate version of the compiler plugin automatically. But there is a way to choose another version of Compose Compiler. For example, you can use Jetpack Compose Compiler published by Google.

VERSIONING.md Outdated

Compose Multiplatform uses Compose Compiler to compile Compose code, and its version is strictly bound to the version of Kotlin. By default it chooses appropriate `org.jetbrains.compose.compiler:compiler` version by the Kotlin version applied to the Gradle project. But there is a way to choose another version of Compose Compiler. For example, you can use Jetpack Compose Compiler published by Google

First check [this page](https://developer.android.com/jetpack/androidx/releases/compose-kotlin#pre-release_kotlin_compatibility) to find a compatible version. If there is one, use it this way:
Copy link
Collaborator

Choose a reason for hiding this comment

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

First,

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

VERSIONING.md Outdated

#### Disabling Kotlin compatability check

If there is no compatible version of Jetpack Compose Compiler, or you encounter errors, you can try to use Compose Compiler for another version of Kotlin, but disable the Kotlin version check:
Copy link
Collaborator

Choose a reason for hiding this comment

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

or you encounter errors

Seems like a very broad suggestion. Ideally, I would like to describe the specific issues, which might be solved by disabling the compatibility check.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need a really big warning about this, so people would not try this for a new major version.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

or you encounter errors

It is about errors when you try Jetpack Compose Compiler.

There is no list of specific issues, which we can write here. We can encounter any errors in compile time and runtime, when we using a Jetpack Compose Compiler version that is not tested with Compose Multiplatform.

One of the recent examples - Jetpack Compose Compiler didn't work in the JS target. First we had a compilation error. We fixed that, but recently discovered a runtime error.

I think we need a really big warning about this

I added an additional note. Not sure that for major version of Kotlin we should write a separate warning. I wouldn't recommend overwriting compiler even for minor Kotlin's.

```
compose {
kotlinCompilerPlugin.set(dependencies.compiler.forKotlin("1.7.20"))
kotlinCompilerPluginArgs.add("suppressKotlinVersionCompatibilityCheck=1.7.21")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we just add suppressKotlinCompatibilityCheck property instead of very generic kotlinCompilerPluginArgs? I'm not sure that giving an ability to pass arbitrary arguments to the compiler plugin just for suppressing the check is worth it.

compose {
    kotlinCompilerPlugin.set(dependencies.compiler.forKotlin("1.7.20"))
    suppressKotlinCompatibilityCheck.set("1.7.21")
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The reason why I added support for arbitrary properties - because there are other useful properties besides suppressKotlinCompatibilityCheck, and I wouldn't want to modify code every time new properties/rules are added. For example, the useful metricsDestination property was added recently.

Keeping API under control is usually good practice, and allows avoiding many issues, but here we explicitly tell that we use a separate entity (Compose Compiler), and so we can support configuration of it bypassing our API, and reusing its own way of configuration (via properties).

Besides that, we should emphasis that we configure the applied Compose Compiler. Without specifying a custom Compose Compiler, arguments don't make sense, because Compiler can be different for each Kotlin and doesn't have required arguments. In case suppressKotlinCompatibilityCheck we have another problem - it won't be used until we set a custom compiler.

So, if we really don't want to pass arbitrary properties (but I still prefer to pass because it is easier to maintain), we should keep somehow connection to Compose Compiler. The alternative are:

compose {
    kotlinCompilerPlugin(dependencies.compiler.forKotlin("1.7.20")) {
        suppressKotlinCompatibilityCheck = "1.7.21"
    }
}

// and we can still write
compose {
    kotlinCompilerPlugin("androidx.compose.compiler:compiler:1.4.0-alpha02")
}

(I am not sure about the style guidelines, maybe it can be written better)

or

compose {
    kotlinCompilerPlugin.set(dependencies.compiler.forKotlin("1.7.20"))
    kotlinCompilerPluginArgs {
        suppressKotlinCompatibilityCheck.set("1.7.21")
    }
}

Copy link
Collaborator Author

@igordmn igordmn Dec 8, 2022

Choose a reason for hiding this comment

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

The alternative are

It seems that they won't work well. For example, we have the property metricsDestination. It only makes sense for the latest Compose Compiler, and will be ignored in the previous versions.

VERSIONING.md Outdated

If you use the hotfix version of Kotlin, it probably will work.

The `kotlinCompilerPluginArgs` here configures the applied Compose Compiler and disables the internal Kotlin check that happens inside it.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure this sentence explains anything. I mean it's obvious, that suppressKotlinVersionCompatibilityCheck disables the check. However, it might be not obvious why it needs a version.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

However, it might be not obvious why it needs a version.

Is it better in the new version of the doc?

igordmn added a commit that referenced this pull request Dec 13, 2022
* Improve DSL for setting a custom Compose Plugin

Fixes #2459

Readme: #2526

1. Add `dependencies: Dependencies` extension that is accessible in `compose { }` block
2. Add `Dependencies.compiler` property that can return versions of Compose compiler used by the plugin:
```
compose {
    kotlinCompilerPlugin.set(dependencies.compiler.forKotlin("1.7.20"))
    //kotlinCompilerPlugin.set(dependencies.compiler.auto) // determined by applied version of Kotlin. It is a default.
}
```

3. Add ability to set arguments for Compose Compiler. Now we can write:
```
compose {
    kotlinCompilerPlugin.set(dependencies.compiler.forKotlin("1.7.20"))
    kotlinCompilerPluginArgs.add("suppressKotlinVersionCompatibilityCheck=1.7.21")
}
```

4. Remove checks for different targets

We had a separate check for JS, when we released 1.2.0. It doesn't support Kotlin 1.7.20 at that moment.

It is hard to refactor this feature in the new code, so I removed it. It is not needed now and it had an ugly code. When we will need it again, we'll write it again.

5. Remove the `compose.tests.androidx.compiler.version` property from gradle.properties and remove `defaultAndroidxCompilerEnvironment`

Because they are used only in one test, and it seems there is no reason to use it in another place in the future

* Discussions
igordmn added a commit that referenced this pull request Dec 13, 2022
* Improve DSL for setting a custom Compose Plugin

Fixes #2459

Readme: #2526

1. Add `dependencies: Dependencies` extension that is accessible in `compose { }` block
2. Add `Dependencies.compiler` property that can return versions of Compose compiler used by the plugin:
```
compose {
    kotlinCompilerPlugin.set(dependencies.compiler.forKotlin("1.7.20"))
    //kotlinCompilerPlugin.set(dependencies.compiler.auto) // determined by applied version of Kotlin. It is a default.
}
```

3. Add ability to set arguments for Compose Compiler. Now we can write:
```
compose {
    kotlinCompilerPlugin.set(dependencies.compiler.forKotlin("1.7.20"))
    kotlinCompilerPluginArgs.add("suppressKotlinVersionCompatibilityCheck=1.7.21")
}
```

4. Remove checks for different targets

We had a separate check for JS, when we released 1.2.0. It doesn't support Kotlin 1.7.20 at that moment.

It is hard to refactor this feature in the new code, so I removed it. It is not needed now and it had an ugly code. When we will need it again, we'll write it again.

5. Remove the `compose.tests.androidx.compiler.version` property from gradle.properties and remove `defaultAndroidxCompilerEnvironment`

Because they are used only in one test, and it seems there is no reason to use it in another place in the future

* Discussions
igordmn added a commit that referenced this pull request Dec 15, 2022
* Compose 1.2.1-rc01

* Fix Web build for Kotlin 1.7.20

* Use 1.3.2.1-rc02 in Gradle plugin

* Fix Gradle Plugin tests

* Fix Gradle Plugin tests

* Compose 1.2.1-rc03

* Update CHANGELOG.md

* Update CHANGELOG.md

* Compose Compiler 1.3.2.1

* Compose 1.2.1

* Update VERSIONING.md

* Update gradle.properties

* Fix custom JDK tests on Linux

* Remove JVM target version override (#2515)

Previously, we were setting kotlin.jvmTarget version
to 1.8 if it was null or < 1.8.
As an unintended consequence we were also overriding
a version set by the jvmToolchain property.
So while users expected the jvmToolchain property
to set both jdk home & jdk target, we were quietly
overriding jdk target.

At the same time, Kotlin 1.7 sets the minimum
target version to 1.8 anyway, so our override
does not make sense with Kotlin 1.7+.

This commit removes overriding altogether.

Fixes #2511

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update Compose

* Update default ProGuard rules with changes from main branch

* Test Gradle plugin on relevant PRs (#2509)

* Update Gradle used in tooling subprojects

* Update Kotlin in Compose Gradle plugin

* Decrease verbosity of Gradle plugin tests

* Disable mac sign test

* Add workflow to test Gradle plugin

* Fix custom jdk tests on Linux

* Make Compose Gradle plugin build compatible with Configuration cache

* Print tests summary

* Remove unused code

* Refactor tests configuration

* Turn off parallel execution

* Try adding windows runner

* Turn off fail fast

* Fix Windows test issues

#2368

* Adjust default proguard rules

The following rule is needed to fix tests on Windows:
```
-dontwarn org.graalvm.compiler.core.aarch64.AArch64NodeMatchRules_MatchStatementSet*
```

Other rules are just to make builds less noisy.
Kotlin's `*.internal` packages often contain
bytecode, which triggers ProGuard's notes.
However, these notes are not actionable for
most users, so we can ignore notes by default.

#2393
# Conflicts:
#	gradle-plugins/gradle/wrapper/gradle-wrapper.properties

* Improve DSL for setting a custom Compose Plugin (#2527)

* Improve DSL for setting a custom Compose Plugin

Fixes #2459

Readme: #2526

1. Add `dependencies: Dependencies` extension that is accessible in `compose { }` block
2. Add `Dependencies.compiler` property that can return versions of Compose compiler used by the plugin:
```
compose {
    kotlinCompilerPlugin.set(dependencies.compiler.forKotlin("1.7.20"))
    //kotlinCompilerPlugin.set(dependencies.compiler.auto) // determined by applied version of Kotlin. It is a default.
}
```

3. Add ability to set arguments for Compose Compiler. Now we can write:
```
compose {
    kotlinCompilerPlugin.set(dependencies.compiler.forKotlin("1.7.20"))
    kotlinCompilerPluginArgs.add("suppressKotlinVersionCompatibilityCheck=1.7.21")
}
```

4. Remove checks for different targets

We had a separate check for JS, when we released 1.2.0. It doesn't support Kotlin 1.7.20 at that moment.

It is hard to refactor this feature in the new code, so I removed it. It is not needed now and it had an ugly code. When we will need it again, we'll write it again.

5. Remove the `compose.tests.androidx.compiler.version` property from gradle.properties and remove `defaultAndroidxCompilerEnvironment`

Because they are used only in one test, and it seems there is no reason to use it in another place in the future

* Discussions

* Update ComposeCompilerCompatability.kt (#2557)

* Update CHANGELOG.md

* 1.2.2-rc01

* Update Compose

* Update CHANGELOG.md

* Compose 1.2.2

* Remove shared.podspec

* Remove usages of deprecated Intellij APIs

Co-authored-by: Alexey Tsvetkov <alexey.tsvetkov@jetbrains.com>
Co-authored-by: Alexey Tsvetkov <654232+AlexeyTsvetkov@users.noreply.github.com>
@igordmn igordmn merged commit eb60a5b into master Dec 15, 2022
@igordmn igordmn deleted the igordmn-patch-2 branch December 15, 2022 03:19
IlyaGulya pushed a commit to IlyaGulya/TodoAppDecomposeMviKotlin that referenced this pull request Aug 11, 2023
* Compose 1.2.1-rc01

* Fix Web build for Kotlin 1.7.20

* Use 1.3.2.1-rc02 in Gradle plugin

* Fix Gradle Plugin tests

* Fix Gradle Plugin tests

* Compose 1.2.1-rc03

* Update CHANGELOG.md

* Update CHANGELOG.md

* Compose Compiler 1.3.2.1

* Compose 1.2.1

* Update VERSIONING.md

* Update gradle.properties

* Fix custom JDK tests on Linux

* Remove JVM target version override (#2515)

Previously, we were setting kotlin.jvmTarget version
to 1.8 if it was null or < 1.8.
As an unintended consequence we were also overriding
a version set by the jvmToolchain property.
So while users expected the jvmToolchain property
to set both jdk home & jdk target, we were quietly
overriding jdk target.

At the same time, Kotlin 1.7 sets the minimum
target version to 1.8 anyway, so our override
does not make sense with Kotlin 1.7+.

This commit removes overriding altogether.

Fixes #2511

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update Compose

* Update default ProGuard rules with changes from main branch

* Test Gradle plugin on relevant PRs (#2509)

* Update Gradle used in tooling subprojects

* Update Kotlin in Compose Gradle plugin

* Decrease verbosity of Gradle plugin tests

* Disable mac sign test

* Add workflow to test Gradle plugin

* Fix custom jdk tests on Linux

* Make Compose Gradle plugin build compatible with Configuration cache

* Print tests summary

* Remove unused code

* Refactor tests configuration

* Turn off parallel execution

* Try adding windows runner

* Turn off fail fast

* Fix Windows test issues

#2368

* Adjust default proguard rules

The following rule is needed to fix tests on Windows:
```
-dontwarn org.graalvm.compiler.core.aarch64.AArch64NodeMatchRules_MatchStatementSet*
```

Other rules are just to make builds less noisy.
Kotlin's `*.internal` packages often contain
bytecode, which triggers ProGuard's notes.
However, these notes are not actionable for
most users, so we can ignore notes by default.

#2393
# Conflicts:
#	gradle-plugins/gradle/wrapper/gradle-wrapper.properties

* Improve DSL for setting a custom Compose Plugin (#2527)

* Improve DSL for setting a custom Compose Plugin

Fixes JetBrains/compose-multiplatform#2459

Readme: JetBrains/compose-multiplatform#2526

1. Add `dependencies: Dependencies` extension that is accessible in `compose { }` block
2. Add `Dependencies.compiler` property that can return versions of Compose compiler used by the plugin:
```
compose {
    kotlinCompilerPlugin.set(dependencies.compiler.forKotlin("1.7.20"))
    //kotlinCompilerPlugin.set(dependencies.compiler.auto) // determined by applied version of Kotlin. It is a default.
}
```

3. Add ability to set arguments for Compose Compiler. Now we can write:
```
compose {
    kotlinCompilerPlugin.set(dependencies.compiler.forKotlin("1.7.20"))
    kotlinCompilerPluginArgs.add("suppressKotlinVersionCompatibilityCheck=1.7.21")
}
```

4. Remove checks for different targets

We had a separate check for JS, when we released 1.2.0. It doesn't support Kotlin 1.7.20 at that moment.

It is hard to refactor this feature in the new code, so I removed it. It is not needed now and it had an ugly code. When we will need it again, we'll write it again.

5. Remove the `compose.tests.androidx.compiler.version` property from gradle.properties and remove `defaultAndroidxCompilerEnvironment`

Because they are used only in one test, and it seems there is no reason to use it in another place in the future

* Discussions

* Update ComposeCompilerCompatability.kt (#2557)

* Update CHANGELOG.md

* 1.2.2-rc01

* Update Compose

* Update CHANGELOG.md

* Compose 1.2.2

* Remove shared.podspec

* Remove usages of deprecated Intellij APIs

Co-authored-by: Alexey Tsvetkov <alexey.tsvetkov@jetbrains.com>
Co-authored-by: Alexey Tsvetkov <654232+AlexeyTsvetkov@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants