jsverse / transloco

πŸš€ 😍 The internationalization (i18n) library for Angular
https://jsverse.github.io/transloco/
MIT License
2.01k stars 195 forks source link

Problem with setting translocoConfig using httpcall in factory, when TranslocoService as dependency anywhere #249

Closed matusbielik closed 3 years ago

matusbielik commented 4 years ago

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request
[ ] Other... Please describe:

Current behavior

After migrating from ngx-translate, transloco can't load translocoConfig using factory, as the config object needs to be populated by data from ConfigService (ngx-config), which loads the config file loaded by http call, if the TranslocoService is dependency in any module or provided service, e.g. HttpInterceptor.

I know this is very mouthful so please check the minimal reproduction example app below.

Expected behavior

We expect to be able to get values from ConfigService in customTranslateConfigFactory, just as when using ngx-translate, as the configuration of our app has not changed.

Minimal reproduction of the problem with instructions

Please check the code (you can install and serve the app directly, using docker is not mandatory) in minimal reproduction app and let me explain:

The tech stack in this app is Transloco (migrated from ngx-translate mainly to support Ivy) and ngx-config.

There are slovak and english translation files, identified by lang identifier (sk or en) in the name of the file. The lang identifiers are to be loaded from config file and are replaced in url in HttpInterceptor. Both config and translation json files are loaded by http calls, hosted on cdn. All the logic is currently in app.module.ts:

for config file:

...
export function configFactory(http: HttpClient): ConfigLoader {
  return new ConfigHttpLoader(http, 'http://cdn.pelikan.sk/app/transloco-minimal-example/market-config.json');
}
...
ConfigModule.forRoot({
  provide: ConfigLoader,
  useFactory: (configFactory),
  deps: [HttpClient]
}),
...

for transaltions:



@Injectable({ providedIn: 'root' })
export class TranslocoHttpLoader implements TranslocoLoader {
  constructor(private readonly http: HttpClient) {}

  private readonly i18nUrl: string = `http://cdn.pelikan.sk/app/transloco-minimal-example/{lang}-translations.json`;

  getTranslation(): Observable {
    return this.http.get(this.i18nUrl);
  }
}
...
providers: [
...
  { provide: TRANSLOCO_LOADER, useClass: TranslocoHttpLoader, deps: [HttpClient] },
...
]
...

all good so far, both files are downloaded, also in correct order. We try to create translocoConfig using factory, as we need values from ConfigService.


export function customTranslateConfigFactory(config: ConfigService): TranslocoConfig {
  const marketLangs: ReadonlyArray = config.getSettings>('market.languages', []);
  const availableLangs: Array = marketLangs.map((language: any) => language.code);
  const defaultLang: string = marketLangs.reduce((prev: string, curr: any) => curr.default ? curr.code : prev, '');

  return translocoConfig({
    defaultLang,
    availableLangs,
    failedRetries        : 0,
    reRenderOnLangChange : true,
    fallbackLang         : defaultLang,
    prodMode             : environment.production
  });
}
...
providers: [
...
  { provide: TRANSLOCO_CONFIG, useFactory: (customTranslateConfigFactory), deps: [ConfigService] },
...
]
...

Now... This setup works! But only as far as the TranslocoService is not used as dependency in anywhere. In this app, it is used as dependency for CustomHttpInterceptor, where we want to set this.translate.getActiveLang() as X-App-Language request header. Please note that this use-case is only for example of the behavior, in the enterprise app we want to use injected TranslocoService in many places, e.g. in ngrx Effects etc.

