feat(core): add option for lazy loaded modules to extend translations

[stemda]

Added an option to merge lazy loaded translations with the initially loaded ones (instead of ignoring them if already present for a given language).

Probably fixes some of the issues discussed in

#425

[monkeycatdog]

I think, before merge this request need update readme

[NicolasSeiler]

@ocombe We would really appreciate it if this change were merged into the master. Thank you.

[laurent67100]

All looking good

[nickyferry]

mergeDeep within the loadingTranslations?
Otherwise would be great to merge this in!!

[rottingcorpse]

Looking good, get this merged ASAP!

[Hagith]

LGTM

[leon-marzahn]

I doubt this is going to see any updates, the creator is not active anymore. Busy with his thing.

[ocombe]

actually I'll have to make a new release for angular v9 because some things will be deprecated, I'll try to squeeze as many PRs as possible at the same time. I just need to take a few days to work on the project for that

[leon-marzahn]

So, what's the status on this?

[leon-marzahn]

At least we got a funding button like 2 months ago

[ocombe]

Well this really gives me the motivation to merge this PR, thanks @PsychoPflanze.

[leon-marzahn]

@ocombe So you react to sarcasm? This library is actively used in a lot of applications and merging complete pull requests is the least i would expect by such a big and awesome library. Sorry if i am coming off like this, but this is a feature that was requested almost half a year ago and it's not that tough to merge (In my opinion)

[ocombe]

It's quite easy to merge any PR, what's complicated is to make sure that the PRs don't break existing users, and that they don't introduce bugs or make the lib slower, and then you have to release a new version, which takes time as well. And then you have to maintain the new functionalities...
I asked a while ago on github if some people would be willing to pay for an advanced version of the lib, and for its development, and almost everyone said no. The alternative being that I only supported the lib to make sure that it wouldn't break with new version of Angular, which is what I've been doing.
The lib is very modular, it's easy to replace almost any part by a custom version if you need it.
With Angular v9 we get will a completely new i18n system in core that covers almost everything that you usually need for i18n (except switching the language at runtime without reloading the app), with much better performance. It was clearly stated from the beginning that this was an alternative because Angular i18n didn't offer the functionalities that people needed, I don't think this is going to be the case anymore.
If you still want an external lib that is actively maintained you can use transloco, their team is doing an excellent work: https://github.com/ngneat/transloco.

[leon-marzahn]

I understand that part, but was there never an option to have other maintainers? I personally would pay for a maintained version, but of course casual users wouldn't. The other issue is that this library is downloaded 300k times a week, and used in a lot of enterprise applications, and the code style of angular is evolving and specifically this feature would help to get it on a new level. Being able to split translations is amazing. Angular i18n is sadly not really possible for most systems, as no one wants to rebuild the whole application for each language, your library is much more valid for applications. Also, you mentioned transloco, transloco sadly isn't enterprise ready at all. The project owner isn't really known for that, and i've made past experiences with him on that front

[ocombe]

@stemda could you add a test and the documentation in README.md so that I can merge this PR?

[ocombe]

@PsychoPflanze I asked for maintainers and got no answers (or some people were motivated at first and then never did anything).
I even asked the transloco team if they wanted to take over this lib instead of releasing yet another one (before they released their lib) and they refused.

In v9 you will be able to build your app once for all languages, and the angular.json config has been simplified a lot (no more duplication of configs for each language).

[leon-marzahn]

@ocombe :O I didn't know that actually, well, we'll have to see how many people can actually migrate to v9, because of Ivy and all that.

Also, that sucks that no one is involving themselves. Well, i can offer myself, because i have been using this library pretty much since the beginning and on enterprise level. But now starting with maintainers is already too late maybe

[ocombe]

Everyone should be able to use Ivy, the team has been working hard on retro-compatibility.
I'm working on a new i18n lib (and tools) called Locl that will be built on top of the new Angular i18n $localize to add all of the missing functionalities: you get the performance of having i18n integrated into the framework, and the benefits of an external lib (helpers and tools).
This one will be paid for commercial usage (and free for open source), as I don't want to make the same mistake again to work on a lib used by so many people for free, it takes too much time for me now (my life is very different from what it was when I started ngx translate)

[leon-marzahn]

@ocombe Yeah, and you deserve that man. Working on such a library for free is a big sacrifice. If you're searching for maintainers, i'm open, but other than that good luck with your project.

I still would appreciate this feature though 🤣

[ocombe]

let's make a deal, I'll merge it if you add docs and a test

[leon-marzahn]

Alright, i'll look into it

[sergiubologa]

@leon-marzahn @stemda any chance adding the test and the docs anytime soon?

[leon-marzahn]

@sergiubologa The thing is: i can't just push to his branch. Problem is that for me to do this i'd have to open another pull request

[ocombe]

No worries, open a new PR and link it to this one, I'll close this one.

[sergiubologa]

I'm not sure if this implementation is really working or I misunderstand how it should work.

As I understand it, if the forChild with extend: true is used in a child module the child translations should be merged with the ones already loaded, but it doesn't work. Here's a Stackblitz demo project that doesn't work, even with the changes from this PR. The child translations are never loaded actually.

Am I missing something?

[leon-marzahn]

I'm not sure if this implementation is really working or I misunderstand how it should work.

