toonvanstrijp / nestjs-i18n

The i18n module for nestjs.
https://nestjs-i18n.com
Other
625 stars 102 forks source link

Best practices for using this.i18n.t() vs I18nContext.current().t() #613

Closed vqhdev closed 4 months ago

vqhdev commented 4 months ago

I'm looking at the code sample you provided in the documentation for : nestjs-i18n

import { Injectable } from '@nestjs/common';
import { I18nContext, I18nService } from 'nestjs-i18n';

@Injectable()
export class AppService {
  constructor(private readonly i18n: I18nService) {}

  getHello(): string {
    return this.i18n.t('test.HELLO', { lang: I18nContext.current().lang });
  }
}

I understand that both this.i18n.t() and I18nContext.current().t() are used to translate strings. However, I'm not sure why the code uses this.i18n.t('test.HELLO', { lang: I18nContext.current().lang }) instead of simply I18nContext.current().t('test.HELLO').

Could you please explain the difference between these two approaches and why the first one is preferred in this case?

rubiin commented 4 months ago

This is to add the language specific translation. Since you are doing the translation on service with i18nservice, it has no way of knowing the requested language and will always use the fallback if you don't add the { lang: I18nContext.current().lang } . e @vqhdev

rubiin commented 4 months ago

The first one is preferred on controller level as it does have request level access and so the context can resolve the language sent through headers, cookies, etc. Using this on service however will always fallback as service doesnt directly interact with request but only through controller.

toonvanstrijp commented 4 months ago

I think we've to make a V11. This version removes the I18nContext all together. I think it's working too vague and abstract now. We could better give the user control in how to deliver it to services/controllers etc.

rubiin commented 4 months ago

True. this.i18n.t('test.HELLO', { lang: I18nContext.current().lang }); this approach works good on both cpntroller and service. We can remove the translation method from I18nContext , that would be a breaking change though