So when TranslocoService is used as dependency of HttpModule (i guess it is as it's used in HttpInterceptor), translocoConfig does not have the ConfigService data (as it needs HttpClient to download data) and the factory returns empty config values. It is a kind of circle dependency problem.

In current state the app works, because the values that have not been able to be set in module are set imperatively in app.component.ts (ConfigService returns correct config data in component):


const marketCode: string = this.config.getSettings('market.code', '');
    const languages: ReadonlyArray = this.config.getSettings('market.languages', []).map((language: any) => language.code);
    const defaultLang: string = this.config.getSettings('market.languages', [])
      .reduce((prev: string, curr: any) => curr.default ? curr.code : prev, '');

    const availableLangs = this.config.getSettings('market.languages', [])
      .map((language: any) => language.code);

    this.translate.setAvailableLangs(availableLangs);
    this.translate.setDefaultLang(defaultLang);
    this.translate.setActiveLang(defaultLang);

But this is not the behavior we prefer (setting imperatively). The app also works if we comment out imperative setting mentioned above, and we remove TranslocoService from CustomHttpInterceptor deps (app.module.ts --- line 85)

What is the motivation / use case for changing the behavior?

Please note that even when this seems to be ngx-config issue at first sight, I really believe it has to do with Transloco too, so please don't shove it off the table right away.

Before, when using ngx-translate, the whole config/translate initialization process was the same, except the class provided for TranslateModule looks little bit different:

in ngx-translate, we use TranslateHttpLoader class to load the data:


export class BrowserTranslateLoader implements TranslateLoader {
  private readonly _browserLoader: TranslateHttpLoader;

  constructor(
    private readonly http   : HttpClient
  ) {
    this._browserLoader = new TranslateHttpLoader(http, http://cdn.pelikan.sk/app/transloco-minimal-example/{lang}-translations.json);
  }

  getTranslation(languageCode: string): Observable {
    return this._browserLoader.getTranslation(languageCode);
  }
}

in Transloco, we load the translation file directly with HttpClient:


@Injectable({ providedIn: 'root' })
export class TranslocoHttpLoader implements TranslocoLoader {
  constructor(private readonly http: HttpClient) {}

  private readonly i18nUrl: string = `http://cdn.pelikan.sk/app/transloco-minimal-example/{lang}-translations.json`;

  getTranslation(): Observable {
    return this.http.get(this.i18nUrl);
  }
}

and this is the only difference.

Is there any way to set up this whole config & translation loading process in AppModule only, as with ngx-translate? Or is this simply impossible by design and the config values must be set imperatively from component?

Thanks for any suggestions!

Environment


Angular version: 9.04


Browser: any

For Tooling issues:
- Node version: any
- Platform:  any            
itayod commented 4 years ago

I am not sure I completely understand what you’re trying to do, could you sum things up for us? What you are trying to achieve and what error do you get?

matusbielik commented 4 years ago

@itayod what I am trying to achieve is to have translations working, as they are not, in circumstances that I have - that's what I've tried to describe to best as I could, it's frustrating that you don't understand. Have you checked the minimal replication app that I have uploaded to github? I've put quite work in replicating the problem in new app.

It's quite easy to follow from there - you just remove imperative setting of configuration from app.component.ts (lines 30-32) and you can see that i18n does not work anymore. It is not able to use translocoConfig settings that are provided by factory in app.module.ts, if TranslocoService is used as dependency... well... anywhere in the app! In this example it is used as dependency for HttpInterceptor in app.module.ts - after removing it, translations work again, but we need it as dependency.

In summary, I am trying to get it work :-/

ArielGueta commented 4 years ago

You have a race condition. customTranslateConfigFactory function isn't going to wait until configFactory request is back.

shaharkazaz commented 4 years ago

@matusbielik have you checked the race condition @ArielGueta is referring to? can you please give an update here πŸ™‚

matusbielik commented 4 years ago

@shaharkazaz hello.

I think it is pretty obvious that there is a race condition as I tried to explain this race condition two times in a row.

I think that there are a few clear questions about my race condition problem that I tried to ask without any luck of having them answered, like:

"Is there any way to set up this whole config & translation loading process in AppModule only, as with ngx-translate? Or is this simply impossible by design and the config values must be set imperatively from component?"

Sure, my question is not best described as a bug but it is more of a asking for design advice for a problem that I would expect in a library's cookbook or something,

because I believe that my use case must be pretty common as using config options that are loaded from file by http call is not something exotic and it worked pretty well with ngx-translate, which recommended transloco as a library to migrate to.

Maybe it is more suitable for stack overflow post than for a github issue, but I hoped this will reach core developers.

I have also put quite a work into recreating minimalistic reproducing of the problem so I was little bit disappointed with answer "race condition", because that's obvious. Maybe saying "this is not possible, you have to use the imperative way from app.component" would help better (and that's how I have "solved" it).

So if nobody is about to try to solve or at least suggest best way to solve this issue, I believe you can close it.

itayod commented 4 years ago

Hi @matusbielik, have you tried loading the configuration you need using APP_INITIALIZER and then use it inside the loader?

emzet commented 4 years ago

@itayod ngx-config uses APP_INITIALIZER

rraziel commented 4 years ago

@matusbielik I think the aforementioned APP_INITIALIZER is what you are looking for, this is what we are using to wait for Transloco to be initialized (i.e. load translation files).

In the main module:

import {APP_INITIALIZER} from '@angular/core';

@NgModule({
  // ...
  providers: [{
    provide: APP_INITIALIZER,
    useFactory: myAppInitialization,
    deps: [TranslocoService]
  }]
})
class MyAppModule { }

The factory itself:

function myAppInitialization(translocoService: TranslocoService) {
    return () => translocoService.load(/* your params */).toPromise();
}

Your application will not be considered initialized until that promise resolves.

emzet commented 4 years ago

Guys, did anyone try to run that docker image @matusbielik prepared? Because what I see from discussion is that no one really understand (or tried to understand) what is the issue. He wants to migrate from ngx-translate to transloco (as transloco is proposed replacement for ngx-translate by the author himself). So summed up and simplified:


Current state: ngx-config + ngx-translate

First application loads configuration by ngx-config (uses APP_INITIALIZER internally) and then load translations by ngx-translate i.e. provides to ngx-translate configuration value (language code) which comes from ngx-config (it loads this value by HTTP GET from config.json), then it is passed to ngx-translate loader, which loads appropriate language file with translations e.g. translations.en.json

No need to set any configuration values imperatively (i.e. list of available languages) in app.component.ts because all is done by BrowserTranslateLoader and moreover TranslateService could be imported in any other injectable e.g. custom HttpInterceptor.


Desired state: ngx-config + transloco (works but not the way he wants)

In this setup it works for him, but he can not use/provide custom TRANSLOCO_CONFIG, because as soon as he set it, he can not use TranslocoService in any other injectable e.g. custom HttpInterceptor (because of race condition/cyclic dependency/whatever). In current state (with ngx-translate) there is not this kind of "issue". The only "workaround" is not to use TRANSLOCO_CONFIG, but set those values imperatively in app.component.ts. (it is his solution)


And finally his questions are:

  • why it does not work the way ngx-translate works? (as there are not so many differences on first sight)
  • does not it work the way he wants because it is purely by design of transloco library? (if yes, then why)
ArielGueta commented 4 years ago

I still can't understand what's the difference with ngx-translate. Can you share how you implemented the _setUpI18n part with ngx-translate?

ArielGueta commented 4 years ago

Because based on your code, I can't see any difference with the loaders. TranslocoHttpLoader and BrowserTranslateLoader are both returns the same results.