As I understand it, if the forChild with extend: true is used in a child module the child translations should be merged with the ones already loaded, but it doesn't work. Here's a Stackblitz demo project that doesn't work, even with the changes from this PR. The child translations are never loaded actually.

Well, yeah, your stackblitz doesn't work, because it's not using the changes from this PR

[sergiubologa]

Well, yeah, your stackblitz doesn't work, because it's not using the changes from this PR

@leon-marzahn I've tried that example on my local machine with the changes from this PR and it behaves exactly the same. It doesn't even trigger the request to get the child translations.

[leon-marzahn]

Well, yeah, your stackblitz doesn't work, because it's not using the changes from this PR

I've tried that example on my local machine with the changes from this PR and it behaves exactly the same. It doesn't even trigger the request to get the child translations.

You put the extend flag on your child module. And it seems to be working for me, at least in the test i wrote.

[leon-marzahn]

@stemda & @sergiubologa & @ocombe I've added a test and a sort-of documentation. Let me know what you think 😄

[sergiubologa]

Well, code looks good but there's something I am missing here. In my use case (stackblitz demo) doesn't seem to work and I don't know why. The child translations are not even loaded, even when I run that code locally with the changes from this PR.

@leon-marzahn can you please take a look and let me know if this is a valid use case for this feature?

Can anybody provide a use case that should work with these changes?

[leon-marzahn]

Well, code looks good but there's something I am missing here. In my use case (stackblitz demo) doesn't seem to work and I don't know why. The child translations are not even loaded, even when I run that code locally with the changes from this PR.

@leon-marzahn can you please take a look and let me know if this is a valid use case for this feature?

Can anybody provide a use case that should work with these changes?

I don't really know why it's not working for you locally, but i also don't really know the interaction with forChild right now, i will take a look later.

[sergiubologa]

@leon-marzahn ok, I've found out why is not working and that's because my example is not using lazy modules. It has to do with the way DI works in Angular because when a module is eagerly loaded there's no way to provide another loader to the Translate module. So, I guess, we need to mention in the docs that it's working only when using lazy modules, to not confuse other users.

Here's the same Stackblitz example, but with lazily loaded child: https://stackblitz.com/edit/angular-wn35xq and this will work with the changes from this PR.

[sergiubologa]

We should mention that this is working only when the child module is lazily loaded.

[leon-marzahn]

How about something like "To make a lazy loaded child module extend translations from parent modules..."?

[sergiubologa]

Yeah, that should work. Maybe make it bold?

[leon-marzahn]

@leon-marzahn ok, I've found out why is not working and that's because my example is not using lazy modules. It has to do with the way DI works in Angular because when a module is eagerly loaded there's no way to provide another loader to the Translate module. So, I guess, we need to mention in the docs that it's working only when using lazy modules, to not confuse other users.

Here's the same Stackblitz example, but with lazily loaded child: https://stackblitz.com/edit/angular-wn35xq and this will work with the changes from this PR.

Alright, didn't think of that, but i assumed it's because the second loader didn't work 😄

[leon-marzahn]

@sergiubologa This is probably out of scope, but i feel like it should also be possible to load another translation file even if not lazy loading

[sergiubologa]

@sergiubologa This is probably out of scope, but i feel like it should also be possible to load another translation file even if not lazy loading

If you manage to make it work without lazy loading the module would be great!

@ocombe when can we have this feature released? 😄

[jkubiszewski]

I don't think this works for files loaded via http.
Or maybe I can't configure it properly. @sergiubologa Can you please show a working example?

[sergiubologa]

@ye3i it should work. What's your configuration?

[jkubiszewski]

For instance in the stackblitz mentioned above
https://stackblitz.com/edit/angular-oqrxkf

[sergiubologa]

@ye3i Actually, seems like there's an issue.

You can make that example to work by changing translateService.use('en'); into translateService.setDefaultLang('en'); in app.component.ts file.

Unfortunately, if you have another lazy loaded component that calls translateService.use will not work.

Here's a stackblitz that demonstrate this: https://stackblitz.com/edit/angular-sm99qe-ngx-translate-lazy-load

I'll open another issue for this.

Here it is: #1175

[jkubiszewski]

@sergiubologa Thanks for your help. I was confused because you wrote above that everything works. In my spare time, I'll try to find a solution.

For now, a better workaround than the one you proposed seems to be the setting this._translateService.currentLang = '';
this._translateService.use ('en');
in the lazy component. And it work with many lazy loaded module.

Here's a stackblitz that demonstrate this:
https://stackblitz.com/edit/angular-sm99qe-ngx-translate-lazy-load-xyg59v?file=src/sibling/sibling.component.ts

[thessoro]

@sergiubologa Thanks for your help. I was confused because you wrote above that everything works. In my spare time, I'll try to find a solution.

For now, a better workaround than the one you proposed seems to be the setting this._translateService.currentLang = '';
this._translateService.use ('en');
in the lazy component. And it work with many lazy loaded module.

Here's a stackblitz that demonstrate this:
https://stackblitz.com/edit/angular-sm99qe-ngx-translate-lazy-load-xyg59v?file=src/sibling/sibling.component.ts

This trick worked for me. Thanks

[PrashantMohta]

does this not propagate language change from the service provided forRoot to all the child modules ?

[leon-marzahn]

does this not propagate language change from the service provided forRoot to all the child modules ?

This shouldn't have changed that behaviour