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

Build has Error by Type Type 'string' is not assignable to type 'string[]', but enable returnObjects #1217

Closed
Him-2C opened this issue Dec 8, 2020 · 20 comments

Comments

@Him-2C
Copy link

Him-2C commented Dec 8, 2020

🐛 Bug Report

Has error on 11.8.1 or newer, but 11.7.3 it's work!

Screen Shot 2563-12-09 at 00 56 31

Your Environment

  • *runtime: node v14, nextjs
  • react-i18next version: ^11.8.0
  • os: Mac
@adrai
Copy link
Member

adrai commented Dec 8, 2020

//cc @pedrodurek

@pedrodurek
Copy link
Member

pedrodurek commented Dec 8, 2020

Hey @Him-2C, I'll change the default TFunctionResult to accept other types. Right now, if you rely on the type augmentation as in this example here https://github.com/i18next/react-i18next/blob/master/example/react-typescript4.1/namespaces/%40types/react-i18next/index.d.ts, your problem should be gone, because it'll automatically infer the keys and the return type for you.

@Him-2C
Copy link
Author

Him-2C commented Dec 8, 2020

@pedrodurek Thank, but I don't use DefaultResources.

I set resource by this
image

@pedrodurek
Copy link
Member

Hey @Him-2C, it should be fixed now in v11.8.2.

@Him-2C
Copy link
Author

Him-2C commented Dec 9, 2020

I have to try build again, but not work. It seems the new type with object.

image

@john-d-pelingo
Copy link

Hello! o/

I am new to this and my question would be why is the t now returning an object as well? Shouldn't it just be a string or null like before?

@BenJenkinson
Copy link

BenJenkinson commented Dec 9, 2020

Hi @pedrodurek,

The fix you made has introduced a new problem for me.

Problem

The following code does not work, and produces a type/build error.

const { t } = useTranslation("exampleNamespace");

const foo: string = t("exampleKey");
Type 'TFunctionResult' is not assignable to type 'string'.
  Type 'undefined' is not assignable to type 'string'.

Cause?

I think the TFunctionResult type you're importing from i18next represents all possible return types, and not what it actually would be. The default return type is TResult, which must extend from TFunctionResult, but is defaulted to string.

https://github.com/i18next/i18next/blob/ba564b3d1f8c66f1323396123378f42ab3225593/index.d.ts#L653-L675

Documentation

According to the documentation (here), the t(..) function should always return a string, since it fallback to returning the requested key if no translation was found.

If a key does not return a value the key acts as fallback:

i18next.t('notExistingKey'); // -> "notExistingKey"

Though it looks like this can be complicated by some of the configuration options like returnNull and returnObjects.

For example, if the translation key exists in the resource, but is null. Depending on the configuration value of returnNull, t(..) should return either string (non-null translation or key) or string | null (non-null translation, or null translation, or key)

pedrodurek added a commit to pedrodurek/react-i18next that referenced this issue Dec 9, 2020
@pedrodurek
Copy link
Member

Hey guys, actually it was returning TFunctionResult (string | object | Array<string | object> | undefined | null) before. But the return type was generic extending from TFunctionResult, which allows typescript to automatically infer according to what you need (within TFunctionResult).
I didn't predict that. Sorry guys, It was my mistake. I'm creating a PR that fixes that.

@BenJenkinson
Copy link

Hi @pedrodurek, thanks for responding so quickly! :)

I've copied your changes from pedrodurek/react-i18next@766103d2 into my local project and I still don't think this is working for me (in every way that useTranslation used to work in 1.7.3)

For example, if I make some simple components like this:

interface IMessageProps {
  value: string;
}
const Message = (props: IMessageProps) => {
  return <span>{props.value}</span>;
};
const WorkingMessage = () => {
  const { t } = useTranslation('exampleNamespace');

  // ✅ This works
  return <Message value={t('exampleKey')}/>;
};

const BrokenMessage = () => {
  const { t } = useTranslation('exampleNamespace');

  const message = t('exampleKey');

  // ❌ This throws a type/build error
  return <Message value={message} />;
};

It causes this type error:

Type 'TFunctionResult' is not assignable to type 'string'.
  Type 'undefined' is not assignable to type 'string'.

