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

[TypeScript] Add default value for TDefaultResult #1853

Merged
merged 1 commit into from Oct 21, 2022

Conversation

pedrodurek
Copy link
Member

Closes #1852

Checklist

  • only relevant code is changed (make a diff before you submit the PR)
  • run tests npm run test
  • tests are included
  • commit message and code follows the Developer's Certification of Origin

Checklist (for documentation change)

  • only relevant documentation part is changed (make a diff before you submit the PR)
  • motivation/reason is provided
  • commit message and code follows the Developer's Certification of Origin

@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.157% when pulling 59c0fd4 on pedrodurek:fix-default-value-TDefaultResult into df759b2 on i18next:master.

@adrai adrai merged commit 0b74e8b into i18next:master Oct 21, 2022
@adrai
Copy link
Member

adrai commented Oct 21, 2022

thank you.... released with v22.0.2

@Erazihel
Copy link

Hey @adrai @pedrodurek, thanks for your amazing work 👍

I've updated i18next to v22.0.2 and I still have the following error (please have in mind that I'm using react-i18next):

TS2322: Type 'DefaultTFuncReturn' is not assignable to type 'ReactI18NextChild | Iterable<ReactI18NextChild>'.
  Type 'object' is not assignable to type 'ReactI18NextChild | Iterable<ReactI18NextChild>'.

75             <span>{t('foo')}</span>

I tried to follow this documentation but since I have no workspace, I don't really now how to make it work 😅

I also tried to use as const, still no luck 😕 (I must admit that if this would have fixed it, I'm not really feeling confortable adding as const for all the translations since I have 500+ of them...)

package.json:

"i18next": "22.0.2",
"react-i18next": "12.0.0",

@adrai
Copy link
Member

adrai commented Oct 21, 2022

@Erazihel until @pedrodurek is back online, you may try to provide a minimal reproducible example showing the issue.

@chybisov
Copy link

chybisov commented Oct 21, 2022

Hey @adrai @pedrodurek, thanks for your amazing work 👍

I've updated i18next to v22.0.2 and I still have the following error (please have in mind that I'm using react-i18next):

TS2322: Type 'DefaultTFuncReturn' is not assignable to type 'ReactI18NextChild | Iterable<ReactI18NextChild>'.
  Type 'object' is not assignable to type 'ReactI18NextChild | Iterable<ReactI18NextChild>'.

75             <span>{t('foo')}</span>

I tried to follow this documentation but since I have no workspace, I don't really now how to make it work 😅

