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

Add very basic Kotlin/JS support: ability to compile the binary #3678

Merged
merged 5 commits into from
Oct 7, 2024

Conversation

0xnm
Copy link
Contributor

@0xnm 0xnm commented Oct 5, 2024

Another part for #3611.

This PR adds a very basic foundation for the Kotlin/JS support. The code provided allows to:

  • Build code with org.jetbrains (most likely) dependencies only
  • Run it with Node (no browser support)
  • Execute tests (using kotlinx-test so far), but without any test results collection/tests selector, etc.

However, I think that full Kotlin/JS support will take a lot of time, because even if there is a Scala.JS counterpart available, the way these technologies work is very different (distribution, format, etc.)

Issues I've encountered:

  • Kotlin 2+ is not supported, because with Kotlin 2+ jars are not published anymore and there is only klib file available (see https://repo1.maven.org/maven2/org/jetbrains/kotlin/kotlin-stdlib-js/2.0.0/). When Gradle is used, it is able to fetch it using attributes declared in .module file, but Coursier is not able to recognize and fetch it.
  • Kotlin versions below 1.8.20 are not supported, because of the different set of compiler arguments. With the certain effort it is possible to go further in supporting older versions, but I'm not sure if it is needed: since Mill is somewhat experimental, probably there is no need for the users to use old Kotlin versions.
  • Even if some Kotlin/JS library has jar file published with Kotlin/JS library inside, it may be rejected by the compiler. For example, this https://repo1.maven.org/maven2/org/jetbrains/kotlinx/kotlinx-html-js/0.8.0/ has the necessary jar file with .meta.js / .kjsm files inside, but it is rejected by the compiler of Kotlin 1.8/1.9. Here https://repo1.maven.org/maven2/org/jetbrains/kotlin/kotlin-stdlib-js/1.9.24/, for example, nothing is rejected, so I suppose there is an issue in the ABI/metadata version. Not sure how it can be solved (maybe by relying only on klib? But klib cannot be fetched by Coursier).
  • Gradle Kotlin plugin is utilizing NPM dependencies to generate package.json and add the necessary JS test frameworks/runners there if executed in Node environment, or even webpack for Browser environment. This is also a big chunk of work to be done. For now I've added only test binary execution, but there is no test results collection / test selector.
  • Kotest cannot be used, because with version 5 only klib is published, and jar of version 4 is not compatible with the 1.8/1.9 IR compiler.
  • Kotlin/JS IR compiler has different modes: it can output either IR/Klib or can produce final JS (basically IR+linking). Kotlin Gradle plugin is using 2 passes: to generate IR and then produce final JS. I guess it is done for the better performance / better incremental support, but I, for the initial drop, rely on a single pass (IR+linking in a single compiler invocation) => need to make compile task to produce only IR code and add kind of link task to produce executable in the future. => addressed in the 2nd commit.

@0xnm 0xnm force-pushed the add-kotlin-js-support branch 3 times, most recently from 4eb7ebd to 26022df Compare October 5, 2024 18:48
@0xnm 0xnm force-pushed the add-kotlin-js-support branch from 26022df to bc5db4f Compare October 5, 2024 19:37
@lihaoyi
Copy link
Member

lihaoyi commented Oct 6, 2024

Thanks for the thorough investigation @0xnm.

Regarding Kotlin 1.8 vs 1.9 vs 2.0, what do we currently support for KotlinModule on the JVM? All of those versions, or only specific version ranges?

I think you're right that a full implementation of the Kotlin-JS will take a lot of time, more than I expected. Perhaps the three big challenges will be handling the .klib files, reproducing any Gradle-specific components like the test runners, and the generally instability of the Kotlin-2.0/Kotlin-Multi-Platform APIs involved.

Given that, I think the right thing to do is as follows:

  1. We review and merge this PR as an experimental proof-of-concept integration of Kotlin-JS, limitations and all. We can tag it clearly as experimental in the docs as well as via an @experimental annotation so it is not subject to bincompat enforcement, so anyone interested can play with it but will know it's not a production-ready integration, and we can document the limitations clearly. Will still be good to have such that anyone who wants to take it further has a good place to start from.

  2. We proceed with the other bullets in Add Kotlin Web (Ktor & KotlinJS) examples (1650USD Bounty) #3611. It looks like most of those should be doable despite the limitations you mentioned, since it's mostly just wiring shared sources into multiple JVM/JS kotlin modules and then wiring the compiled JVM classpaths and JS javascript files back together, and won't need any third party libraries. Like (1) above, this would all be pretty experimental, but still good to have as a basis for future work. This would involve skipping newer KMP features and other stuff that only exists in kotlin 2.0 (due to the limitations above) and doing it the old way of having a pair of JVM/JS modules share sources that get compiled twice

  3. We can continue to look into the details of the broader Kotlin ecosystem to see how we can support it. @alexarchambault may be able to help with Coursier changes. Mid term, I'd like to support Kotlin-Android (Add hello-world Android Java & Kotlin examples using Mill (2000USD bounty) #3550), and longer term Kotlin Multi-Platform, so we will need to dig into these quirks of the Kotlin ecosystem sooner or later. Even if it ends up taking a lot more time and effort, I would be willing to fund it.

  4. We can hold off on trying to reach full feature-parity for Kotlin-JS on Mill v.s. Gradle for now: e.g. setting up webpack, browser integration testing, JS test runners, and so on. Once the above examples are done, we can see whether there is interest in a more robust integration

import java.util.zip.ZipFile

/**
* This module is very experimental. Don't use it, it is still under the development, APIs can change.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any API reference or example code you used to find all the different configuration keys that KotlinJS exposes, and you had to translate to Mill here? If so we should put it in the scaladoc so future readers can see where all this stuff came from

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is where I would avoid adding any documentation for now, because this Kotlin/JS module is not ready for being used at all, so it shouldn't be in the docs/example. The first blocker is inability to use .klib instead of .jar, the second one is API: it is subject to change anyway, and I took an inspiration from ScalaJSModule, but API there looks strange - some methods have scalajs prefix, some don't, etc.

The point of this PR is just to add the first building block to the Kotlin/JS support.

Comment on lines +379 to +388
private def moduleName() = millModuleSegments.value
.filter(_.isInstanceOf[Segment.Label])
.map(_.asInstanceOf[Segment.Label])
.last
.value

private def fullModuleName() = millModuleSegments.value
.filter(_.isInstanceOf[Segment.Label])
.map(_.asInstanceOf[Segment.Label].value)
.mkString("-")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where do these two names end up being used? If it's just for the compiled .js file, I wonder if we can get away with just providing a hardcoded name (IIRC ScalaJS just hardcodes it as main.js) and leaving the user to override and rename the file themselves if they need to

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess here comes the difference between the binary output of Scala.JS and Kotlin/JS: Kotlin/JS is able to split final .js files per module by using the following flag, it can be seen here.

I guess the benefit of doing so lays mainly on the browser side, where such .js files can be loaded by the client in parallel.

Considering that, it is not possible to hardcode the default name (and I would avoid doing so), having the output matching the module name is more meaningful.

@lihaoyi
Copy link
Member

lihaoyi commented Oct 6, 2024

The implementation and unit tests look pretty good. Just need an example test so we have something we can include in the docs

@0xnm
Copy link
Contributor Author

0xnm commented Oct 6, 2024

Regarding Kotlin 1.8 vs 1.9 vs 2.0, what do we currently support for KotlinModule on the JVM? All of those versions, or only specific version ranges?

No, for JVM target there is no restriction, because it is pretty established ecosystem of distributing .class files there (with Kotlin Metadata annotation on top). The problem comes from Kotlin/JS only: I believe they switched from legacy compiler to IR compiler at some point, so .jar files produced by the old one are rejected by the new one, but not KLIBs (which are coming with IR only, it seems). I think it is briefly mentioned here (but I'm not sure if it is the same). One klib support is there, I guess this problem should go away even for the artifacts produced with the older Kotlin version (also publication may have klib and jar at the same time, in this case klib should be preferred).

Perhaps the three big challenges will be handling the .klib files, reproducing any Gradle-specific components like the test runners, and the generally instability of the Kotlin-2.0/Kotlin-Multi-Platform APIs involved.

Yes, more or less. Although once .klib fetch support is added, Kotlin 2+ support comes automatically (I believe only klib is a blocker there). I'm not sure about the Kotlin Multiplatform in general, Kotlin/Native may bring another challenge. And anyway, fetching klib is only the tip of the iceberg, ideally it is worth to support .module files parsing instead of trying to fetch klib blindly.

For the test runners, yes - it is another blocker even for web[3-kotlinjs-module], because need to implement the runner (supporting test selectors) and test reports collection/reporting.

We review and merge this PR as an experimental proof-of-concept integration of Kotlin-JS, limitations and all. We can tag it clearly as experimental in the docs as well as via an @experimental annotation so it is not subject to bincompat enforcement, so anyone interested can play with it but will know it's not a production-ready integration, and we can document the limitations clearly. Will still be good to have such that anyone who wants to take it further has a good place to start from.

Documentation can be added (but not something in example folder, because of the blockers), but imo with blockers it is pretty much useless even for the curious users (although they may report issues for the parts which are already working).

It looks like most of those should be doable despite the limitations you mentioned, since it's mostly just wiring shared sources into multiple JVM/JS kotlin modules and then wiring the compiled JVM classpaths and JS javascript files back together, and won't need any third party libraries

Well, if not only 1:1, because full tests support is not there, klib dependency management is not there, browser support is not there as well (this PR only supports Node).

Mid term, I'd like to support Kotlin-Android

By the way, Android libraries are published as aar archives, not as jar (simple example). I believe it will be the same problem as fetching klib (although it is easier there, because no .module file is probably involved)?

@lihaoyi lihaoyi force-pushed the add-kotlin-js-support branch from 6a781e5 to b3b1780 Compare October 7, 2024 13:56
@lihaoyi
Copy link
Member

lihaoyi commented Oct 7, 2024

@0xnm I pushed a simple example/kotlinlib/web/3-hello-kotlinjs/ to the PR based on the work you've already done. It's definitely pretty skeletal - it doesn't contain the third party Scalatags library that the equivalent Scala example has - but it works well enough I think for a basic demo.

Going further to the next bullet example/kotlinlib/web/4-webapp-kotlinjs would require org.jetbrains.kotlinx:kotlinx-browser:0.1, which I believe may require coursier changes. My current plan is to merge this initial implementation/example, clearly documenting it as experimental/WIP/demo-quality, and then putting the last 3 bullets on hold until we get the necessary KMP/Coursier stuff working

@0xnm
Copy link
Contributor Author

0xnm commented Oct 7, 2024

Thanks for adding the docs/simple example. Indeed, such plan makes sense.

@lihaoyi lihaoyi merged commit ebee7a0 into com-lihaoyi:main Oct 7, 2024
24 checks passed
@lihaoyi
Copy link
Member

lihaoyi commented Oct 7, 2024

@0xnm I'll split that section of the bounty in half and pass you the half for the first bullet just to close that out, since it may be a while before we can proceed with the subsequent bullets

@lefou lefou added this to the 0.12.0 milestone Oct 7, 2024
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.

3 participants