makkarpov / scalingua

A simple gettext-like internationalization (aka i18n) library for Scala and Play Framework
Apache License 2.0
53 stars 9 forks source link

Scala.js support? #5

Closed ngbinh closed 3 years ago

ngbinh commented 7 years ago

I am wondering if it's possible to add Scala.js support for this library?

makkarpov commented 7 years ago

Oh, I even didn't tried to get it working with Scala.js. But if you need it, obviously, it is possible. I will test it on Scala.js and report results here. I think that the only Java-dependent part here is strings compiled to resources.

ngbinh commented 7 years ago

Thanks, I think we probably only need to cross compile with scala.js the runtime part.

przemekd commented 3 years ago

I could add support for Scala.js, but scalingua relies a lot on Java code so I guess it would be necessary to split this project to make it work on Scala.js. I have created a PR draft https://github.com/makkarpov/scalingua/pull/15 to have a place for discussion. Right now the compilation fails, of course.

przemekd commented 3 years ago

@makkarpov For now I have encountered these problems:

makkarpov commented 3 years ago

Messages object in core project uses reflection

This is rather convenience feature. I don't think that this method is even applicable in JavaScript, since there is no "global class namespace", from which you can magically extract things.

Maybe instead of creating the binary file and reading it, let's place the content of if directly in an object encoded with base64? What's your thought on this?

Java has a limit on string literal length (65535) and total literals count in class (65535 too), you will quickly hit these. So binary file was intentional.

But I think that this approach will work on scala.js.

przemekd commented 3 years ago

Java has a limit on string literal length (65535) and total literals count in class (65535 too), you will quickly hit these. So binary file was intentional.

Yeah, right. So it might not be the best approach for scala.js too... Let me think about some different solutions and feel free to pitch me some ideas if you have any thoughts on this. Having an additional configuration parameter for scalingua-sbt plugin that would allow specifying the URI of the place from where the class would download this binary file might be another option.

rleibman commented 3 years ago

For scala.js, ideally, I'd like something that gets language files at runtime when needed from a url, that way only the necessary languages need to be downloaded.

przemekd commented 3 years ago

@rleibman @makkarpov Let's say I would implement the LoadFromURI strategy. Any idea how would you like me to implement this? If that would work only for Scala.js, is it enough? The AJAX call is asynchronous, so, maybe I should add isInitialized method to the CompiledLanguage class and take Future[InputStream] in the initialization block to not block the JavaScript execution?

makkarpov commented 3 years ago

If that would work only for Scala.js, is it enough?

Sure. I don't think that async loading is needed for standalone applications.

As for the futures, this is more complicated. Maybe return Future[Language] instead of Language when loading, so no blocking (and no modifications in the language itself) will be necessary, and users can decide how to handle asynchronity.

rleibman commented 3 years ago

I like the idea of a future. I would keep with simple ajax, personally, I like sttp, but there's no point in adding a required dependency. You could also leave the call open and allow the user to provide both their client and server side calls.

przemekd commented 3 years ago

I like the idea of a future. I would keep with simple ajax, personally, I like sttp, but there's no point in adding a required dependency.

Yep, I share this sentiment.

As for the futures, this is more complicated. Maybe return Future[Language] instead of Language when loading, so no blocking (and no modifications in the language itself) will be necessary, and users can decide how to handle asynchronity.

That would be definitely a cleaner approach. But as for now, the compileLocales task creates CompiledLanguage classes along with Messeges object that provides these initialized classes. I wonder if I should create a separate mode of operation for LoadFromURI strategy (e.g. create LazyLanguages helper class that would keep the list of available, lazy-loadable languages and that would return Future[Language]) or maybe I should try to rework the API in order to use Future[Language] everywhere...

przemekd commented 3 years ago

Hi! First of all, sorry for this PR taking so long, but I didn't have much time lately. I decided to provide the LoadInRuntime strategy to enable creating Language_xx_XX objects the way a user wants during runtime.

What do you think about the PR @makkarpov? Do you think it's good enough to be merged? If there is still something to improve, let me know.

makkarpov commented 3 years ago

