lephyrus / ngx-translate-messageformat-compiler

Advanced pluralization (and more) for ngx-translate, using standard ICU syntax which is compiled with the help of messageformat.js.
MIT License
93 stars 30 forks source link

Add angular V9 support #55

Closed 7jpsan closed 4 years ago

7jpsan commented 4 years ago

Hi,

I hope this is useful and can be used to enable angular 9. I have tested it locally with my app and everything seems in order.

During integration, when building the lib locally and installing in the consumer app from file (local dist) make sure you have the preserveSymlinks set to true:

In package.json (consumer app) path relative to app...

"ngx-translate-messageformat-compiler": "file:../ngx-translate-messageformat-compiler/dist",

in angular.json of consumer app:

...
 "architect": {
        "build": {
          "builder": "@angular-devkit/build-angular:browser",
          "options": {
            "preserveSymlinks": true,
            "aot": true,
...

Learned that from:

https://github.com/angular/angular/issues/25813#issuecomment-440283483

lephyrus commented 4 years ago

Hey @7jpsan, thanks for the PR, nicely done! 👍 I've just had a quick look, but am I right to assume that the change that makes Ivy work is the added @Injectable() decorator for MessageFormatCompiler? Are any other changes instrumental in that?

Also, did you find that dropping support for older versions of Angular and ngrx-translate strictly necessary to support Angular 9 and Ivy? I don't want to force people to upgrade unnecessarily, I think offering the broadest possible support in terms of peer dependency versions is good policy for a library.

7jpsan commented 4 years ago

Hey @7jpsan, thanks for the PR, nicely done! 👍 I've just had a quick look, but am I right to assume that the change that makes Ivy work is the added @Injectable() decorator for MessageFormatCompiler? Are any other changes instrumental in that?

Also, did you find that dropping support for older versions of Angular and ngrx-translate strictly necessary to support Angular 9 and Ivy? I don't want to force people to upgrade unnecessarily, I think offering the broadest possible support in terms of peer dependency versions is good policy for a library.

When I first tried adding the decorator, but building it with Angular 8 it didn't work properly when the translate was being used as an attribute, only when used as a Pipe {{ 'some-key' | translate }}. I haven't tried the other around... Building with IVY and using it in a ng8 project.

I'll give it a try and see how it looks, but my hunch is that it needs Ivy.

lephyrus commented 4 years ago

Ah, the library needs to be built with Ivy, I see. I really haven't looked into Ivy yet, that's why I'm not aware of what it means for libraries. Would be unfortunate if they force libraries to either work with Ivy or work with older Angular apps. Thanks for looking to clarify that.

lephyrus commented 4 years ago

I've looked into this, and compiling a library with Ivy is in fact strongly discouraged for the time being. The good thing is that I can support Angular 6 through 9. Anyway, I've migrated the library to use the Angular CLI which should "always do the right thing". The TranslateMessageFormatCompiler class still needed the @Injectable decorator to avoid an error with Angular 9/Ivy. I hope everything's working now.

Thanks again for the PR, @7jpsan - even though I did it differently, you've put me on the right track.

psoares-resilient commented 4 years ago

I've looked into this, and compiling a library with Ivy is in fact strongly discouraged for the time being. The good thing is that I can support Angular 6 through 9. Anyway, I've migrated the library to use the Angular CLI which should "always do the right thing". The TranslateMessageFormatCompiler class still needed the @Injectable decorator to avoid an error with Angular 9/Ivy. I hope everything's working now.

Thanks again for the PR, @7jpsan - even though I did it differently, you've put me on the right track.

No worries, sorry about the lack of update, but I've been on Holidays and just back now... Glad it got resolved and we don't have any breaking changes!

Cheers!