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

Change DefaultTFuncReturn to return null if returnNull typeOption is true #1865

Merged
merged 2 commits into from Nov 12, 2022

Conversation

pedrodurek
Copy link
Member

Closes i18next/react-i18next#1559, i18next/react-i18next#1576, i18next/react-i18next#1574

Since returnNull option is true by default, t function can return string or null. By declaring the resources type, we can infer the exact return type, so this is only useful to people who can't declare the resource type .

This PR allows to change this ☝️ behaviour if to set the returnNull type to false.

// i18next.d.ts
import 'i18next';

declare module 'i18next' {
  interface CustomTypeOptions {
    returnNull: false;
  }
}

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

@adrai
Copy link
Member

adrai commented Nov 11, 2022

tests are failing

@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.157% when pulling a83518f on pedrodurek:remove-null-if-returnNull-true into 26073c4 on i18next:master.

@adrai adrai merged commit 95489dd into i18next:master Nov 12, 2022
@adrai
Copy link
Member

adrai commented Nov 12, 2022

included in v22.0.5

@Jonnboy91
Copy link

Might be a dumb question but after this was included, it removed undefined as one of the possible returns of DefaultTFuncReturn, which in some cases gives an error then. For example TextInput requires either string | undefined in the placeholder, and with this update I'm getting this error:

Overload 1 of 2, '(props: TextInputProps | Readonly<TextInputProps>): TextInput', gave the following error. Type 'DefaultTFuncReturn' is not assignable to type 'string | undefined'. Type 'null' is not assignable to type 'string | undefined'.

This can be fixed by setting customTypeOptions returnNull: false, but was just wondering is that the preferred way instead of having also undefined as an option 🤔

@pedrodurek
Copy link
Member Author

pedrodurek commented Nov 14, 2022

Hey @Jonnboy91, t function can return null by default, if you don't want this behaviour set returnNull to false.

// i18next.d.ts
import 'i18next';

declare module 'i18next' {
  interface CustomTypeOptions {
    returnNull: false;
  }
}

@adrai can correct if I'm wrong, but t function will never return undefined, that's why I removed it from the union.

@Jonnboy91
Copy link

Hi @pedrodurek Thanks, this is what I thought and meant by "This can be fixed by setting customTypeOptions returnNull: false", but yeah it used to have the option of undefined and now that it was deleted some places where translation was used were giving errors now, but I could fix it by setting this customTypeOptions, so this was more of a question is the undefined coming back or will I just use this 💪 😄

@adrai
Copy link
Member

adrai commented Nov 15, 2022

@adrai can correct if I'm wrong, but t function will never return undefined, that's why I removed it from the union.

Yes, I'm not aware of any use case returning undefined by purpose...

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.

withTranslation - Type error: Type 'TFunctionResult' is not assignable to type 'ReactNode'
4 participants