-
Notifications
You must be signed in to change notification settings - Fork 331
update: adding a Wasm target to the full stack KMP tutorial #631
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
Conversation
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes introduce support for the WebAssembly JavaScript target in the Kotlin Multiplatform project by updating dependencies and build configurations. New Compose UI implementations for the task manager app are added. The server is updated to support CORS, and documentation is revised for clarity and accuracy, with no changes to public code interfaces. Changes
Suggested reviewers
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 10
🧹 Nitpick comments (1)
codeSnippets/snippets/full-stack-task-manager/composeApp/src/commonMain/kotlin/com/example/ktor/full_stack_task_manager/App.kt (1)
60-63
: Modifier order: callfillMaxSize()
beforesafeContentPadding()
safeContentPadding()
adds insets; placing it first means the column might not actually fill the full size in some layouts.-LazyColumn( - modifier = Modifier - .safeContentPadding() - .fillMaxSize() -) { +LazyColumn( + modifier = Modifier + .fillMaxSize() + .safeContentPadding() +) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
images/full_stack_development_tutorial_create_project.png
is excluded by!**/*.png
images/full_stack_development_tutorial_run_android.png
is excluded by!**/*.png
images/full_stack_development_tutorial_run_web.png
is excluded by!**/*.png
📒 Files selected for processing (9)
codeSnippets/snippets/full-stack-task-manager/composeApp/build.gradle.kts
(1 hunks)codeSnippets/snippets/full-stack-task-manager/composeApp/src/commonMain/kotlin/com/example/ktor/full_stack_task_manager/App.kt
(2 hunks)codeSnippets/snippets/full-stack-task-manager/composeApp/src/commonMain/kotlin/com/example/ktor/full_stack_task_manager/App_first.kt
(1 hunks)codeSnippets/snippets/full-stack-task-manager/composeApp/src/commonMain/kotlin/com/example/ktor/full_stack_task_manager/App_second.kt
(1 hunks)codeSnippets/snippets/full-stack-task-manager/composeApp/src/commonMain/kotlin/com/example/ktor/full_stack_task_manager/network/TaskApi.kt
(1 hunks)codeSnippets/snippets/full-stack-task-manager/gradle/libs.versions.toml
(1 hunks)codeSnippets/snippets/full-stack-task-manager/server/build.gradle.kts
(1 hunks)codeSnippets/snippets/full-stack-task-manager/server/src/main/kotlin/com/example/ktor/full_stack_task_manager/Application.kt
(2 hunks)topics/full-stack-development-with-kotlin-multiplatform.topic
(24 hunks)
🔇 Additional comments (3)
codeSnippets/snippets/full-stack-task-manager/gradle/libs.versions.toml (1)
41-41
:❌ Incorrect review comment
Potentially wrong artifact coordinates for WASM client
For Ktor 3.1.3 the published artifact is
io.ktor:ktor-client-wasm-js
(no extrajs-wasm-js
suffix).
If the coordinates are off,./gradlew :composeApp:wasmJsMainDependenciesMetadata
will fail with “Could not find …”.
Please double-check and adjust:-ktor-client-wasm = { module = "io.ktor:ktor-client-js-wasm-js", version.ref = "ktor"} +ktor-client-wasm = { module = "io.ktor:ktor-client-wasm-js", version.ref = "ktor" }
🏁 Script executed:
#!/usr/bin/env bash echo "io.ktor:ktor-client-js-wasm-js 3.1.3:" $(curl -s "https://search.maven.org/solrsearch/select?q=g:io.ktor%20AND%20a:ktor-client-js-wasm-js%20AND%20v:3.1.3&rows=1&wt=json" | grep -Po '"numFound":\K\d+') echo "io.ktor:ktor-client-wasm-js 3.1.3:" $(curl -s "https://search.maven.org/solrsearch/select?q=g:io.ktor%20AND%20a:ktor-client-wasm-js%20AND%20v:3.1.3&rows=1&wt=json" | grep -Po '"numFound":\K\d+')Length of output: 494
Artifact coordinates are correct as-is
The Maven Central lookup confirms that for Ktor 3.1.3 only
io.ktor:ktor-client-js-wasm-js
exists (numFound=1) whileio.ktor:ktor-client-wasm-js
does not (numFound=0). No change tolibs.versions.toml
is needed.Likely an incorrect or invalid review comment.
codeSnippets/snippets/full-stack-task-manager/composeApp/src/commonMain/kotlin/com/example/ktor/full_stack_task_manager/network/TaskApi.kt (1)
1-1
: Import added – compilation issue resolvedAdding the
Task
import fixes the missing reference; no further issues spotted here.topics/full-stack-development-with-kotlin-multiplatform.topic (1)
151-170
: Doc-only change – looks fine
No technical concerns on the codebase side.
ktor-serialization-kotlinx-json = { module = "io.ktor:ktor-serialization-kotlinx-json", version.ref = "ktor" } | ||
ktor-serialization-kotlinx-json-jvm = { module = "io.ktor:ktor-serialization-kotlinx-json-jvm", version.ref = "ktor" } | ||
ktor-server-content-negotiation-jvm = { module = "io.ktor:ktor-server-content-negotiation-jvm", version.ref = "ktor" } | ||
ktor-server-cors = { module = "io.ktor:ktor-server-cors", version.ref = "ktor" } |
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.
💡 Verification agent
🧩 Analysis chain
Missing -jvm
suffix breaks server build
Other server artefacts (e.g. ktor-server-core-jvm
) carry the -jvm
suffix; the CORS module follows the same pattern.
Using the suffix-less variant will resolve to the common (non-JVM) module and Netty will not see the CORS plugin at runtime.
-ktor-server-cors = { module = "io.ktor:ktor-server-cors", version.ref = "ktor" }
+ktor-server-cors = { module = "io.ktor:ktor-server-cors-jvm", version.ref = "ktor" }
🏁 Script executed:
#!/bin/bash
sed -n '30,60p' codeSnippets/snippets/full-stack-task-manager/gradle/libs.versions.toml
Length of output: 2828
🏁 Script executed:
#!/bin/bash
echo "### Checking availability of io.ktor:ktor-server-cors ###"
curl -s "https://search.maven.org/solrsearch/select?q=g:%22io.ktor%22+AND+a:%22ktor-server-cors%22&rows=5&wt=json" | jq '.response.docs[] | {id: .id, latestVersion: .latestVersion}'
echo
echo "### Checking availability of io.ktor:ktor-server-cors-jvm ###"
curl -s "https://search.maven.org/solrsearch/select?q=g:%22io.ktor%22+AND+a:%22ktor-server-cors-jvm%22&rows=5&wt=json" | jq '.response.docs[] | {id: .id, latestVersion: .latestVersion}'
Length of output: 775
Use the JVM-specific CORS module
The alias in libs.versions.toml
currently points to the common (non-JVM) artifact, so the CORS plugin won’t load at runtime on your server. Update it to the JVM variant:
• Location:
codeSnippets/snippets/full-stack-task-manager/gradle/libs.versions.toml
(around line 45)
• Change needed:
-ktor-server-cors = { module = "io.ktor:ktor-server-cors", version.ref = "ktor" }
+ktor-server-cors = { module = "io.ktor:ktor-server-cors-jvm", version.ref = "ktor" }
Other server modules (core, netty, content-negotiation, test-host) use the -jvm
suffix—CORS should match.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
ktor-server-cors = { module = "io.ktor:ktor-server-cors", version.ref = "ktor" } | |
ktor-server-cors = { module = "io.ktor:ktor-server-cors-jvm", version.ref = "ktor" } |
🤖 Prompt for AI Agents
In codeSnippets/snippets/full-stack-task-manager/gradle/libs.versions.toml at
line 45, the ktor-server-cors module is referenced without the required -jvm
suffix, causing the server to load the common non-JVM artifact and fail to
recognize the CORS plugin at runtime. To fix this, update the module reference
from "io.ktor:ktor-server-cors" to "io.ktor:ktor-server-cors-jvm" so it matches
the JVM-specific suffix used by other server modules.
codeSnippets/snippets/full-stack-task-manager/server/build.gradle.kts
Outdated
Show resolved
Hide resolved
codeSnippets/snippets/full-stack-task-manager/composeApp/build.gradle.kts
Show resolved
Hide resolved
...ask-manager/composeApp/src/commonMain/kotlin/com/example/ktor/full_stack_task_manager/App.kt
Outdated
Show resolved
Hide resolved
...-task-manager/server/src/main/kotlin/com/example/ktor/full_stack_task_manager/Application.kt
Outdated
Show resolved
Hide resolved
val httpClient = createHttpClient() | ||
val taskApi = remember { TaskApi(httpClient) } | ||
val tasks = remember { mutableStateOf(emptyList<Task>()) } | ||
val scope = rememberCoroutineScope() | ||
|
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.
🛠️ Refactor suggestion
Prevent heavy HttpClient
re-creation on every recomposition
createHttpClient()
is called on each recomposition, leaking connections and wasting resources.
- val httpClient = createHttpClient()
+ val httpClient = remember { createHttpClient() }
Add cleanup to avoid leaking resources:
DisposableEffect(Unit) {
onDispose { httpClient.close() }
}
🤖 Prompt for AI Agents
In
codeSnippets/snippets/full-stack-task-manager/composeApp/src/commonMain/kotlin/com/example/ktor/full_stack_task_manager/App_first.kt
around lines 20 to 24, the HttpClient is recreated on every recomposition
causing resource leaks. To fix this, wrap the HttpClient creation in a
DisposableEffect with Unit as the key and call httpClient.close() inside
onDispose to properly clean up resources when the composable leaves the
composition.
...nager/composeApp/src/commonMain/kotlin/com/example/ktor/full_stack_task_manager/App_first.kt
Outdated
Show resolved
Hide resolved
val httpClient = createHttpClient() | ||
val taskApi = remember { TaskApi(httpClient) } | ||
var tasks by remember { mutableStateOf(emptyList<Task>()) } | ||
val scope = rememberCoroutineScope() | ||
|
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.
🛠️ Refactor suggestion
Reuse a single HttpClient
instance
Move creation into remember {}
and dispose it on exit as shown for App_first.kt
. This greatly reduces socket churn.
🤖 Prompt for AI Agents
In
codeSnippets/snippets/full-stack-task-manager/composeApp/src/commonMain/kotlin/com/example/ktor/full_stack_task_manager/App_second.kt
around lines 33 to 37, the HttpClient is created directly each time, causing
unnecessary socket churn. To fix this, wrap the HttpClient creation inside a
remember { } block to ensure a single instance is reused across recompositions,
and add proper disposal of the client when the composable exits, similar to the
pattern used in App_first.kt.
...ager/composeApp/src/commonMain/kotlin/com/example/ktor/full_stack_task_manager/App_second.kt
Outdated
Show resolved
Hide resolved
...ager/composeApp/src/commonMain/kotlin/com/example/ktor/full_stack_task_manager/App_second.kt
Show resolved
Hide resolved
...nager/composeApp/src/commonMain/kotlin/com/example/ktor/full_stack_task_manager/App_first.kt
Show resolved
Hide resolved
...ask-manager/composeApp/src/commonMain/kotlin/com/example/ktor/full_stack_task_manager/App.kt
Show resolved
Hide resolved
codeSnippets/snippets/full-stack-task-manager/composeApp/build.gradle.kts
Outdated
Show resolved
Hide resolved
Co-authored-by: Vik Nikolova <[email protected]>
No description provided.