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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

types(Trans): add typechecking on context prop #1732

Merged
merged 1 commit into from
Mar 7, 2024
Merged

types(Trans): add typechecking on context prop #1732

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 6 additions & 4 deletions TransWithoutContext.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,15 @@ type TransChild = React.ReactNode | Record<string, unknown>;
export type TransProps<
Key extends ParseKeys<Ns, TOpt, KPrefix>,
Ns extends Namespace = _DefaultNamespace,
TOpt extends TOptions = {},
KPrefix = undefined,
TContext extends string | undefined = undefined,
TOpt extends TOptions & { context?: TContext } = { context: TContext },
Copy link

@natoehv natoehv Mar 7, 2024 •
edited

Choose a reason for hiding this comment

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

I know this was closed, but if we remove the { context: TContext } it works much better, like:
TOpt extends TOptions & { context?: TContext } in this way the IDE TS autocomplete works:
without context assignation:
image
with context assignation:
image

Copy link

Choose a reason for hiding this comment

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

Does it still correctly handle the typings for the context property of the Trans component?

Copy link

Choose a reason for hiding this comment

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

I think so
image

Copy link

Choose a reason for hiding this comment

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

not exactly what I meant :D I meant more something like this:

image image

E = React.HTMLProps<HTMLDivElement>,
> = E & {
children?: TransChild | readonly TransChild[];
components?: readonly React.ReactElement[] | { readonly [tagName: string]: React.ReactElement };
count?: number;
context?: string;
context?: TContext;
defaults?: string;
i18n?: i18n;
i18nKey?: Key | Key[];
Expand All @@ -29,7 +30,8 @@ export type TransProps<
export function Trans<
Key extends ParseKeys<Ns, TOpt, KPrefix>,
Ns extends Namespace = _DefaultNamespace,
TOpt extends TOptions = {},
KPrefix = undefined,
TContext extends string | undefined = undefined,
TOpt extends TOptions & { context?: TContext } = { context: TContext },
E = React.HTMLProps<HTMLDivElement>,
>(props: TransProps<Key, Ns, TOpt, KPrefix, E>): React.ReactElement;
>(props: TransProps<Key, Ns, KPrefix, TContext, TOpt, E>): React.ReactElement;
26 changes: 13 additions & 13 deletions test/typescript/custom-types/Trans.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,25 +72,25 @@ describe('<Trans />', () => {
const { t } = useTranslation('alternate', { keyPrefix: 'foobar.deep' });

// <Trans t={t} i18nKey="deeper.deeeeeper">foo</Trans>
expectTypeOf<
typeof Trans<'deeper.deeeeeper', 'alternate', {}, 'foobar.deep'>
>().toBeCallableWith({
t,
i18nKey: 'deeper.deeeeeper',
});
expectTypeOf<typeof Trans<'deeper.deeeeeper', 'alternate', 'foobar.deep'>>().toBeCallableWith(
{
t,
i18nKey: 'deeper.deeeeeper',
},
);
});

it('should throw error with `t` function with key prefix and wrong `i18nKey`', () => {
const { t } = useTranslation('alternate', { keyPrefix: 'foobar.deep' });

// <Trans t={t} i18nKey="xxx">foo</Trans>
expectTypeOf<
typeof Trans<'deeper.deeeeeper', 'alternate', {}, 'foobar.deep'>
>().toBeCallableWith({
t,
// @ts-expect-error
i18nKey: 'xxx',
});
expectTypeOf<typeof Trans<'deeper.deeeeeper', 'alternate', 'foobar.deep'>>().toBeCallableWith(
{
t,
// @ts-expect-error
i18nKey: 'xxx',
},
);
});
});

Expand Down
74 changes: 60 additions & 14 deletions test/typescript/custom-types/TransWithoutContext.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { describe, it, expectTypeOf } from 'vitest';
import { describe, it, expectTypeOf, assertType } from 'vitest';
import * as React from 'react';
import { useTranslation } from 'react-i18next';
import { Trans } from '../../../TransWithoutContext';
Expand Down Expand Up @@ -73,25 +73,25 @@ describe('<Trans />', () => {
const { t } = useTranslation('alternate', { keyPrefix: 'foobar.deep' });

// <Trans t={t} i18nKey="deeper.deeeeeper">foo</Trans>
expectTypeOf<
typeof Trans<'deeper.deeeeeper', 'alternate', {}, 'foobar.deep'>
>().toBeCallableWith({
t,
i18nKey: 'deeper.deeeeeper',
});
expectTypeOf<typeof Trans<'deeper.deeeeeper', 'alternate', 'foobar.deep'>>().toBeCallableWith(
{
t,
i18nKey: 'deeper.deeeeeper',
},
);
});

