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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
10e6df9
Fix skiko.js unpacking for k/js target
eymar Jul 22, 2024
b32c6ba
refactoring
eymar Jul 23, 2024
213041b
fix a test
eymar Jul 23, 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
12 changes: 11 additions & 1 deletion ...ins/compose/src/main/kotlin/org/jetbrains/compose/web/internal/configureWebApplication.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ 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
Expand Down Expand Up @@ -77,8 +79,16 @@ internal fun configureWebApplication(
}

project.tasks.withType(IncrementalSyncTask::class.java) {
if (it.name.contains("wasmJs", ignoreCase = true)) {
it.dependsOn(unpackRuntime)
it.from.from(unpackedRuntimeDir)
}
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 •
edited
Loading

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

}

project.tasks.named("jsProcessResources", ProcessResources::class.java) {
it.from(unpackedRuntimeDir)
it.dependsOn(unpackRuntime)
it.from.from(unpackedRuntimeDir)
it.exclude("META-INF")
}
}

Expand Down
9 changes: 9 additions & 0 deletions .../compose/src/test/kotlin/org/jetbrains/compose/test/tests/integration/GradlePluginTest.kt
View file
Open in desktop
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
Loading