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

[WIP] JS Support #43

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
Draft

Conversation

ankushg
Copy link
Contributor

@ankushg ankushg commented Feb 17, 2022

Work toward #17

Let's get JS support working in here!

To Extract into Separate PRs

Open Questions

  • do we need to add @JsExport? or are the typealiases fine if we just @JsExport the entire class in commonMain? Though we get warnings for this if it has a public suspend fun…
  • should the defaultCoroutineScope be different on JS than Native?

Done in this PR (will be separated into separate PRs)

Main Code

  • create supportedTarget as a sourceset between common and apple/js
  • created a cross-platform expect fun for freeze()
  • created PlatformError as an expect class
    • exposed as NSError on Apple and Throwable on JS
    • create an internal kotlinCause extension that we that we can use instead of digging into userInfo so the tests work on non-Apple platforms

Tests

  • switch from Kotlin/Native AtomicInt to atomicfu AtomicInt (extracted to this PR: )
  • copied tests from appleTest into supportedTargetTest
    • convert from fun `test method with spaces()` to fun test_method_with_underscores() because JS doesn't support spaces (even with backticks)
    • removed freezing-related tests from supportedTargetTest

Remaining

  • figure out how to properly migrate from runBlocking to runTest (from kotlinx-coroutines-test)
    • Tests in supportedTargetTest are passing in JS
    • Tests in supportedTargetTest are currently getting InvalidMutabilityExceptions on Native
    • delete overlapping tests from AppleNative____ tests and only leave freezing ones in there
  • update KSP to generate the additional methods on the JS platform

@rickclephas
Copy link
Owner

An InvalidMutabilityException seems to prevent the conversion to runTest.
Opened an issue: Kotlin/kotlinx.coroutines#3195.

@rickclephas
Copy link
Owner

some sort of test case of consuming this code?

We should probably add a react client to the sample as discussed in #17 (comment).

do we need to add @JsExport? or are the typealiases fine if we just @JsExport the class?

Not 100% sure, but I think we won't need the @JsExport in the library itself.
We'll definitely need to add those to the generated functions though.

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.

None yet

2 participants