Yeah, I think that it's very good.

One issue remains, hovewer. Since I had never worked with Scala.js I'm not sure whether current publishing task will correctly publish ScalaJS artifacts to maven.

Is command like sbt +publishSigned still sufficient?

przemekd commented 3 years ago

@makkarpov Yep, it should work :+1:

makkarpov commented 3 years ago

Ok, I will try to publish it as 1.0 version then

makkarpov commented 3 years ago

By the way Travis tests failed somehow (https://travis-ci.org/github/makkarpov/scalingua/jobs/754903041#L1224), trying to pull mysterious ru.makkarpov:scalingua_sjs1_2.12:0.9.1-SNAPSHOT artifact.

przemekd commented 3 years ago

By the way Travis tests failed somehow (https://travis-ci.org/github/makkarpov/scalingua/jobs/754903041#L1224), trying to pull mysterious ru.makkarpov:scalingua_sjs1_2.12:0.9.1-SNAPSHOT artifact.

Hmmm, that's the Scala.js compiled scalingua package. I guess it's not there because plugin depends on JVM version of scalingua. I have it published locally and that's why it works for me...

makkarpov commented 3 years ago

That was also the case with 'regular' artifacts, so publishLocal is included before SBT tests.

przemekd commented 3 years ago

Hmm, that's what I have locally after the execution of publishLocal task:

~$ find .ivy2/local/ru.makkarpov/*/ -mindepth 1 -maxdepth 1
.ivy2/local/ru.makkarpov/scalingua_2.11/0.9.1-SNAPSHOT
.ivy2/local/ru.makkarpov/scalingua_2.12/0.9.1-SNAPSHOT
.ivy2/local/ru.makkarpov/scalingua_2.13/0.9.1-SNAPSHOT
.ivy2/local/ru.makkarpov/scalingua-core_2.11/0.9.1-SNAPSHOT
.ivy2/local/ru.makkarpov/scalingua-core_2.12/0.9.1-SNAPSHOT
.ivy2/local/ru.makkarpov/scalingua-core_2.13/0.9.1-SNAPSHOT
.ivy2/local/ru.makkarpov/scalingua-core_sjs1_2.11/0.9.1-SNAPSHOT
.ivy2/local/ru.makkarpov/scalingua-core_sjs1_2.12/0.9.1-SNAPSHOT
.ivy2/local/ru.makkarpov/scalingua-core_sjs1_2.13/0.9.1-SNAPSHOT
.ivy2/local/ru.makkarpov/scalingua-play_2.12/0.9.1-SNAPSHOT
.ivy2/local/ru.makkarpov/scalingua-play_2.13/0.9.1-SNAPSHOT
.ivy2/local/ru.makkarpov/scalingua-sbt/scala_2.12
.ivy2/local/ru.makkarpov/scalingua-shaded_2.11/0.9.1-SNAPSHOT
.ivy2/local/ru.makkarpov/scalingua-shaded_2.12/0.9.1-SNAPSHOT
.ivy2/local/ru.makkarpov/scalingua-shaded_2.13/0.9.1-SNAPSHOT
.ivy2/local/ru.makkarpov/scalingua_sjs1_2.11/0.9.1-SNAPSHOT
.ivy2/local/ru.makkarpov/scalingua_sjs1_2.12/0.9.1-SNAPSHOT
.ivy2/local/ru.makkarpov/scalingua_sjs1_2.13/0.9.1-SNAPSHOT
przemekd commented 3 years ago

Yeah, that was my bad. I forgot that this dependency is defined in build.sbt file and not in the Travis job. I made a fix, I am testing it locally right now.

przemekd commented 3 years ago

I've run sbt +test scripted locally after removing local packages first, and it works now:

[success] Total time: 454 s (07:34), completed Jan 17, 2021, 9:01:27 PM

The Travis job should finish successfully as well, FYI @makkarpov.

makkarpov commented 3 years ago

https://repo1.maven.org/maven2/ru/makkarpov/scalingua_sjs1_2.13/1.0/

I had published it as 1.0. Thanks for the contribution!