Support AoT fullTemplateTypeCheck flag + AsyncPipe

[bfricka]

I'm submitting a ... (check one with "x")

[x ] bug report => check the FAQ and search github for a similar issue or PR before submitting
[ ] support request => check the FAQ and search github for a similar issue before submitting
[ ] feature request

Current behavior Due to the type signature of the TranslatePipe, it doesn't currently place nice with Angular fullTemplateTypeCheck flag and AsyncPipe. The type signature of AsyncPipe is:

transform<T>(obj: null): null;
transform<T>(obj: undefined): undefined;
transform<T>(obj: Observable<T>): T | null;
transform<T>(obj: Promise<T>): T | null;

This means, in order to be compatible, the signature of TranslatePipe should change:

// Current
transform(query: string, ...args: any[]): any;

// New
transform(query: string | null | undefined, ...args: any[]): any;

At the very least, it should accept null. This seems like a pretty easy fix: Just don't call updateValue if query == null (double equals to handle null | undefined).

Expected/desired behavior Translate pipe should be compatible w/ AsyncPipe and fullTemplateTypeCheck.

Reproduction of the problem

@Component({templateUrl: './foo.html'})
class Foo {
  public placeholder$: BehaviorSubject<string> = new BehaviorSubject('');
}

<!-- foo.html -->
<input type="text" [placeholder]="$placeholder | async | translate" />

// tsconfig.json AoT
{
  "angularCompilerOptions": {
    "fullTemplateTypeCheck": true,
  }
}

Results in:

Argument of type 'string | null' is not assignable to parameter of type 'string'.
  Type 'null' is not assignable to type 'string'.

Even though $placeholder will always be a string, AsyncPipe overload type is matching on T | null. While technically this could go back to Angular's type defs for AsyncPipe, there are likely reasons they are using a null union, and also good luck getting any movement on a real Angular issue.

NOTE: This is only going to affect certain things that require a string type. But for built-ins like placeholder, type widening is probably a non-starter.

What is the motivation / use case for changing the behavior? Not having to wrap all components that use $foo | async | translate.

Please tell us about your environment:

• ngx-translate version: ^8.0.0 - ^9.0.0

• Angular version: 5.2.4

• Browser: N/A

[sandangel]

this is simple, I think we can submit a PR

[ocombe]

Ah yes that'd be nice please!

[wiesnery]

Any update to this? We really want to enable strict template checks and a conditional check inside the template really messes it up... If you guys are still interested I would offer to submit a PR.

[AleksandrSibiakov]

As I see PR is still viable and would provide QOL improvement with strictTemplates,

@ocombe could you take a glance?