Support KMP js #63
Support KMP js #63
Conversation
WalkthroughThe pull request adds JavaScript target support to the Kotlin Multiplatform build configuration and provides a platform-specific implementation of Changes
Sequence DiagramsequenceDiagram
actor App as App/Runtime
participant Logger as DefaultRecompositionLogger.js
participant Console as Browser Console
App->>Logger: log(recompositionEvent)
rect rgb(200, 220, 255)
Note over Logger: Format header with count & composable name
Logger->>Console: console.log(header)
end
rect rgb(200, 220, 255)
loop For each parameter change
alt changed
Logger->>Logger: safeToString(oldValue)
Logger->>Logger: safeToString(newValue)
else stable/unstable
Logger->>Logger: safeToString(newValue)
end
Logger->>Console: console.log(formatted change line)
end
end
opt If unstable parameters exist
rect rgb(220, 200, 220)
Logger->>Console: console.log(unstable summary)
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
kotlin-js-store/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
stability-runtime/build.gradle.kts(2 hunks)stability-runtime/src/jsMain/kotlin/com/skydoves/compose/stability/runtime/DefaultRecompositionLogger.js.kt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Spotless check
- GitHub Check: Build and Tests
- GitHub Check: API check
- GitHub Check: Gradle Plugin Tests
- GitHub Check: Lint Module Tests
- GitHub Check: Compiler Plugin Tests
- GitHub Check: Runtime Module Tests
- GitHub Check: Compiler Tests (FIR/IR Dumps)
🔇 Additional comments (5)
stability-runtime/build.gradle.kts (2)
35-38: LGTM! JS target configuration is correct.The JS target is properly configured for both browser and Node.js environments, following standard Kotlin Multiplatform conventions.
49-49: LGTM! JS properly included in hierarchy.The
withJs()call correctly includes the JS target in the common hierarchy template, ensuring the JS platform has access to shared code.stability-runtime/src/jsMain/kotlin/com/skydoves/compose/stability/runtime/DefaultRecompositionLogger.js.kt (3)
30-31: LGTM! Proper expect/actual implementation.The class declaration correctly implements the
actualpattern for the JS target.
34-36: LGTM! Header logging is clean.The header format and use of
console.logis appropriate for the JavaScript environment.
67-76: LGTM! Robust error handling.The
safeToStringhelper provides good defensive coding with appropriate fallback for anytoString()failures.
| event.parameterChanges.forEachIndexed { index, change -> | ||
| val isLast = index == event.parameterChanges.size - 1 | ||
| val prefix = if (isLast) " └─" else " ├─" | ||
|
|
||
| val status = when { | ||
| change.changed -> { | ||
| val oldStr = safeToString(change.oldValue) | ||
| val newStr = safeToString(change.newValue) | ||
| "changed ($oldStr → $newStr)" | ||
| } | ||
|
|
||
| change.stable -> "stable (${safeToString(change.newValue)})" | ||
| else -> "unstable (${safeToString(change.newValue)})" | ||
| } | ||
|
|
||
| console.log("$prefix ${change.name}: ${change.type} $status") | ||
| } | ||
|
|
||
| // Log unstable parameters summary | ||
| if (event.unstableParameters.isNotEmpty()) { | ||
| console.log(" └─ Unstable parameters: ${event.unstableParameters}") | ||
| } |
There was a problem hiding this comment.
Fix tree structure when unstable parameters are present.
The tree prefix logic doesn't account for the unstable parameters line. When unstableParameters is not empty, the last parameter change incorrectly uses "└─" (line 41), but then the unstable parameters line (line 59) also uses "└─", creating two terminal branches. This contradicts the example in your documentation (lines 23-27), which correctly shows the last parameter using "├─" when followed by the unstable line.
Apply this diff to fix the tree structure:
// Log parameter changes
event.parameterChanges.forEachIndexed { index, change ->
- val isLast = index == event.parameterChanges.size - 1
+ val isLast = index == event.parameterChanges.size - 1 && event.unstableParameters.isEmpty()
val prefix = if (isLast) " └─" else " ├─"📝 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.
| event.parameterChanges.forEachIndexed { index, change -> | |
| val isLast = index == event.parameterChanges.size - 1 | |
| val prefix = if (isLast) " └─" else " ├─" | |
| val status = when { | |
| change.changed -> { | |
| val oldStr = safeToString(change.oldValue) | |
| val newStr = safeToString(change.newValue) | |
| "changed ($oldStr → $newStr)" | |
| } | |
| change.stable -> "stable (${safeToString(change.newValue)})" | |
| else -> "unstable (${safeToString(change.newValue)})" | |
| } | |
| console.log("$prefix ${change.name}: ${change.type} $status") | |
| } | |
| // Log unstable parameters summary | |
| if (event.unstableParameters.isNotEmpty()) { | |
| console.log(" └─ Unstable parameters: ${event.unstableParameters}") | |
| } | |
| event.parameterChanges.forEachIndexed { index, change -> | |
| val isLast = index == event.parameterChanges.size - 1 && event.unstableParameters.isEmpty() | |
| val prefix = if (isLast) " └─" else " ├─" | |
| val status = when { | |
| change.changed -> { | |
| val oldStr = safeToString(change.oldValue) | |
| val newStr = safeToString(change.newValue) | |
| "changed ($oldStr → $newStr)" | |
| } | |
| change.stable -> "stable (${safeToString(change.newValue)})" | |
| else -> "unstable (${safeToString(change.newValue)})" | |
| } | |
| console.log("$prefix ${change.name}: ${change.type} $status") | |
| } | |
| // Log unstable parameters summary | |
| if (event.unstableParameters.isNotEmpty()) { | |
| console.log(" └─ Unstable parameters: ${event.unstableParameters}") | |
| } |
🤖 Prompt for AI Agents
In
stability-runtime/src/jsMain/kotlin/com/skydoves/compose/stability/runtime/DefaultRecompositionLogger.js.kt
around lines 39 to 60, the tree prefix for the last parameter change doesn't
consider the unstableParameters line, causing two "└─" terminal branches; update
the logic that sets isLast so it only returns true when this parameter is the
final line overall (i.e., index == event.parameterChanges.size - 1 AND
event.unstableParameters is empty), otherwise treat it as non-terminal so the
last parameter uses "├─" when an unstableParameters summary will follow.
Support KMP js.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.