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

(feat): Namespace support #1284

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

ReschAndr
Copy link

@ReschAndr ReschAndr commented Jan 29, 2021

First of all this PR does not contain breaking changes.
The only issue could be, that the @biesbjerg/ngx-translate-extract tool doesn't support extracting the keys from the namespace translate service, pipe, and directive yet, but I will also provide a PR on their repo to add support for that.

Motivation

We are currently working on migrating an existing web application (asp.net forms) to angular. Until now we used our own low-key namespace-translate pipe, which came with the issue of not being able to use the ngx-translate-extract tool. With this pr, I want to add the functionality of not having to define the absolute path of the translation key within very deep nested components. This will prevent spelling mistakes within the key and should help code more orderly.

Added Components

NamespaceTranslateService

The service should be provided on the component level with the namespaceTranslateServiceProvider helper.

It provides functions to get the translation for a specific key(s) which will be prefixed with the namespace given to the namespaceTranslateServiceProvider function.

NamespaceTranslatePipe

The NamespaceTranslatePipe provides the same functionality as the TranslatePipe but prefixes all keys with the namespace given to the namespaceTranslateServiceProvider.

NamespaceTranslateDirective

The NamespaceTranslateDirectiveprovides the same functionality as the TranslateDirective but prefixes all keys with the namespace given to the namespaceTranslateServiceProvider.

Comparison

Without Namespace Translate

import {Component} from '@angular/core';
import {TranslateService} from '@ngx-translate/core';

@Component({
    selector: 'my-deep-nested-component',
    template: `
        <div>{{ 'PATH.TO.MY.DEEP.NESTED.COMPONENT.HELLO' | translate:param }}</div>
        <div translate [translateParams]="{value: 'world'}" >PATH.TO.MY.DEEP.NESTED.COMPONENT.HELLO</div>
    `
})
export class MyDeepNestedComponent {
    param = {value: 'world'};

    constructor(translate: TranslateService) {
      const instant = translate.instant("PATH.TO.MY.DEEP.NESTED.COMPONENT.HELLO", param);
      
      const observable = translate.get("PATH.TO.MY.DEEP.NESTED.COMPONENT.HELLO", param);
    }
}

With Namespace Translate

import {Component} from '@angular/core';
import {NamespaceTranslateService, namespaceTranslateServiceProvider} from '@ngx-translate/core';

@Component({
    selector: 'my-deep-nested-component',
    template: `
        <div>{{ 'HELLO' | namespaceTranslate:param }}</div>
        <div namespace-translate [translateParams]="{value: 'world'}" >Hello</div>
    `,
    providers: [
        { provide: TRANSLATION_NAMESPACE, useValue: "PATH.TO.MY.DEEP.NESTED.COMPONENT"},
        NamespaceTranslateService
    ]
})
export class MyDeepNestedComponent {
    param = {value: 'world'};

    constructor(namespaceTranslate: NamespaceTranslateService) {
      const instant = namespaceTranslate.instant("HELLO", param);
      
      const observable = namespaceTranslate.get("HELLO", param);
    }
}

@ReschAndr ReschAndr marked this pull request as draft January 29, 2021 10:29
angular doesn't allow the usage of function in providers with the default settings.
@kapsiR
Copy link

kapsiR commented Feb 2, 2021

@ocombe Would be cool if this gets merged 😉

}
}

// const namespaceTranslateFactory = (namespace: string) => (translate: TranslateService) => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgotten commented code

@hosuaby
Copy link

hosuaby commented Mar 5, 2021

Hello @ReschAndr, @ocombe

I came across this PR because I have a very similar implementation in my project. I define translations per component, and use namespaces to avoid collisions between translation keys.

I checked the code and tests and currently don't noticed something missing.

I confirm that it would be usefull to have namespace support where ever withing this projet, or as separate plugin.

loader: { provide: TranslateLoader, useClass: FakeLoader }
})
],
providers: [{ provide: TRANSLATION_NAMESPACE, useValue: "NAMESPACE" }]
Copy link

@hosuaby hosuaby Mar 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must be:

providers: [
  { provide: TRANSLATION_NAMESPACE, useValue: "NAMESPACE" },
  NamespaceTranslateService
]

loader: { provide: TranslateLoader, useClass: FakeLoader }
})
],
providers: [{ provide: TRANSLATION_NAMESPACE, useValue: "NAMESPACE" }]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must be:

providers: [
  { provide: TRANSLATION_NAMESPACE, useValue: "NAMESPACE" },
  NamespaceTranslateService
]

@@ -0,0 +1,273 @@
import { ChangeDetectionStrategy, Component, ElementRef, Injectable, ViewChild, ViewContainerRef } from '@angular/core';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to call this file namespace-translate.directive.spec.ts (with dash)

imports: [
TranslateModule.forRoot()
],
declarations: [App]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing:

providers: [
  { provide: TRANSLATION_NAMESPACE, useValue: "NAMESPACE" },
  NamespaceTranslateService
],

@@ -0,0 +1,272 @@
import { ChangeDetectionStrategy, ChangeDetectorRef, Component, Injectable, ViewContainerRef } from "@angular/core";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to call this file namespace-translate.pipe.spec.ts (with dash)

loader: { provide: TranslateLoader, useClass: FakeLoader }
})
],
providers: [{ provide: TRANSLATION_NAMESPACE, useValue: "NAMESPACE" }],
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must be:

providers: [
  { provide: TRANSLATION_NAMESPACE, useValue: "NAMESPACE" },
   NamespaceTranslateService
],

@FortinFred
Copy link

FortinFred commented Mar 22, 2021

Sorry if this sounds like criticism but I find this new feature overly complexe to use. If I understand it correctly, it introduces a new service and new Pipe with an injection token that has to be provided to defined the namespace.

Having the namespace provided means it has to be static, What if we want to have a dynamic namespace?

I briefly made a test on my side. This is not meant to be complete but I've been able to make something that would work almost seamlessly in existing components with the translate pipe.

Usage in template:

<translate-context prefix="PATH.TO.MY.DEEP.NESTED.COMPONENT">

    <p translate>HELLO</p>

</translate-context>

TranslateContextComponent

import { Component, Inject, Injectable, Input, OnInit, SkipSelf } from '@angular/core';
import { TranslateService } from '@ngx-translate/core';
import { Observable } from 'rxjs';


@Injectable()
export class ContextTranslateService extends TranslateService {

  prefix = '';

  public get(key: string | Array<string>, interpolateParams?: Object): Observable<string | any> {
    return super.get(this.prefix + '.' + key, interpolateParams);
  }



}

@Component({
  selector: 'translate-context',
  template: `<ng-content></ng-content>`,
  styles: [],
  providers: [{provide: TranslateService, useClass: ContextTranslateService}]
})
export class TranslateContextComponent implements OnInit {

  @Input()
  prefix: string;



  constructor(@Inject(TranslateService) private contextTranslateService: ContextTranslateService) { }

  ngOnInit() {
    this.contextTranslateService.prefix = this.prefix;
  }

}

I'm not sure yet if it's a bad idea to extend the TranslateService... Worst case, I would implement all methods of TranslateService, inject the real TranslateService using the @SkipSelf constructor decorator and proxy all functions calls to it.

@FortinFred
Copy link

FortinFred commented Jan 10, 2022

I've added my implementation:
#1358

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants