Skip to content

Conversation

@bashor
Copy link

@bashor bashor commented Jan 29, 2026

Subsystem
ktor-client-core for wasm-js target

Motivation
With the current implementation, when they are generated into a JS file, the resulting code is syntactically incorrect, so webpack and JS VMs reject such code even if it's unreachable.

It is mostly unobservable since the declarations are not used inside ktor, and the kotlin toolchain doesn't generate helper functions for them by default. But it breaks projects using ktor when we try to build them with wip per-module compilation.

Solution
Fix incorrect JS code.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 29, 2026

Walkthrough

Visibility of four internal helper functions reduced to private in a Kotlin/WASM JavaScript interop utility file. JavaScript interop code simplified by removing unnecessary return statements and semicolons. Public APIs remain unchanged.

Changes

Cohort / File(s) Summary
Visibility Reduction
ktor-client/ktor-client-core/wasmJs/src/io/ktor/client/fetch/LibEs5.kt
Four internal helper functions (getMethodImplForUint8Array, setMethodImplForUint8Array, getMethodImplForArrayLike, setMethodImplForArrayLike) marked as private. JavaScript interop calls simplified by removing unnecessary return statements and semicolons from JS code strings.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix get and set implementations for Uint8Array and ArrayLike' clearly and specifically describes the main changes made in this pull request: fixing implementations for array access methods.
Description check ✅ Passed The pull request description provides clear subsystem identification, motivation grounded in a real compilation issue, and describes the solution. All required template sections are present and adequately filled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

🧪 Unit Test Generation v2 is now available!

We have significantly improved our unit test generation capabilities.

To enable: Add this to your .coderabbit.yaml configuration:

reviews:
  finishing_touches:
    unit_tests:
      enabled: true

Try it out by using the @coderabbitai generate unit tests command on your code files or under ✨ Finishing Touches on the walkthrough!

Have feedback? Share your thoughts on our Discord thread!


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@bashor
Copy link
Author

bashor commented Jan 29, 2026

It would also be nice to have the fix in some upcoming (fix?) releases.

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.

1 participant