Kotlin / kotlinx-datetime

KotlinX multiplatform date/time library
Apache License 2.0
2.39k stars 99 forks source link

Export types to JavaScript #72

Open Smoofie opened 3 years ago

Smoofie commented 3 years ago

Hello,

Is there any reason, why library classes are not exported to JS using @JsExport? I would love to use kotlinx-datetime in multiplatform library module compiled to JS using recently introduced IR backend. Library is used by Typescript-based project, so without JS exports my compiled classes and corresponding d.ts file are only partially usable.

From Kotlin documentation:

If you are targeting the IR compiler backend, you must use the @JsExport annotation to make your functions visible from Kotlin in the first place.

Is that lack of export a design decision, or a missing feature?

dkhalanskyjb commented 3 years ago

Hi! Exporting library classes to JS is not at all straightforward. @JsExport does not support many of the features that we use in the library, like enum classes or hiding primary constructors in favor of secondary ones; our extensive use of Long, also unsupported by @JsExport, doesn't help as well.

But even more importantly, even if we could do it, we probably would not: exporting every class to be visible from the JS code would negatively affect the users of pure Kotlin/JS, as then the compiler would have a harder time eliminating dead code there. There are plans to mitigate this on the Kotlin/JS side, for example, by using opaque TS types in the exported signatures, in the future.

The recommended way to use @JsExport currently is to do it sparingly, only for calling a couple of specifically selected entry points to the Kotlin code.

Smoofie commented 3 years ago

I do understand now the limitations of @JsExport and using @JsExport on utilities that process kotlinx-datetime classes is indeed a workaround.

However this library has a huge potential to be used in code that is exported to JS - for example DTO classes. If @JsExport evolves in the future, it would be really convenient to have DTO classes with datetime fields fully exported to JS. It would play nicely with kotlinx-serialization library. Currently kotlinx-datetime classes instantiated by deserialization in JS code can't be directly used.

dkhalanskyjb commented 3 years ago

Currently kotlinx-datetime classes instantiated by deserialization in JS code can't be directly used.

Could you clarify this point? Can't the programmer perform deserialization on the Kotlin side of Kotlin + JS?

Whathecode commented 2 years ago

I have a Kotlin multiplatform library which includes JS as a target. As per the use case highlighted by @dkhalanskyjb, the library exposes kotlinx-datetime types in its API, in particular Instant.

Using the LEGACY backend, I wrote a small TypeScript (TS) declaration file to access datetime functionality for clients using the JS target, which included Kotlin-DateTime-library-kotlinx-datetime-js-legacy.js. This way they could pass and read out instances of Instant in the API of my library.

Now trying to upgrade to the IR backend, I can't figure out how to do something similar. In the build files, the Kotlin-DateTime-library-kotlinx-datetime-js-ir folder in packages_imported only contains a package.json file, but no .js file, even though the package.json file refers to it:

{
  "name": "Kotlin-DateTime-library-kotlinx-datetime-js-ir",
  "version": "0.3.1",
  "main": "Kotlin-DateTime-library-kotlinx-datetime-js-ir.js",
  "types": "Kotlin-DateTime-library-kotlinx-datetime-js-ir.d.ts",
  "devDependencies": {},
  "dependencies": {
    "@js-joda/core": "3.2.0"
  },
  "peerDependencies": {},
  "optionalDependencies": {},
  "bundledDependencies": []
}

If I understood correctly, the idea is that dependencies, thus including kotlinx-datetime, are all compiled into one .js file as determined by the main compiled artifact. Dependencies within the project I am compiling are included and exported in the resulting JS package. However, I have no control over whether or not to export dependencies (such as Instant) in the resulting JS package, and, they aren't! As a result, I can no longer use the JS IR compilation target as a JS library the way I did using the LEGACY backend. Similarly, the generated TS declarations refer to kotlinx.datetime types, but those declarations are not specified anywhere.

Am I missing something, or is this a bug?

andylamax commented 2 years ago

@JsExport in IR has received so much love in kotlin 1.6.20. Will we be getting these fields out now?

andylamax commented 2 years ago

Exporting library classes to JS is not at all straightforward. @JsExport does not support many of the features that we use in the library, like enum classes or hiding primary constructors in favor of secondary ones; our extensive use of Long, also unsupported by @JsExport, doesn't help as well.

