jsverse / transloco

🚀 😍 The internationalization (i18n) library for Angular
https://jsverse.github.io/transloco/
MIT License
2.01k stars 194 forks source link

Does not work as root in lazy loaded module #253

Closed Azbesciak closed 4 years ago

Azbesciak 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

When loaded in root or directly connected module, it works fine but when loaded as lazy module there is a problem:

taticInjectorError(AppModule)[TranslocoService -> InjectionToken TRANSLOCO_TRANSPILER]: 
    StaticInjectorError(Platform: core)[TranslocoService -> InjectionToken TRANSLOCO_TRANSPILER]: 
      NullInjectorError: No provider for InjectionToken TRANSLOCO_TRANSPILER!
NullInjectorError: StaticInjectorError[I18nService -> TranslocoService]: 
  StaticInjectorError(AppModule)[TranslocoService -> InjectionToken TRANSLOCO_TRANSPILER]: 
    StaticInjectorError(Platform: core)[TranslocoService -> InjectionToken TRANSLOCO_TRANSPILER]: 
      NullInjectorError: No provider for InjectionToken TRANSLOCO_TRANSPILER!

Expected behavior

works not only as imported in static module

Minimal reproduction of the problem with instructions

same as https://github.com/ngneat/transloco/issues/152. I have also tried to use TranslocoService as an own provider from useFactory, but it is not used. Maybe the cause is provideIn: 'root'?

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

I am creating application/library which have translation module configured before usage. Configuration is not predefined, and may be different

Environment


Angular version: 8.2.14
transloco: 2.3.15


Browser:
- [x] Chrome (desktop) version 80.0.3987.122
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [ ] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX

For Tooling issues:
- Node version: v12.13.1
- Platform: windows            
ArielGueta commented 4 years ago

You should provide the config once in the root module.

Azbesciak commented 4 years ago

But in a root module I dont know my config.

ArielGueta commented 4 years ago

I'm not sure I understand your application. What if the user doesn't enter the lazy route? You will not have translations

Azbesciak commented 4 years ago

He does not enter the route, he loads the entire app as a component, at the beggining he needs to pass the configuration. And yes, multiple configurations are possible. What is the problem? As I remember singleton is an anti-pattern. And not-managed especially.

Azbesciak commented 4 years ago

Is it possible to fix this behaviour?

draylegend commented 4 years ago

@Azbesciak maybe you could use this approach by using a static function in TranslocoRootModule. I needed to pass in some environment vars, so I came to the static function. I was inspired by NGRX StoreModule. It is also a recommended way to provide dependencies to a whole app.

In the end you have the AppModule:

@NgModule({
  imports: [
    TranslocoRootModule.forRoot({
      prodMode: environment.production,
    }),
  ],
})
export class AppModule {
}

If you need transloco functionality somewhere else, just import it in your LazyModule:

@NgModule({
  imports: [
    TranslocoRootModule,
  ],
})
export class LazyModule {
}

or

If you have multiple entry points, you could also use the forRoot static function by passing a config.

PRO TIP: you can get quicker help if you provide a repro repo ;) so we can better understand your problem PRO TIP 2: stackblitz

Azbesciak commented 4 years ago

As I said, my config is not known at the begining - I need to pass it via variable to master component, which is application/custom-element/angular-component. So any forRoot is possible. I mean, I have forRoot. But there are multiple roots, and not in terms of provideIn: "root".

BTW: By lazy module I don't have in mind router dependant modules. I think of modules imported via webpack import() and NgModuleFactory; they don't have the same injector, possibly common base, but this is not a solution, because I am already on the lowest possible level. One lower would be sharing between (independant) components, which I don't want to have.

JamboBuenna commented 4 years ago

Bump, also on this issue. Seems like an important problem. Will XDescribe the jasmine tests around this but should come back to this...

shaharkazaz commented 4 years ago

@JamboBuenna @Azbesciak The configuration isn't supposed to be lazy-loaded if you need to fetch it from the server or do some other logic to determine which one to use you should do it in the app initializer.

Azbesciak commented 4 years ago

you should do it in the app initializer.

What if I create library? As you do...? IMO equiring the user to use something in root is not a good idea, and lazy loading is one of the problem. As I said, I am providing lib which, because of it size and optionality, need to be lazy loaded. So far it is impossible because of your solution (since I use it).

Coly010 commented 4 years ago

need to be lazy loaded

I don't know if you're using the correct terminology. What you're looking for is a way to programmatically set the config via the TranslocoService API.

So your setup must look something like:

export class RootComponent {
   @Input() translocoConfig: TranslocoConfig;
}

And you want to be able to do

this.translocoService.setConfig(this.translocoConfig)

