[WIP] Time: Add access to JS-Joda Locales

florian-h05 commented 2 years ago

Should fix #54.

Access to some JS-Joda locales is possible using time.Locale.locale-id, e.g. time.locale.ENGLISH. To make this possible, the required packages from JS-Joda have to be installed, so I have added them to the dependencies.

I decided to only include the english (en, en-*), german (de, de-DE) and french (fr, fr-FR) Locales to find a compromise between bundle (webpack) size and functionality. (I decided based on where I think most openHAB users come from.)

I added a README section for how to install additional Locales.

@rkoshak I got your code from the issue working:

var formatDT = function(dt) {
  //var DateTimeFormatter = Java.type("java.time.format.DateTimeFormatter");
  var formatter = time.DateTimeFormatter.ofPattern("h:mm a, ccc MMM dd").withLocale(time.Locale.US);
  return dt.format(formatter);
}

Signed-off-by: Florian Hotze florianh_dev@icloud.com

digitaldan commented 2 years ago

Do you know how much this increases the size of the bundle? If you run npm run webpack can you see how big the file is ?

florian-h05 commented 2 years ago

I just tried it out, the bundle (using wepack) is about 24MB large on this branch. The webpacked bundle of the main branch is just 808KB in size. That's much larger, I was not aware of webpack.

What if we decide only to include some locales and remove some locales that are not so common used in openHAB? (I would start removing Chinese, Japanes, Russian, ...)

digitaldan commented 2 years ago

Thanks for testing that out. I was also looking at this a few weeks ago but got distracted. I think pulling out languages will help, i also found a project (which i need to find again) which pruned out a lot of unnecessary files in each language module that are included from some upstream library, this had the possibility of reducing languages dramatically. I'll try and google that again.

florian-h05 commented 2 years ago

Okay, then I'll have a look at pulling out languages later.

florian-h05 commented 2 years ago

@digitaldan

I removed most Locales and decided to only keep the following Locales:

en, en-**
de, de-DE
fr, fr-FR

I decided for those Locales as in my experience openHAB contributors and users usually come from the english, german or french speaking countries.

florian-h05 commented 2 years ago

