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

Fix skiko.js unpacking for k/js target #5105

Merged
merged 3 commits into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,17 @@ import org.gradle.api.artifacts.Configuration
import org.gradle.api.artifacts.ResolvedDependency
import org.gradle.api.artifacts.UnresolvedDependency
import org.gradle.api.artifacts.component.ModuleComponentIdentifier
import org.gradle.api.file.DuplicatesStrategy
import org.gradle.api.provider.Provider
import org.gradle.language.jvm.tasks.ProcessResources
import org.jetbrains.compose.ComposeBuildConfig
import org.jetbrains.compose.ComposeExtension
import org.jetbrains.compose.internal.utils.detachedComposeDependency
import org.jetbrains.compose.internal.utils.registerTask
import org.jetbrains.compose.web.WebExtension
import org.jetbrains.compose.web.tasks.UnpackSkikoWasmRuntimeTask
import org.jetbrains.kotlin.gradle.targets.js.KotlinWasmTargetType
import org.jetbrains.kotlin.gradle.targets.js.ir.KotlinJsIrTarget
import org.jetbrains.kotlin.gradle.tasks.IncrementalSyncTask

internal fun Project.configureWeb(
Expand Down Expand Up @@ -46,13 +50,16 @@ internal fun Project.configureWeb(
}
}

val targets = webExt.targetsToConfigure(project)

// configure only if there is k/wasm or k/js target:
if (webExt.targetsToConfigure(project).isNotEmpty()) {
configureWebApplication(project, shouldRunUnpackSkiko)
if (targets.isNotEmpty()) {
configureWebApplication(targets, project, shouldRunUnpackSkiko)
}
}

internal fun configureWebApplication(
targets: Collection<KotlinJsIrTarget>,
project: Project,
shouldRunUnpackSkiko: Provider<Boolean>
) {
Expand All @@ -76,9 +83,29 @@ internal fun configureWebApplication(
outputDir.set(unpackedRuntimeDir)
}

project.tasks.withType(IncrementalSyncTask::class.java) {
it.dependsOn(unpackRuntime)
it.from.from(unpackedRuntimeDir)
targets.forEach { target ->
target.compilations.all { compilation ->
// `wasmTargetType` is available starting with kotlin 1.9.2x
if (target.wasmTargetType != null) {
// Kotlin/Wasm uses ES module system to depend on skiko through skiko.mjs.
// Further bundler could process all files by its own (both skiko.mjs and skiko.wasm) and then emits its own version.
// So that’s why we need to provide skiko.mjs and skiko.wasm only for webpack, but not in the final dist.
compilation.binaries.all {
it.linkSyncTask.configure {
it.dependsOn(unpackRuntime)
it.from.from(unpackedRuntimeDir)
}
}
} else {
// Kotlin/JS depends on Skiko through global space.
// Bundler cannot know anything about global externals, so that’s why we need to copy it to final dist
project.tasks.named(compilation.processResourcesTaskName, ProcessResources::class.java) {
it.from(unpackedRuntimeDir)
it.dependsOn(unpackRuntime)
it.exclude("META-INF")
}
}
}
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 leave small explanation on why we are actually doing this?
The other question - any chance those "wasmJs" tasks have some narrower type we can rely on rather then IncrementalSyncTask

Copy link
Member Author

@eymar eymar Jul 23, 2024

Choose a reason for hiding this comment

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

refactored and added the comments. Thanks to @ilgonmic for help with KGP and explanation

}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class GradlePluginTest : GradlePluginTestBase() {
check.taskSuccessful(":compileKotlinJs")
check.taskSuccessful(":compileKotlinWasmJs")
check.taskSuccessful(":wasmJsBrowserDistribution")
check.taskSuccessful(":jsBrowserDistribution")

file("./build/dist/wasmJs/productionExecutable").apply {
checkExists()
Expand All @@ -61,6 +62,14 @@ class GradlePluginTest : GradlePluginTestBase() {
// one file is the app wasm file and another one is skiko wasm file with a mangled name
assertEquals(2, distributionFiles.filter { it.endsWith(".wasm") }.size)
}

file("./build/dist/js/productionExecutable").apply {
checkExists()
assertTrue(isDirectory)
val distributionFiles = listFiles()!!.map { it.name }.toList()
assertTrue(distributionFiles.contains("skiko.wasm"))
assertTrue(distributionFiles.contains("skiko.js"))
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class KotlinCompatibilityTest : GradlePluginTestBase() {
fun testKotlinMpp_1_9_10() = testMpp("1.9.10")

@Test
fun testKotlinJsMpp_1_9_10() = testJsMpp("1.9.10")
fun testKotlinJsMpp_1_9_24() = testJsMpp("1.9.24")

@Test
fun testKotlinMpp_1_9_20() = testMpp("1.9.20")
Expand Down
Loading