Closed cbourget closed 1 year ago
@cbourget . In a future PR (PWA app) , I will add an APP_INITIALIZER for translation. At this moment, your injector will not be useful anymore! Am I right?
The APP_INITIALIZER would need to make sure that the config is loaded. If it does, maybe the one in our app will become redundant. I'll get back to it when your PR is merged and we upgrade the lib version. Thanks for the heads-up.
After all, my APP_INITIALIZER would be inside the igo2 project. Not inside the igo2-lib. It's alright for you?
Yes it's ok. I think it would be a good idea to consolidate the app initialization in the library to make sure everything is initialized correctly and in a predictable order (the config being the main culprit) but, for us, this is not blocking with the workaround we've used above.
@cbourget you could take a look to https://github.com/infra-geo-ouverte/igo2/pull/625
You want me to look at the APP_INITIALIZER specifically?
@cbourget How did you get this dependency schema ?
I produced it manually by going through the code. I don't know if it's still valid because a lot has changed in the mean time.
Thanks! In the next branch, the issue is randomly generated... I've tried to add an appinitializer into the core module, with no effects...
I will take a look to this https://github.com/rbalet/ngx-translate-multi-http-loader
I had an issue in the most recent update as well. Basically, the issue stems from the fact that languageService.translate.getTranslation
now returns an observable instead of a promise. The code in my first comment is still valid but we need to ensure that we're returning a promise. Here's how we did it:
import { lastValueFrom } from 'rxjs';
function appInitializerFactory(
configLoader: Promise<unknown>,
languageService: LanguageService,
) {
return () => new Promise<any>((resolve: any) => {
configLoader.then(() => {
const language = languageService.getLanguage();
const promises = [
lastValueFrom(languageService.translate.getTranslation(language))
];
Promise.all(promises).then(() => resolve());
});
});
}
You can replace the appInitializerFactory
in my first comment with this one.
seem to be solved in 1.15 release
Please tell us about your environment:
Igo Version:
1.10.0
Node:
Context
Our app has its own translations as defined in its JSON config.
Upon loading the app, a bunch of errors are thrown and all our translations aren't loaded. It appears that the issue is one of circular dependency and the order in which they are resolved.
Details
Here's the dependency schema of the services involved.
As you can see, there is a circular dependency between the language service and the config service. Also, when
TranslateLoader
reads the translation path fromConfigService
, there is no guarantee that the JSON config has been loaded. In fact, in our case, it's not loaded and this is precisely what causes the missing translation issue.Solution (part 1)
To circumvent this issue, we added an APP_INITIALIZER in our app that makes sure the config is loaded before
LanguageService
andTranslateLoader
.TranslateLoader
is also forced to load the translation files. Here's a snippet of our code:This solution works but it raises another issue: Since
TranslateLoader
requiresHTTP_INTERCEPTORS
(ErrorInterceptor),MessageService
andToastrService
are also loaded. Upon loading,ToastrService
tries to attach its container to the app component but it doesn't exist yet since all of this is happening during the app initialization (APP_INITIALIZER hook). This raises the following issue:TypeError: this._appRef.attachView is not a function at URe.attachComponentPortal
The
toastr
is now broken and no message will ever show up.Solution (part 2)
I think it would be a good idea to revisit the dependency schema and make sure everything is loaded in a predictable order but, for now, we're content with our solution.
We still need to fix the
toastr
issue but there's actually a quite simple fix that can be done inmessage.service.ts
. It is to use the injector to injecttoastr
when it's needed, instead of injecting it in the constructor. I will provide a PR for this.