it('should throw error with `t` function with key prefix and wrong `i18nKey`', () => {
const { t } = useTranslation('alternate', { keyPrefix: 'foobar.deep' });

// <Trans t={t} i18nKey="xxx">foo</Trans>
expectTypeOf<
typeof Trans<'deeper.deeeeeper', 'alternate', {}, 'foobar.deep'>
>().toBeCallableWith({
t,
// @ts-expect-error
i18nKey: 'xxx',
});
expectTypeOf<typeof Trans<'deeper.deeeeeper', 'alternate', 'foobar.deep'>>().toBeCallableWith(
{
t,
// @ts-expect-error
i18nKey: 'xxx',
},
);
});
});

Expand All @@ -118,4 +118,50 @@ describe('<Trans />', () => {
});
});
});

describe('usage with context', () => {
it('should work with default namespace', () => {
assertType<React.ReactElement>(<Trans i18nKey="some" context="me" />);

// @ts-expect-error should throw error when context is not valid
assertType<React.ReactElement>(<Trans i18nKey="some" context="one" />);
});

it('should work with `ns` prop', () => {
assertType<React.ReactElement>(<Trans ns="context" i18nKey="beverage" />);

assertType<React.ReactElement>(
// @ts-expect-error should throw error when context is not valid
<Trans ns="context" i18nKey="beverage" context="strawberry" />,
);
});

it('should work with default `t` function', () => {
const { t } = useTranslation();

assertType<React.ReactElement>(<Trans t={t} i18nKey="some" context="me" />);

// @ts-expect-error should throw error when context is not valid
assertType<React.ReactElement>(<Trans t={t} i18nKey="some" context="Test1222" />);
});

it('should work with custom `t` function', () => {
const { t } = useTranslation('context');

assertType<React.ReactElement>(<Trans t={t} i18nKey="dessert" context="cake" />);

// @ts-expect-error should throw error when context is not valid
assertType<React.ReactElement>(<Trans t={t} i18nKey="dessert" context="sake" />);
});

it('should work with `ns` prop and `count` prop', () => {
const { t } = useTranslation('plurals');
assertType<React.ReactElement>(<Trans ns="plurals" i18nKey="foo" count={2} />);
});

it('should work with custom `t` function and `count` prop', () => {
const { t } = useTranslation('plurals');
assertType<React.ReactElement>(<Trans t={t} i18nKey="foo" count={2} />);
});
});
});
12 changes: 12 additions & 0 deletions test/typescript/custom-types/i18next.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ declare module 'i18next' {
custom: {
foo: 'foo';
bar: 'bar';

some: 'some';
some_me: 'some context';
};

alternate: {
baz: 'baz';
foobar: {
Expand All @@ -22,13 +24,23 @@ declare module 'i18next' {
};
};
};

plurals: {
foo_zero: 'foo';
foo_one: 'foo';
foo_two: 'foo';
foo_many: 'foo';
foo_other: 'foo';
};

context: {
dessert_cake: 'a nice cake';
dessert_muffin_one: 'a nice muffin';
dessert_muffin_other: '{{count}} nice muffins';

beverage: 'beverage';
beverage_beer: 'beer';
};
};
}
}
26 changes: 14 additions & 12 deletions test/typescript/misc/Trans.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { describe, expectTypeOf, it } from 'vitest';
import { assertType, describe, expectTypeOf, it } from 'vitest';
import * as React from 'react';
import { Trans, useTranslation } from 'react-i18next';

Expand Down Expand Up @@ -95,14 +95,14 @@ describe('<Trans />', () => {
);
expectTypeOf(Trans).toBeCallableWith({ parent: CustomRedComponent, children: 'Foo' });

<Trans parent="div" style={{ color: 'green' }}>
Foo
</Trans>;
assertType<React.ReactElement>(
<Trans parent="div" style={{ color: 'green' }}>
Foo
</Trans>,
);

{
/* div is the default parent */
}
<Trans style={{ color: 'green' }}>Foo</Trans>;
/* div is the default parent */
assertType<React.ReactElement>(<Trans style={{ color: 'green' }}>Foo</Trans>);
});

it('should work with `tOptions`', () => {
Expand All @@ -124,9 +124,11 @@ describe('<Trans />', () => {
});

it('should not work with object child', () => {
<Trans>
{/* @ts-expect-error */}
<span>This {{ var: '' }} is an error since `allowObjectInHTMLChildren` is disabled</span>
</Trans>;
assertType<React.ReactElement>(
<Trans>
{/* @ts-expect-error */}
<span>This {{ var: '' }} is an error since `allowObjectInHTMLChildren` is disabled</span>
</Trans>,
);
});
});
2 changes: 2 additions & 0 deletions tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
{
"exclude": ["example/**/*"],

"compilerOptions": {
"module": "commonjs",
"target": "es5",
Expand Down