Bundle size with webpack now is 11 MB, what is still large but I think it is a good compromise. Removing either German (wouldn't be cool as I come from Germany ;-)) or French would only save about 1MB each.

florian-h05 commented 2 years ago

@digitaldan Merge conflicts resolved.

What do you think of this PR?

digitaldan commented 2 years ago

800KB -> 11MB is still a pretty big increase. My concern here is that the whole library gets loaded into memory every-time a script is run, i don't know if that will have a performance issue on more resource constraint systems like a Raspberry Pi.

I was hoping we could somehow prune the cldr data that i believe is most of the bloat, https://github.com/rxaviers/cldr-data-npm , but looking again, this does not look easy

Thinking about other options:

could we load the locale dynamically based on the user's system setting, possibly making it configurable?
Try and use openHAB's built in locale settings?

Just to be clear, #45 gives the example of time.DateTimeFormatter.ofPattern needing locale data, what other use cases are there? I ask b/c if there are only a few, we might be able to proxy those requests to OH native formatters and return a js-joda object.

@jpg0 do you have any thoughts on this?

digitaldan commented 2 years ago

FYI i'm also reading https://github.com/js-joda/js-joda/blob/master/packages/locale/README.md#packaging-with-webpack-minimizing-package-size to see if there are other options.

florian-h05 commented 2 years ago

800KB -> 11MB is still a pretty big increase. My concern here is that the whole library gets loaded into memory every-time a script is run, i don't know if that will have a performance issue on more resource constraint systems like a Raspberry Pi.

That's a good point, I do not know how it would behave on Raspberry Pi as my development openHAB instance is on my Synology. And my production system on Raspberry Pi is production and not testing...

Thinking about other options:

could we load the locale dynamically based on the user's system setting, possibly making it configurable?

Try and use openHAB's built in locale settings?

I would add:

Switching from en and en-* locales to en and en-US (removing en-GB ...) would reduce webpacked size to 2MB.
Including no locales at all but documenting how to use locales in the README.

rkoshak commented 2 years ago

Just to be clear, https://github.com/openhab/openhab-js/issues/45 gives the example of time.DateTimeFormatter.ofPattern needing locale data, what other use cases are there?

Based on https://js-joda.github.io/js-joda/manual/formatting.html I think

ofPattern
parse
any use of DateTimeFormatter that includes localized entries (e.g e)
LocalDate (I think)

jpg0 commented 2 years ago

@digitaldan I tend to agree that including locale information here increases the size dangerously.

It's unfortunate that the size increase is so large given the relatively small increase in functionality. My view is that we should:

Attempt to handle the vast majority use-cases where they apply (or can be made to apply) the same way - if this isn't possible reasonably in js-joda, then I would suggest we monkey-patch or add some utility methods in the time package.
When a user hits the 90% case, give instructions on how to either work around it or install the locale themselves (which may require invoking npm)

Another thing that I do believe is worth investigating before we do this however, is leaning on the JVM APIs. Js-joda is roughly mirrors the functionality there, so it may be possible to call into JVM classes to convert locale strings to a universal locale that js-joda can handle.

digitaldan commented 2 years ago

Js-joda is roughly mirrors the functionality there, so it may be possible to call into JVM classes to convert locale strings to a universal locale that js-joda can handle.

That was my thought as well. I started looking at this last weekend, but did not get far. I was going to see if we could emulate https://github.com/js-joda/js-joda/blob/master/packages/locale/src/plug.js and plugin in our own formatter.
This would require us to provide the methods found in https://github.com/js-joda/js-joda/blob/master/packages/locale/src/format/LocaleDateTimeFormatter.js and in https://github.com/js-joda/js-joda/blob/master/packages/locale/src/format/cldr/CldrDateTimeFormatterBuilder.js . Any thoughts on that approach ?

digitaldan commented 2 years ago

FYI i decided to profile this to see if the extra size would make any noticeable difference. I created a simple rule in a file like this:

scriptLoaded = function () {
    console.log("script loaded");
}

scriptUnloaded = function () {
    console.log("script unloaded");
}

Then copied it 20 times

for i in `seq 1 20`; do cp test.js mem_test_${i}.js;done

So i had 20 mem_test_x.js files to simulate loading the library in a realistic openHAB setup.

Here is the memory stats without the locale packages loaded, the first images shows memory before the files are loaded, the second shows the loaded memory. Those step increases are the various files loading.

And here is the memory with the local packages loaded

So without locales, the system is using .83 GB of memory, with locales its 2.72GB , so unfortunately a lot. I was hoping it would not be so bad as polyfilling with openHAB locales is not an easy route. Another option is to try and load these lazily, so unless a user specifically calls time.Locale.FRANCE it would not load all the other stuff, this would require some webpack magic but might be doable (have not thought this all the way through)

florian-h05 commented 2 years ago

Another option is to try and load these lazily, so unless a user specifically calls time.Locale.FRANCE it would not load all the other stuff, this would require some webpack magic but might be doable

Does the use of webpack for lazy loading break the installation of the library with npm?

I was hoping it would not be so bad as polyfilling with openHAB locales is not an easy route.

I had a look at js-joda and I can only find one method in the docs that needs a Locale: DateTimeFormatter.withLocale(Locale).

I confirmed the following script to work:

var Locale = Java.type("java.util.Locale").US;
var formatDT = function(dt) {
  var DateTimeFormatter = Java.type("java.time.format.DateTimeFormatter");
  var formatter = DateTimeFormatter.ofPattern("h:mm a, ccc MMM dd").withLocale(Locale);
  return dt.format(formatter);
}

var ZonedDateTime = Java.type("java.time.ZonedDateTime");
console.log(formatDT(ZonedDateTime.now()));

Therefore polyfilling DateTimeFormatter with the Java version might be an option?

florian-h05 commented 2 years ago

When I use the JS-Joda ZonedDateTime in my script instead of the Java one, I get the following error:

2022-03-19 23:39:30.245 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'time-locale' failed: java.lang.Exception: java.lang.StackOverflowError

digitaldan commented 2 years ago

var formatter = time.DateTimeFormatter.ofPattern("h:mm a, ccc MMM dd").withLocale(Locale);

hmm, I would not think passing a "java.util.Locale" into js-joda will work as its a native object with different signature methods. I'm a little confused with your suggestion on how that would work ?

florian-h05 commented 2 years ago

Oh, I forgot to the remove the time. in front of it for this example. This script is from my test machine where I have already monkey-patched the DateTimeFormatter with the Java one.

The monkey-patch in time.js:

time.DateTimeFormatter = Java.type("java.time.format.DateTimeFormatter");

I have corrected my script.

digitaldan commented 2 years ago

Right, I think i'm suggesting something similar in my comment https://github.com/openhab/openhab-js/pull/94#issuecomment-1059827153 , where we provide our own LocaleDateTimeFormatter , its probably doable, its just very unfamiliar to me at least.

digitaldan commented 2 years ago

time.DateTimeFormatter = Java.type("java.time.format.DateTimeFormatter");

Sorry if i'm missing something, but how would the Java formatter work here with JS-Joda? I would not think they are compatible?

florian-h05 commented 2 years ago

Sorry if i'm missing something, but how would the Java formatter work here with JS-Joda? I would not think they are compatible?

I was thinking of replacing the JS-Joda formatter with the Java formatter.

florian-h05 commented 2 years ago

As there has been no recent progress or activity and the solution proposed in this PR is no solution we can use due to the increased heap usage, I will close this PR now Discussion can be continued in issue #54 .

rkoshak commented 8 months ago

I was helping someone on the forum. While the inclusion of some or all of the locale's by default isn't an option, I think one thing we missed from this PR is the instructions for users to install the jods-js locales themselves. Was that deliberate or an oversight? If an oversight I'll submit a quick PR to the docs to document how to install and use a locale.

florian-h05 commented 8 months ago

Was that deliberate or an oversight? If an oversight I'll submit a quick PR to the docs to document how to install and use a locale.

Definitely not deliberate, I think it would be a great help for users to have these docs!

openhab / openhab-js

[WIP] Time: Add access to JS-Joda Locales #94