I'm not sure if Angular allows you to override an InjectionToken using their Injector that you can inject into the Component. It does have a create method but I think that creates a new Injector rather than overriding a provider.

itayod commented 4 years ago

@Azbesciak I am not sure what exactly is you use case, but why can't you simply set only the configuration eagerly in your main application and then lazy load the entire library.

Also could you please create a small reproduction so I can look into it?

Azbesciak commented 4 years ago

You're welcome: https://github.com/Azbesciak/transloco-lazy-issue I can't because:

  • I am not owner of parent app
  • it is not just a single one
  • it is leak of knowledge and not a good practice to require from client to know what is used behind, especially in such things like translations lib (what if they are using ngx or angular i18n?)
  • makes my API more complex and harder to understand

Theoretically I can do it via 'need-to-be-in-root' module and then to delegate config, but it will be spaghetti code. I like spaghetti, bolognese for example, but not in such scenarios :).

@Coly010 I think that Injector.create will create child injector, and if you pass there a token which already exists it will be overriden because of scope. Just try the following code:

const injector = Injector.create({
      providers: [{
        provide: 'xyz',
        useValue: {a: 'b'},
        deps: []
      }],
      name: 'older',
});
const child = Injector.create({
      providers: [{
        provide: 'xyz',
        useValue: {a: 'c'},
        deps: []
      }],
      parent: injector,
      name: 'child'
});
console.log('GET', child.get('xyz'));

Also, this is used in transloco with defaults. But the problem is you won't override provider already provided in root (not exacly, but I hope you get an idea).

JamboBuenna commented 4 years ago

I no longer support this issue as being a problem. Apologies for false positive. I had made a mistake.

Azbesciak commented 4 years ago

Any update on this subject?

shaharkazaz commented 4 years ago

@Azbesciak Just to make sure I understand, you want to use the TranslocoService only inside your lib/external app correct? like in the repo you shared here.

If so, you can just add the service in the providers array so a local instance can be created since the service that's provided in root has no access to the tokens in the lazy loaded module (new injector is created)

shaharkazaz commented 4 years ago

@Azbesciak Any update?

Azbesciak commented 4 years ago

Sorry, I am a bit busy now, but have this in my mind. I have tried it before I made an issue and there was some problem, don't remember now what it was. What comes to my mind is translate function which uses service in root, but I can make my own in the same manner (rather recommended anyway in my approach). I assume that pipes won't be a problem. Just wondering about other dependant services.

shaharkazaz commented 4 years ago

@Azbesciak no worries, just give an update as soon as you get the chance to check it and we will take it from there 🙂

Azbesciak commented 4 years ago

I've finished with this

@NgModule({
  imports: [TranslocoMessageFormatModule.init(), TranslocoModule],
  exports: [TranslocoDirective, TranslocoPipe],
})
export class TranslationsModule {
  static forRoot(): ModuleWithProviders<TranslationsModule> {
    return {
      ngModule: TranslationsModule,
      providers: [{
        provide: TRANSLOCO_CONFIG,
        useFactory: createConfig,
        deps: [ConfigurationService]
      },
        TranslocoService,
        {
          provide: TRANSLOCO_LOADER,
          useClass: TranslocoHttpLoader
        },
        {
          provide: TRANSLOCO_FALLBACK_STRATEGY,
          useClass: DefaultFallbackStrategy,
          deps: [TRANSLOCO_CONFIG]
        }]
    }
  }
}

At first I tried to also export TRANSLOCO_MISSING_HANDLER and TRANSLOCO_INTERCEPTOR but

Module not found: Error: Can't resolve '@ngneat/transloco/lib/transloco-missing-handler'
Module not found: Error: Can't resolve '@ngneat/transloco/lib/transloco.interceptor'

For all who will come after me - I would expect that because of root something will be (or rather - can, because it is lazy) be loaded faster, so I have used translate function quite often. Now I see very often that some part of app is translated, other some time later, so be aware of it.

Azbesciak commented 4 years ago

Actually, I will reopen. I have noticed that translations are loaded twice, because of pipes. As you see I export those imported from TranslocoModule, and that is my mistake. But when I removed TranslocoModule import, declare both directive and pipe in my module, it requires TRANSLOCO_MISSING_HANDLER and TRANSLOCO_INTERCEPTOR... Ok, but both DefaultHandler and DefaultInterceptor are not exported (are private, take a look at the public-api.ts).

And even when I create my own implementations (as I did), pipe (directive propably also) still reffers to TranslocoService from root... I don't have reference to TranslocoModule anywhere at this moment:


