Kotlin / kotlinx-datetime

KotlinX multiplatform date/time library
Apache License 2.0
2.41k stars 101 forks source link

Make JS TimeZone support easier to integrate #178

Open wkornewald opened 2 years ago

wkornewald commented 2 years ago

Currently, in order to use time zones in JS you have to add an NPM dependency and custom code:

@JsModule("@js-joda/timezone")
@JsNonModule
external object JsJodaTimeZoneModule

private val jsJodaTz = JsJodaTimeZoneModule

Would it be possible to just provide a kotlinx-datetime-timezones module which does both steps automatically, so all you need to do is add the module dependency? Then no extra code would be needed and in theory this module could also provide time zone databases for other platforms where you'd also need extra dependencies and init code.

dkhalanskyjb commented 2 years ago

The JS timezone support does not seem that cumbersome—as you show, it's just four lines, listed in our README.

Providing our own package for this, however, is not as straightforward as it seems. In JS, for example, there's sometimes the need to aggressively reduce the size of the package, given how it is transferred to the browser, so a reduced timezone database may be used: https://github.com/js-joda/js-joda/tree/master/packages/timezone#reducing-js-joda-timezone-file-size We haven't evaluated how much of a problem this is for Kotlin+JS projects, but if it is a problem, we would need to provide reduced-size packages as well. Additionally, the code is not ready for supporting timezone databases loaded from the outside, and we're not sure about the API for choosing which database to use—the system one, if present, or the loaded one, if present, or maybe something else?.. Should we publish a new library release each time a new timezone database is released? If so, we would need some automation for this. Etc.

So, not at all simple. There are more pressing matters, like formatting, adding LocalTime, etc., so spending all the effort just for this doesn't seem optimal.

wkornewald commented 2 years ago

Absolutely, this is just a minor feature request. :)

set321go commented 2 years ago

It would be useful if this information was much clearer in the documentation, I only worked out why my timezone lookups were failing from this ticket

fluidsonic commented 2 years ago

Workaround no longer worked for me in Kotlin 1.6.20 (IR compiler).

This works:

@JsModule("@js-joda/timezone")
@JsNonModule
private external object JsJodaTimeZoneModule

@EagerInitialization
@OptIn(ExperimentalStdlibApi::class)
@Suppress("unused")
private val init = JsJodaTimeZoneModule
dkhalanskyjb commented 2 years ago

Strange, in my toy project where I smoke-test datetime, this is not required, JS-IR + 1.6.20 recognizes the time zones just fine with the old trick, when testing in both Node and the browsers.

fluidsonic commented 2 years ago

Might have been because the respective module is loaded dynamically in my project.

fluidsonic commented 2 years ago

Also, it only happened when deploying the final build including Webpack & terser optimizations. The library was then optimized away.

dkhalanskyjb commented 2 years ago

Does the problem persist if you don't mark the external object as private?

fluidsonic commented 2 years ago

Unfortunately I've got no time for additional tests here

set321go commented 2 years ago

This is how I have it setup (implementation(npm("@js-joda/timezone", "2.12.0")))

package lib.timezone

@JsModule("@js-joda/timezone")
@JsNonModule
external object JsJodaTimeZoneModule

val jsJodaTz = JsJodaTimeZoneModule

The generated js has require('@js-joda/timezone') which then internally is initialized by

  var jsJodaTz;
  var properties_initialized_JodaTimeZone_kt_1382912675;
  function init_properties_JodaTimeZone_kt_4282475903() {
    if (!properties_initialized_JodaTimeZone_kt_1382912675) {
      properties_initialized_JodaTimeZone_kt_1382912675 = true;
      jsJodaTz = JsJodaTimeZoneModule;
    }
  }

I've had problems like this with other libs and I find the best debugging path is to look at the generated code in build/js/packages

brianguertin commented 2 years ago

For me, the instructions in the README are working for a debug build, but not in webpack release build. In the release build only, no time zones can be found.

The workaround that worked for me is to add a reference to the timezone module in the function where I'm accessing it. I suspect the top-level one in the README is not getting loaded in release mode?

val forceModuleImport = JsJodaTimeZoneModule
ZoneId.of(timezone)
mikedawson commented 1 year ago

I experienced the same issue and did some further testing: making the val jsJodaTz public (instead of private) does not fix it. Accessing the property from the main function fixed it - e.g.

fun main() {
    ...
    //Avoid joda time timezones being DCE'd out  in release-
    jsJodaTz
}

@EagerInitialization might be a fix: but I see this is already deprecated. Should this be filed as a case for keeping EagerInitialization?

In the meantime: the README should be updated. I can make a pull request for that if it helps.

zhimbura commented 4 months ago

In my case this changes helps for my js library

@JsModule("@js-joda/timezone")
@JsNonModule
external object JsJodaTimeZoneModule

@JsExport // add export
val jsJodaTz = JsJodaTimeZoneModule // and public
dkhalanskyjb commented 3 months ago

Could someone please provide a project where the code from the README isn't enough? I just tried creating another Kotlin/JS project, and everything works for me without JsExport, making jsJodaTz public, or accessing it in function bodies.