@pedrodurek
Copy link
Member

pedrodurek commented Dec 9, 2020

@BenJenkinson thanks for flagging that, yeah I didn't open the PR yet because I'm figuring out a way to default string considering the new types, right now it's defaulting TFunctionResult. In your case, it'd work like this:

const message: string = t('exampleKey');

which would infer based on the message type

@BenJenkinson
Copy link

BenJenkinson commented Dec 9, 2020

@pedrodurek, does this work?

export type TFuncReturn<TResult extends TFunctionResult = string, N, P, T = Resources> = N extends (keyof T)[]
  ? NormalizeMultiReturn<T, P>
  : N extends keyof T
  ? NormalizeReturn<T[N], P>
  : TResult;

export interface TFunction<TResult extends TFunctionResult = string, N extends Namespace = DefaultNamespace> {
  <
    K extends TFuncKey<N> | TemplateStringsArray, 
    R extends TFuncReturn<TResult, N, K>,
    I extends object = StringMap 
  >(
    key: K | K[],
    options?: TOptions<I> | string,
  ): R;
  <
    K extends TFuncKey<N> | TemplateStringsArray, 
    R extends TFuncReturn<TResult, N, K>,
    I extends object = StringMap
  >( 
    key: K | K[],
    defaultValue?: string,
    options?: TOptions<I> | string,
  ): R;
}

That whole file has more complicated types than I've had to write before, so this might not be correct, but it seems to solve my immediate problem. It allows TResult to be anything from TFunctionResult, but also defaults it to string

@pedrodurek
Copy link
Member

@BenJenkinson Thanks!!! I was trying a similar approach, it seems so, I'm just testing with all scenarios (in particular augmenting the DefaultResoures to see if it's inferring the appropriate Return Type).

@BenJenkinson
Copy link

BenJenkinson commented Dec 9, 2020

@pedrodurek Great 👍 I wouldn't be using or testing the type inference from an augmented Resource since my translations are loaded from the server using i18next-http-backend. I don't have an object of translations in the source.

@pedrodurek
Copy link
Member

Is the backend in the same repo? If so, it's possible.

pedrodurek added a commit to pedrodurek/react-i18next that referenced this issue Dec 9, 2020
@pedrodurek
Copy link
Member

Hey @BenJenkinson, I had to change the approach because the generic TResult must be on the t function, otherwise, it won't infer the type automatically and it'll always default string. So it wouldn't work on this case:

const message: string[] = t('exampleKey');

When you have the chance, could you try with the fix here? pedrodurek@0fb1140

pedrodurek added a commit to pedrodurek/react-i18next that referenced this issue Dec 9, 2020
pedrodurek added a commit to pedrodurek/react-i18next that referenced this issue Dec 9, 2020
@BenJenkinson
Copy link

BenJenkinson commented Dec 9, 2020

Hi @pedrodurek,

Is the backend in the same repo? If so, it's possible.

I don't think that would work easily for us, our translations can be dynamic.

When you have the chance, could you try with the fix here? pedrodurek@0fb1140

That commit seems to no longer on a branch, so I've tested the file at pedrodurek@24b1ee4 which seems to be your latest commit.

const a = t('exampleKey'); // ✅ string

const b: string[] = t('exampleKey'); // ✅ string[]

const c: string | null = t('exampleKey'); // ✅ string | null

const d: string | { foo: 'bar' } | undefined = t('exampleKey'); // ✅ string | { foo: 'bar' } | undefined

const e: string = t(['foo', 'bar']); // ✅ string

const f: string[] = t(['foo', 'bar']); // ✅ string[]

That certainly builds now, and seems to be working OK. Thanks! 🥳

@BenJenkinson
Copy link

Hi @Him-2C,

Would you check if your problem is also resolved by the new version 11.8.3? (and close this issue if it does) 🙂

@Him-2C
Copy link
Author

Him-2C commented Dec 10, 2020

@BenJenkinson @pedrodurek
Thank Gods!

So now, all me it's work!.

@Him-2C Him-2C closed this as completed Dec 10, 2020
@onionhammer
Copy link

Works here too, thanks for all the good work guys!

@john-d-pelingo
Copy link

Works for me now as well. You folks rock!

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

No branches or pull requests

6 participants