@NgModule({
  imports: [],
  declarations: [TranslocoDirective, TranslocoPipe],
  exports: [TranslocoDirective, TranslocoPipe]
})
export class TranslationsModule {
  static forRoot(): ModuleWithProviders<TranslationsModule> {
    return {
      ngModule: TranslationsModule,
      providers: [{
        provide: TRANSLOCO_CONFIG,
        useFactory: createConfig,
        deps: [ConfigurationService]
      },
        TranslocoService,
        {
          provide: TRANSLOCO_MISSING_HANDLER,
          useClass: MissingTranslationsHandler,
          deps: [LOG_FACT]
        },
        {
          provide: TRANSLOCO_INTERCEPTOR,
          useClass: NoOpTranslocoTranslationInterceptor
        },
        {
          provide: TRANSLOCO_FALLBACK_STRATEGY,
          useClass: DefaultFallbackStrategy,
          deps: [TRANSLOCO_CONFIG]
        },
        { provide: TRANSLOCO_MESSAGE_FORMAT_CONFIG, useValue: undefined },
        {
          provide: TRANSLOCO_TRANSPILER,
          useClass: MessageFormatTranspiler
        },
        {
          provide: TRANSLOCO_LOADER,
          useClass: TranslocoHttpLoader
        }]
    }
  }
}
itayod commented 4 years ago

I personally kind of lost... what you are trying to achieve? It would be great if you could re-explain what is it that you are trying to do, why @shaharkazaz comment didn't work for you, what's the current issue, and update your reproduction repo. Thanks

Azbesciak commented 4 years ago
  • I need to use your lib in my app
  • same app is used as an attached, optional, library in another apps.

So it needs to be a module, in a lazy manner. In current situation, which I hope I showed you, it is impossible. Everything leeks to the root, even when I try to stop it.

I don't want my users to configure anything in root. I have my main module, which they need to include in their apps, but it opens configuration port, and when config is passed, fetches the rest of the content. Yes, config includes language and translations. And optionaly, when they will have transloco also in their apps, in the current situation when they have it in a root, they will conflict with each other, because of this pipes/directives which uses root one. Just mentioning.

itayod commented 4 years ago

@Azbesciak Did you try to add TranslocoService in your providers list as @shaharkazaz suggested? could you update your reproduction so we can try to resolve it and better understand your use-case?

Azbesciak commented 4 years ago
itayod commented 4 years ago

@Azbesciak sure, this is why I wanted you to update your reproduction so we can try to debut that.

Azbesciak commented 4 years ago

I have to admit that It was not from root, but just not configured one. Because of lazy modules two instances (at least) are created, but this can be solved with factory, for example:

{
        provide: TranslocoService,
        useFactory: translocoFactor,
        deps: [
          [new Optional(), new SkipSelf(), TranslocoService],
          [new Optional(), TRANSLOCO_LOADER],
          TRANSLOCO_TRANSPILER,
          TRANSLOCO_MISSING_HANDLER,
          TRANSLOCO_INTERCEPTOR,
          TRANSLOCO_CONFIG,
          TRANSLOCO_FALLBACK_STRATEGY
        ]
      }

But when I tried to replicate the issue (and failed ATM, if something change I will update), but found another one... strange, but don't know where it comes from.

core.js:6014 ERROR Error: Uncaught (in promise): NullInjectorError: StaticInjectorError(xyz)[DynamicService -> TranslocoService]: 
  StaticInjectorError(AppModule)[TranslocoService -> InjectionToken TRANSLOCO_TRANSPILER]: 
    StaticInjectorError(Platform: core)[TranslocoService -> InjectionToken TRANSLOCO_TRANSPILER]: 
      NullInjectorError: No provider for InjectionToken TRANSLOCO_TRANSPILER!
NullInjectorError: StaticInjectorError(xyz)[DynamicService -> TranslocoService]: 
  StaticInjectorError(AppModule)[TranslocoService -> InjectionToken TRANSLOCO_TRANSPILER]: 
    StaticInjectorError(Platform: core)[TranslocoService -> InjectionToken TRANSLOCO_TRANSPILER]: 
      NullInjectorError: No provider for InjectionToken TRANSLOCO_TRANSPILER!

I have noticed that this is because of custom TranslocoService provision:

{
        provide: TranslocoService,
        useFactory: translocoFactor,
        deps: [
          [new Optional(), new SkipSelf(), TranslocoService],
          [new Optional(), TRANSLOCO_LOADER],
          TRANSLOCO_TRANSPILER,
          TRANSLOCO_MISSING_HANDLER,
          TRANSLOCO_INTERCEPTOR,
          TRANSLOCO_CONFIG,
          TRANSLOCO_FALLBACK_STRATEGY
        ]
      }

When provided without it in providers ([..., TranslocoService, ...]), it does not fail. Branch https://github.com/Azbesciak/transloco-lazy-issue/tree/transloco-transpiler-not-found

