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.

Sign up for GitHub

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(transloco|locale): Improve pipe type definitions for strict mode #770

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

feat(transloco|locale): Improve pipe type definitions for strict mode #770

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
09df44c
feat(transloco): 🎸 improve strict mode type
Kaemmelot Jul 24, 2024
70338af
feat(locale): 🎸 improve strict mode types for pipes
Kaemmelot Jul 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 28 additions & 3 deletions libs/transloco-locale/src/lib/pipes/transloco-currency.pipe.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import {

import { BaseLocalePipe } from './base-locale.pipe';

type CurrencyDisplayType = 'code' | 'symbol' | 'narrowSymbol' | 'name';

@Pipe({
name: 'translocoCurrency',
pure: false,
Expand All @@ -35,14 +37,37 @@ export class TranslocoCurrencyPipe
* 1000000 | translocoCurrency: 'narrowSymbol' : {minimumFractionDigits: 0 } : CAD // $1,000,000
*
*/
// overloads for strict mode
transform(
value: number | string,
display: 'code' | 'symbol' | 'narrowSymbol' | 'name' = 'symbol',
display?: CurrencyDisplayType,
numberFormatOptions?: NumberFormatOptions,
currencyCode?: Currency,
locale?: Locale
): string;
transform<T extends null | undefined>(
value: T,
display?: CurrencyDisplayType,
numberFormatOptions?: NumberFormatOptions,
currencyCode?: Currency,
locale?: Locale
): T;
transform<T extends null | undefined>(
value: number | string | T,
Copy link
Collaborator

Choose a reason for hiding this comment

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

My suggestion didn't work?

Copy link
Author

Choose a reason for hiding this comment

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

Not sure if I understood you correctly. I guess you meant to remove the last overload? If I would remove this overload from this pipe and pass in a string | null value, I would get this error:

No overload matches this call.
Overload 1 of 2, '(value: string | number, display?: CurrencyDisplayType | undefined, numberFormatOptions?: NumberFormatOptions | undefined, currencyCode?: string | undefined, locale?: string | undefined): string', gave the following error.
Argument of type 'string | null' is not assignable to parameter of type 'string | number'.
Type 'null' is not assignable to type 'string | number'.
Overload 2 of 2, '(value: null | undefined, display?: CurrencyDisplayType | undefined, numberFormatOptions?: NumberFormatOptions | undefined, currencyCode?: string | undefined, locale?: string | undefined): null | undefined', gave the following error.
Argument of type 'string | null' is not assignable to parameter of type 'null | undefined'. ts(2769)
transloco-currency.pipe.ts(56, 3): The call would have succeeded against this implementation, but implementation signatures of overloads are not externally visible.

So I think I need all three overloads.

display?: CurrencyDisplayType,
numberFormatOptions?: NumberFormatOptions,
currencyCode?: Currency,
locale?: Locale
): string | T;

transform(
value?: number | string | null,
display: CurrencyDisplayType = 'symbol',
numberFormatOptions: NumberFormatOptions = {},
currencyCode?: Currency,
locale?: Locale
): string {
if (isNil(value)) return '';
): string | null | undefined {
if (isNil(value)) return value;
locale = this.getLocale(locale);

const options = {
Expand Down
25 changes: 23 additions & 2 deletions libs/transloco-locale/src/lib/pipes/transloco-date.pipe.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,29 @@ export class TranslocoDatePipe extends BaseLocalePipe implements PipeTransform {
* 1 | translocoDate: { dateStyle: 'medium' } // Jan 1, 1970
* '2019-02-08' | translocoDate: { dateStyle: 'medium' } // Feb 8, 2019
*/
transform(date: ValidDate, options: DateFormatOptions = {}, locale?: Locale) {
if (isNil(date)) return '';
// overloads for strict mode
transform(
date: ValidDate,
options?: DateFormatOptions,
locale?: Locale
): string;
transform<T extends null | undefined>(
date: T,
options?: DateFormatOptions,
locale?: Locale
): T;
transform<T extends null | undefined>(
date: ValidDate | T,
options?: DateFormatOptions,
locale?: Locale
): string | T;

transform(
date: ValidDate | null | undefined,
options: DateFormatOptions = {},
locale?: Locale
): string | null | undefined {
if (isNil(date)) return date;
locale = this.getLocale(locale);

return this.localeService.localizeDate(date, locale, {
Expand Down
23 changes: 20 additions & 3 deletions libs/transloco-locale/src/lib/pipes/transloco-decimal.pipe.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,29 @@ export class TranslocoDecimalPipe
* 1234567890 | translocoDecimal: {useGrouping: false}: en-US // 1234567890
*
*/
// overloads for strict mode
transform(
value: string | number,
value: number | string,
numberFormatOptions?: NumberFormatOptions,
locale?: Locale
): string;
transform<T extends null | undefined>(
value: T,
numberFormatOptions?: NumberFormatOptions,
locale?: Locale
): T;
transform<T extends null | undefined>(
value: number | string | T,
numberFormatOptions?: NumberFormatOptions,
locale?: Locale
): string | T;

transform(
value?: string | number | null,
numberFormatOptions: NumberFormatOptions = {},
locale?: Locale
): string {
if (isNil(value)) return '';
): string | null | undefined {
if (isNil(value)) return value;
locale = this.getLocale(locale);

const options = {
Expand Down
21 changes: 19 additions & 2 deletions libs/transloco-locale/src/lib/pipes/transloco-percent.pipe.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,29 @@ export class TranslocoPercentPipe
* "1" | translocoPercent : {} : en-US // 100%
*
*/
// overloads for strict mode
transform(
value: number | string,
numberFormatOptions?: NumberFormatOptions,
locale?: Locale
): string;
transform<T extends null | undefined>(
value: T,
numberFormatOptions?: NumberFormatOptions,
locale?: Locale
): T;
transform<T extends null | undefined>(
value: number | string | T,
numberFormatOptions?: NumberFormatOptions,
locale?: Locale
): string | T;

transform(
value?: number | string | null,
numberFormatOptions: NumberFormatOptions = {},
locale?: Locale
): string {
if (isNil(value)) return '';
): string | null | undefined {
if (isNil(value)) return value;
locale = this.getLocale(locale);

const options = {
Expand Down
28 changes: 22 additions & 6 deletions libs/transloco-locale/src/lib/tests/pipes/transloco-currency.pipe.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { SpectatorPipe } from '@ngneat/spectator';
import { ChangeDetectorRef } from '@angular/core';
import { TestBed } from '@angular/core/testing';
import { Mock } from 'ts-mocks';

import { TranslocoCurrencyPipe } from '../../pipes/transloco-currency.pipe';
import { LOCALE_CONFIG_MOCK, provideTranslocoServiceMock } from '../mocks';
Expand Down Expand Up @@ -60,17 +63,30 @@ describe('TranslocoCurrencyPipe', () => {
});

describe('None transformable values', () => {
let pipe: TranslocoCurrencyPipe;
let cdrMock: ChangeDetectorRef;

beforeEach(() => {
cdrMock = new Mock<ChangeDetectorRef>({
markForCheck: () => {},
}).Object;

TestBed.configureTestingModule({
providers: [{ provide: ChangeDetectorRef, useValue: cdrMock }],
});
pipe = TestBed.runInInjectionContext(() => new TranslocoCurrencyPipe());
});
it('should handle null', () => {
spectator = pipeFactory(`{{ null | translocoCurrency }}`);
expect(spectator.element).toHaveText('');
expect(pipe.transform(null)).toBeNull();
});
it('should handle undefined', () => {
expect(pipe.transform(undefined)).toBeUndefined();
});
it('should handle {}', () => {
spectator = pipeFactory(`{{ {} | translocoCurrency }}`);
expect(spectator.element).toHaveText('');
expect(pipe.transform({} as any)).toBe('');
});
it('should handle none number string', () => {
spectator = pipeFactory(`{{ 'none number string' | translocoCurrency }}`);
expect(spectator.element).toHaveText('');
expect(pipe.transform('none number string')).toBe('');
});
});

Expand Down
28 changes: 22 additions & 6 deletions libs/transloco-locale/src/lib/tests/pipes/transloco-date.pipe.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { SpectatorPipe } from '@ngneat/spectator';
import { ChangeDetectorRef } from '@angular/core';
import { TestBed } from '@angular/core/testing';
import { Mock } from 'ts-mocks';

import { TranslocoDatePipe } from '../../pipes';
import {
Expand Down Expand Up @@ -88,17 +91,30 @@ describe('TranslocoDatePipe', () => {
});

describe('None date values', () => {
let pipe: TranslocoDatePipe;
let cdrMock: ChangeDetectorRef;

beforeEach(() => {
cdrMock = new Mock<ChangeDetectorRef>({
markForCheck: () => {},
}).Object;

TestBed.configureTestingModule({
providers: [{ provide: ChangeDetectorRef, useValue: cdrMock }],
});
pipe = TestBed.runInInjectionContext(() => new TranslocoDatePipe());
});
it('should handle null', () => {
spectator = pipeFactory(`{{ null | translocoDate }}`);
expect(spectator.element).toHaveText('');
expect(pipe.transform(null)).toBeNull();
});
it('should handle undefined', () => {
expect(pipe.transform(undefined)).toBeUndefined();
});
it('should handle {}', () => {
spectator = pipeFactory(`{{ {} | translocoDate }}`);
expect(spectator.element).toHaveText('');
expect(pipe.transform({} as any)).toBe('');
});
it('should handle none number string', () => {
spectator = pipeFactory(`{{ 'none number string' | translocoDate }}`);
expect(spectator.element).toHaveText('');
expect(pipe.transform('none number string')).toBe('');
});
});

Expand Down
28 changes: 22 additions & 6 deletions libs/transloco-locale/src/lib/tests/pipes/transloco-decimal.pipe.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { SpectatorPipe } from '@ngneat/spectator';
import { ChangeDetectorRef } from '@angular/core';
import { TestBed } from '@angular/core/testing';
import { Mock } from 'ts-mocks';

import { TranslocoDecimalPipe } from '../../pipes';
import {
Expand Down Expand Up @@ -66,17 +69,30 @@ describe('TranslocoDecimalPipe', () => {
});

describe('None transformable values', () => {
let pipe: TranslocoDecimalPipe;
let cdrMock: ChangeDetectorRef;

beforeEach(() => {
cdrMock = new Mock<ChangeDetectorRef>({
markForCheck: () => {},
}).Object;

TestBed.configureTestingModule({
providers: [{ provide: ChangeDetectorRef, useValue: cdrMock }],
});
pipe = TestBed.runInInjectionContext(() => new TranslocoDecimalPipe());
});
it('should handle null', () => {
spectator = pipeFactory(`{{ null | translocoDecimal }}`);
expect(spectator.element).toHaveText('');
expect(pipe.transform(null)).toBeNull();
});
it('should handle undefined', () => {
expect(pipe.transform(undefined)).toBeUndefined();
});
it('should handle {}', () => {
spectator = pipeFactory(`{{ {} | translocoDecimal }}`);
expect(spectator.element).toHaveText('');
expect(pipe.transform({} as any)).toBe('');
});
it('should handle none number string', () => {
spectator = pipeFactory(`{{ 'none number string' | translocoDecimal }}`);
expect(spectator.element).toHaveText('');
expect(pipe.transform('none number string')).toBe('');
});
});
});
28 changes: 22 additions & 6 deletions libs/transloco-locale/src/lib/tests/pipes/transloco-percent.pipe.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { SpectatorPipe } from '@ngneat/spectator';
import { ChangeDetectorRef } from '@angular/core';
import { TestBed } from '@angular/core/testing';
import { Mock } from 'ts-mocks';

import { TranslocoPercentPipe } from '../../pipes';
import { LOCALE_CONFIG_MOCK, provideTranslocoLocaleConfigMock } from '../mocks';
Expand Down Expand Up @@ -31,17 +34,30 @@ describe('TranslocoPercentPipe', () => {
});

describe('None transformable values', () => {
let pipe: TranslocoPercentPipe;
let cdrMock: ChangeDetectorRef;

beforeEach(() => {
cdrMock = new Mock<ChangeDetectorRef>({
markForCheck: () => {},
}).Object;

TestBed.configureTestingModule({
providers: [{ provide: ChangeDetectorRef, useValue: cdrMock }],
});
pipe = TestBed.runInInjectionContext(() => new TranslocoPercentPipe());
});
it('should handle null', () => {
spectator = pipeFactory(getPipeTpl(null));
expect(spectator.element).toHaveText('');
expect(pipe.transform(null)).toBeNull();
});
it('should handle undefined', () => {
expect(pipe.transform(undefined)).toBeUndefined();
});
it('should handle {}', () => {
spectator = pipeFactory(getPipeTpl({}));
expect(spectator.element).toHaveText('');
expect(pipe.transform({} as any)).toBe('');
});
it('should handle none number string', () => {
spectator = pipeFactory(getPipeTpl('none number string'));
expect(spectator.element).toHaveText('');
expect(pipe.transform('none number string')).toBe('');
});
});

Expand Down
15 changes: 15 additions & 0 deletions libs/transloco/src/lib/tests/pipe/pipe.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,21 @@ describe('TranslocoPipe', () => {
expect(pipe.transform('title', {})).toBe('');
});

describe('None transformable values', () => {
beforeEach(() => {
pipe = new TranslocoPipe(serviceMock, undefined, 'es', cdrMock);
});
it('should handle null', () => {
expect(pipe.transform(null)).toBeNull();
});
it('should handle undefined', () => {
expect(pipe.transform(undefined)).toBeUndefined();
});
it('should handle empty string', () => {
expect(pipe.transform('')).toBe('');
});
});

it('should use provided language', fakeAsync(() => {
spyOn(serviceMock, 'translate').and.callThrough();
pipe = new TranslocoPipe(serviceMock, undefined, 'es', cdrMock);
Expand Down
11 changes: 7 additions & 4 deletions libs/transloco/src/lib/transloco.pipe.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,18 @@ export class TranslocoPipe implements PipeTransform, OnDestroy {
this.scopeResolver = new ScopeResolver(this.service);
}

// null is for handling strict mode + async pipe types https://github.com/jsverse/transloco/issues/311
// null is for handling strict mode + optional chaining types https://github.com/jsverse/transloco/issues/488
// overloads for strict mode
transform(key: string, params?: HashMap, inlineLang?: string): string;
transform<T extends null | undefined>(key: T, params?: HashMap, inlineLang?: string): T;
transform<T extends null | undefined>(key: string | T, params?: HashMap, inlineLang?: string): string | T;

transform(
key?: string | null,
params?: HashMap,
inlineLang?: string
): string {
): string | null | undefined {
if (!key) {
return key as any;
return key;
}

const keyName = params ? `${key}${JSON.stringify(params)}` : key;
Expand Down
Loading