Alot of arguments here are valid, but I think there is a very acceptable way to solve this issue and here are my suggestions. First of all, we don't need to mark every definition with @JsExport, just a few important (and exportable) ones

  1. All enums (i.e. DayOfWeek and Month) can be marked as @JsExport as enums are now supported by @JsExport
  2. After marking all enums, you'll find marking the LocalDate class with @JsExport easy as (now) every member it has is exportable
  3. With LocalDate exportable, the next candidate would be LocalDateTime as every member (now including LocalDate) is also exportable
  4. The Instant class would be a little bit tricky because of the val epochSeconds: Long member, but an easy mitigation can be done here before marking it with @JsExport,
    • We can add an extra getter in the lines of val epochSecondsAsDouble: Double get() = epochSeconds.toDouble()
    • The we can mark it with @JsExport and typescript will have types in the lines of
      class Instant {
      // . . .
      get epochSeconds(): any
      get epochSecondsAsDouble(): number
      }

      Which is a fair compromise (without breaking behaviour) if you ask me

  5. classes TimeZone,UtcOffset,FixedOffsetTimezone,ZoneOffset, are already exportable (and can be marked as such) coz all of their members are exportable

With all of that, I believe this library would be highly usable from Javascript/Typescript

nmammeri commented 2 years ago

We have managed to use this library from Javascript/Typescript by creating a wrapper class annotated with @JSEXPORT. This is what we use to wrap around LocalDateTime:

@JsExport
class KMPLocalDateTime(
    val year: Int,
    val monthNumber: Int,
    val dayOfMonth: Int,
    val hour: Int,
    val minute: Int,
    val second: Int,
) {

    private val mDateTime = LocalDateTime(
        year = year,
        monthNumber =monthNumber,
        dayOfMonth = dayOfMonth,
        hour = hour,
        minute = minute,
        second = second,
    )

    override fun toString(): String {
        return mDateTime.toString()
    }

    fun isEqual(other: KMPLocalDateTime): Boolean {
        return (year == other.year && monthNumber == other.monthNumber &&
                dayOfMonth == other.dayOfMonth && hour == other.hour &&
                minute == other.minute && second == other.second)
    }
}

With all the JSEXPORT problems mentioned by @andylamax, I personally think a quick workaround would be that as part of kotlinx-datetime library we provide wrapper classes annotated with @JSEXPORT for Instant, LocalDateTime and LocalDate.

Whathecode commented 2 years ago

My current workaround, similar to what I did on the legacy backend, is to write custom TypeScript declarations for the types which are exported through $crossModule$ (more information in this JS backend feature request). The following is all I have for now (based on my current API requirements):

declare module 'Kotlin-DateTime-library-kotlinx-datetime-js-ir'
{
    interface System
    {
        now_0_k$(): $crossModule$.Instant
    }

    namespace $crossModule$
    {
        abstract class Instant { }
        function System_getInstance(): System
        function InstantIso8601Serializer_getInstance(): any
    }
}

I then tried to clean up the API a bit by using the following wrapper which re-exports types in matching namespaces:

tjerkw commented 6 months ago

Not having Instant to be @JsExport-export-table heavily limits on what code you can re-use in typescript.

For example, we have some DTO classes that consists of primives and enums.... And instants... However, i cant expose them to typescript because of the instant, and i have to create a wrapper class, or mark the Instant as either JsReference<Instant> or as @JsExport.Ignore

All in all its quite limiting.. I agree that maybe at least Instant and LocalDateTime should be marked as @JsExport Or at least have an option to mark them as such.

lppedd commented 6 months ago

Marking any library class is a no-go in the actual state.

As highlighted above, what will happen is every new compilation for every consumer of the library will increase bundle size (DCE is not performed anymore on those code paths) and generate additional TypeScript declarations, if the option is on.

This would be a totally unexpected behavior.

tjerkw commented 6 months ago

So the only solution is to create a custom interface/class CustomInstant and have a wrapper around Instant?

I guess thats do-able, but not ideal.

lppedd commented 6 months ago

Yes, as of now. You can play with inline member functions if you want to eliminate the additional call stack depth (check if it works tho).

The way JsExport works is perfectly fine, as it minimizes the JS API surface. But we need more control over what is exported from third party libraries, e.g., specify export patterns via Gradle DSL. There should be an issue in YT for it.