Azbesciak commented 4 years ago

Guys, any update?

Azbesciak commented 4 years ago

Removing [new Optional(), new SkipSelf(), TranslocoService], from deps and making it file-scope singleton 'solves' a problem. Rather dirty workaround.

So, do you have anything new?

ArielGueta commented 4 years ago

Could you explain from scratch your issue? I think everyone lost you. Try to explain in simple words what you are trying to do.

Azbesciak commented 4 years ago
  • I need to have an ability to use Transloco both in a single app
    • here "provideIn": "root" is OK
  • in library, which can be used in concurrent with another translations lib (i.e. another Transloco), and avoid conflicts;
    • here root is not good
    • it polutes client side/can infer with another implementation
    • it requires the consumer to have translations provided on his own.

Thus, I need to provide Transloco (whole) without root scope.

The problem is all the time I find something what is provided in root, and ATM I don't know what will blow up. And sometimes it blows up in a very weird way, maybe because of my Angular understanding.

Hope it helps.

ArielGueta commented 4 years ago

The Transloco service is meant to be used as a wide-app singleton. It doesn't make sense to have multiple loaders or something like this.

I don't understand what is the library purpose.

Azbesciak commented 4 years ago

Originally it was just an app. Now it need to be also used in another apps

  • in PHP, where it works as an angular-element included dynamically,
  • in another angular app, with a note that you can have 2 'apps' (mine) run in parrarel in the single consumer.

Language will be in 99.99% of usage the same, and I even would not like to support other option (I mean 2 languages), but the problem is with that isolation, which I want to achieve.

I think it is rather a good practice...

ArielGueta commented 4 years ago

It's not possible and does not make sense. You have the TranslocoRootModule, just use it in every application. You are not repeating yourself or something like this.

Azbesciak commented 4 years ago

You have the TranslocoRootModule, just use it in every application

But then my clients are dependant on your implementation, and OK - if they are using it as a lib, in angular for example, then they need to include it in dependencies any way, but what bother me - they need to know all translations I use. I mean - they need to manage them on their own. It is not just a simple plug-in then.

That is the problem - they also use your lib (in angular), and if provided in root it will just crash.

ArielGueta commented 4 years ago

They need to know them anyway. You need to add them in an accessible place such as the assets folder

Azbesciak commented 4 years ago

Do they? if it is provided as a package through npm, they would need to just have it declared in assets in angular.json. I thought rather about each key definition.

ArielGueta commented 4 years ago

No. How do you expect to get it via HTTP?

Azbesciak commented 4 years ago

I have assets path exposed in the configuration, like everything else.

So, as I mentioned - they need to just put my app i18n assets (and everything else like this) in angular.json and make a correct configuration.

ArielGueta commented 4 years ago

So they know about it anyway.

Azbesciak commented 4 years ago

They know that something exists. Don't know what exacly and what is inside. That is a big difference, don't you think?

For example when I update some translations, they don't need to do it also. It is just provided.

example from my app:

 {
        "glob": "**/*",
        "input": "node_modules/leaflet-draw/dist/images",
        "output": "assets/leaflet"
},
 {
       "glob": "**/*",
        "input": "libs/imap/domain-components/styles/assets/maneuvers",
        "output": "assets/maneuvers"
},
{
       "glob": "**/*",
       "input": "libs/imap/domain-components/styles/assets/tacho",
        "output": "assets/tacho"
},
{
        "glob": "**/*",
        "input": "libs/imap/shared/i18n/translations",
         "output": "assets/i18n"
}
ArielGueta commented 4 years ago

So let them include the module, and provide the translation as you wish.

Azbesciak commented 4 years ago

Sure - and here comes the problem. Now, as you mentioned, transloco is whole a root scope, so translations will crash. Example: They have their own TranslocoService, own http provision. What with my translations? Will they be used if the service is in the root? I doubt, even think that it won't compile. Am I wrong?

ArielGueta commented 4 years ago

No. You should have only ONE Transloco module in the app.

Azbesciak commented 4 years ago

I try to tell you that this is not possible :)

ArielGueta commented 4 years ago

So you can't achieve what you want. Sorry.

ArielGueta commented 4 years ago

Don't waste your time.

Azbesciak commented 4 years ago

I already wasted, and achieved what I wanted, but as I said - it is rather a workaround. Thank you though

jdiemke commented 1 year ago

This is in fact a real world problem that can happend in a micro frontend architecture where each remote needs to provide its own translations. Any chance to get micro frontend support where each micro frontend (lazy loaded module) can have its own transloco instance?

keithcarter5 commented 1 year ago

I have exactly the same use case and need this functionality as well.

@Azbesciak, could you provide a link to your workaround?