Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fail to load translations #1004

Closed
cbourget opened this issue Mar 11, 2022 · 12 comments
Closed

Fail to load translations #1004

cbourget opened this issue Mar 11, 2022 · 12 comments
Labels
bug Incorrect or unexpected behaviors // Anomalies de fonctionnement

Comments

@cbourget
Copy link
Collaborator

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.

"language": {
   "prefix": ["./locale/"]
}

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.

  • LanguageService

    • TranslateService

      • TranslateLoader

        • ConfigService

        • HttpClient

          • HTTP_INTERCEPTORS

            • ErrorInterceptor

              • MessageService

                • ConfigService

                • LanguageService

                • ToastrService

As you can see, there is a circular dependency between the language service and the config service. Also, when TranslateLoader reads the translation path from ConfigService, 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 and TranslateLoader. TranslateLoader is also forced to load the translation files. Here's a snippet of our code:

import {
  APP_INITIALIZER,
  InjectionToken,
  NgModule,
  ModuleWithProviders,
  Provider
} from '@angular/core';

import {
  CONFIG_OPTIONS,
  IgoCoreModule,
  ConfigOptions,
  ConfigService,
  LanguageService,
} from '@igo2/core';

export let CONFIG_LOADER = new InjectionToken<Promise<ConfigService>>('Config Loader');

function configLoader(
  configService: ConfigService,
  configOptions: ConfigOptions,
): Promise<unknown> {
  const promiseOrTrue = configService.load(configOptions);
  if (promiseOrTrue instanceof Promise) {
    return promiseOrTrue;
  }
  return Promise.resolve();
}

function appInitializerFactory(
  configLoader: Promise<unknown>,
  languageService: LanguageService,
) {
  return () => new Promise<any>((resolve: any) => {
    configLoader.then(() => {
      const promises = [
        languageService.translate.getTranslation(languageService.getLanguage())
      ];
      Promise.all(promises).then(() => resolve());
    });
  });
}

const providers: Provider[] = [
  {
    provide: CONFIG_LOADER,
    useFactory: configLoader,
    deps: [ConfigService, CONFIG_OPTIONS],
  },
  {
    provide: APP_INITIALIZER,
    useFactory: appInitializerFactory,
    deps: [CONFIG_LOADER, LanguageService],
    multi: true
  },
];

@NgModule({
  imports: [
    IgoCoreModule.forRoot()
  ],
  declarations: [],
  exports: [
    IgoCoreModule,
  ]
})
export class FadqLibCoreModule {
  static forRoot(): ModuleWithProviders<FadqLibCoreModule> {
    return {
      ngModule: FadqLibCoreModule,
      providers: providers
    };
  }
}

This solution works but it raises another issue: Since TranslateLoader requires HTTP_INTERCEPTORS (ErrorInterceptor), MessageService and ToastrService 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 in message.service.ts. It is to use the injector to inject toastr when it's needed, instead of injecting it in the constructor. I will provide a PR for this.

@cbourget cbourget added the bug Incorrect or unexpected behaviors // Anomalies de fonctionnement label Mar 11, 2022
@pelord
Copy link
Member

pelord commented Mar 16, 2022

@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?

@cbourget
Copy link
Collaborator Author

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.

@pelord
Copy link
Member

pelord commented Mar 17, 2022

After all, my APP_INITIALIZER would be inside the igo2 project. Not inside the igo2-lib. It's alright for you?

@cbourget
Copy link
Collaborator Author

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.

@pelord
Copy link
Member

pelord commented Mar 25, 2022

@cbourget you could take a look to infra-geo-ouverte/igo2#625

@cbourget
Copy link
Collaborator Author

You want me to look at the APP_INITIALIZER specifically?

@pelord
Copy link
Member

pelord commented Apr 5, 2023

@cbourget How did you get this dependency schema ?

@cbourget
Copy link
Collaborator Author

cbourget commented Apr 5, 2023

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.

@pelord
Copy link
Member

pelord commented Apr 5, 2023

Thanks! In the next branch, the issue is randomly generated... I've tried to add an appinitializer into the core module, with no effects...

@pelord
Copy link
Member

pelord commented Apr 5, 2023

@cbourget
Copy link
Collaborator Author

cbourget commented Apr 5, 2023

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.

@pelord
Copy link
Member

pelord commented May 2, 2023

seem to be solved in 1.15 release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect or unexpected behaviors // Anomalies de fonctionnement
Projects
None yet
Development

No branches or pull requests

2 participants