I also tried to use as const, still no luck 😕 (I must admit that if this would have fixed it, I'm not really feeling confortable adding as const for all the translations since I have 500+ of them...)

package.json:

"i18next": "22.0.2",
"react-i18next": "12.0.0",

image

I have a similar issue with @mui and Vite.

@jderusse
Copy link

I have a similar issue:

  • thousand of calls to label={t('label')} => adding as const everywhere sounds a huge change
  • hundred of dynamic json files => I can't follow the documentation and declare a Type with known Resources.

Is there a way to disable it and to say everything is a string like

// /!\ this does not work /!\ 
declare module "i18next" {
  interface CustomTypeOptions {
    resources: {
      [key: string]: string | undefined;
    };
  }
}

@Erazihel
Copy link

Erazihel commented Oct 21, 2022

Hey @adrai 👋

I tried to reproduce the issue on a CodeSandbox without any luck. Everything works fine even though I tried to have a Sandbox as close as my current project.

I don't know why but it seems that in my project the DefaultTFuncReturn gets resolved as an object whereas in the CodeSandbox it gets resolved as a string... 😅

I guess it has something to do with the TFunction interface:

export interface TFunction<N extends Namespace = DefaultNamespace, TKPrefix = undefined> {
  <
    TKeys extends TFuncKey<N, TKPrefix> | TemplateStringsArray extends infer A ? A : never,
    TDefaultResult extends DefaultTFuncReturn = string,
    TInterpolationMap extends object = StringMap,
  >(
    key: TKeys | TKeys[],
  ): TFuncReturn<N, TKeys, TDefaultResult, TKPrefix>;
  <
    TKeys extends TFuncKey<N, TKPrefix> | TemplateStringsArray extends infer A ? A : never,
    TDefaultResult extends DefaultTFuncReturn = object,
    TInterpolationMap extends object = StringMap,
  >(
    key: TKeys | TKeys[],
    options?: TOptions<TInterpolationMap> & { returnDetails: true; returnObjects: true },
  ): TFunctionDetailedResult<TFuncReturn<N, TKeys, TDefaultResult, TKPrefix>>;
...

I managed to force the type using t<string>('foo') but again, since I have 500+ keys on my project, this is not maintainable and I won't migrate to v22.0.x for the moment.

Hoping that someone manages to reproduce this behaviour or that you have an idea where this could come from and a solution to this.

Cheers!

@piotrponikowski
Copy link

piotrponikowski commented Oct 21, 2022

Hi!
I am also facing typescript issue, and was able to reproduced this in new project: https://github.com/piotrponikowski/i18next-ts-issue

I face typings issue when using library like this (see App.tsx):

      <div>{t('translation.key')}</div>

Workarounds:

      <div>{t('translation.key') as string}</div>
      <div>{`${t('translation.key')}`}</div>

PS. I know example doesn't contain i18next initialization, however wanted to use minium number of steps to reproduce - please let me know if more info is needed

@adrai
Copy link
Member

adrai commented Oct 21, 2022

Take also a look at this: #1852

@chybisov
Copy link

chybisov commented Oct 21, 2022

Take also a look at this: #1852

import 'i18next';
import en from './en.json';

const defaultResource = { translation: en };

declare module 'i18next' {
  interface CustomTypeOptions {
    defaultNS: 'translation';
    resources: typeof defaultResource;
  }
}

We have the Vite React app with @mui, and in dev mode with esbuild everything works fine, but I have Type 'DefaultTFuncReturn' is not assignable to type 'ReactNode'. issue when trying to build the app, Vite uses Rollup for this.

@pedrodurek
Copy link
Member Author

In React 18, they change the React type declaration to stop accepting object in JSX elements. Right now, there is a limitation for people who opt out following the instructions here https://www.i18next.com/overview/typescript, I'll see what I can do. In the meantime, I encourage everyone to follow the instructions in the document. Later today, I'll update i18next and react-i18next examples.
Here is a working example based on @piotrponikowski project.

@jderusse
Copy link

Is it possible to get an example with a project like https://github.com/locize/react-tutorial/blob/main/step_4/src/i18n.js or https://github.com/locize/react-tutorial/blob/main/step_0/src/i18n.js

@piotrponikowski
Copy link

How to handle case with translation files fetched from remote server? I don't need typed keys for translations, but only looking for way to return string from t function of useTranslation. Anyone was able to find working configuration for such case?

@adrai
Copy link
Member

adrai commented Oct 24, 2022

@pedrodurek I'm not a TypeScript user, but maybe changing this here in react-i18next would help?

- type ReactI18NextChild = React.ReactNode | ObjectOrNever;
+ type ReactI18NextChild = React.ReactNode | DefaultTFuncReturn | ObjectOrNever;

Issue shown in example based on your i18next-ts-issue repo: pedrodurek/i18next-ts-issue@main...adrai:i18next-ts-issue:main

@NoZiL
Copy link

NoZiL commented Oct 24, 2022

Hey @adrai @pedrodurek, thanks for your amazing work 👍

I've updated i18next to v22.0.2 and I still have the following error (please have in mind that I'm using react-i18next):

TS2322: Type 'DefaultTFuncReturn' is not assignable to type 'ReactI18NextChild | Iterable<ReactI18NextChild>'.
  Type 'object' is not assignable to type 'ReactI18NextChild | Iterable<ReactI18NextChild>'.

75             <span>{t('foo')}</span>

I tried to follow this documentation but since I have no workspace, I don't really now how to make it work 😅

I also tried to use as const, still no luck 😕 (I must admit that if this would have fixed it, I'm not really feeling confortable adding as const for all the translations since I have 500+ of them...)

package.json:

"i18next": "22.0.2",
"react-i18next": "12.0.0",

Hey @Erazihel :p
You have to use the declaration file => https://react.i18next.com/latest/typescript.
But where the documentation is wrong is that you have to declare module 'i18next' instead of declare module 'react-i18next'.

@Erazihel
Copy link

Hey @NoZiL 😊
The documentation is actually right since it mentions the following:

We recently moved all type enhancements described here to i18next, so if you are relying on i18next@v22.0.0 and react-i18next@v12.0.0, please ignore this section and follow the instructions here.

The link says to use declare module "i18next" as you mentioned but this was already what I tried when I first commented this PR and wasn't able to make it work 😕

@adrai
Copy link
Member

adrai commented Oct 26, 2022

@pedrodurek I'm not a TypeScript user, but maybe changing this here in react-i18next would help?

- type ReactI18NextChild = React.ReactNode | ObjectOrNever;
+ type ReactI18NextChild = React.ReactNode | DefaultTFuncReturn | ObjectOrNever;

Issue shown in example based on your i18next-ts-issue repo: pedrodurek/i18next-ts-issue@main...adrai:i18next-ts-issue:main

@pedrodurek this really seems to still be a problem, can you check if my suggested change is correct?

@vitharanagedonjani-i2e
Copy link

vitharanagedonjani-i2e commented Oct 26, 2022

Declaring it like this worked for me.

// References: https://www.i18next.com/overview/typescript

import "i18next";

declare module 'i18next' {
  interface CustomTypeOptions {
    // custom namespace type, if you changed it
    defaultNS: "common";
    // custom resources type
    resources: {
      nl: Record<string, string>;
      en: Record<string, string>;
      de: Record<string, string>;
      fr: Record<string, string>;
    };
  }
}

@adrai
Copy link
Member

adrai commented Oct 27, 2022

can you try with i18next v22.0.3?

@chybisov
Copy link

@adrai works for me, thank you! 🙏

@Erazihel
Copy link

@adrai Just tested and everything works fine once again! Thank you so much! 🙏

@piotrponikowski
Copy link

Looks like it is working also for me, thank you! ❤️

@vitharanagedonjani-i2e
Copy link

@adrai it worked thank you!

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.

Add documentation for the new breaking change in TS
9 participants