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

Linting of TFunction fails since the last update #1212

Closed
mxsxs2 opened this issue Dec 7, 2020 · 21 comments · Fixed by #1193
Closed

Linting of TFunction fails since the last update #1212

mxsxs2 opened this issue Dec 7, 2020 · 21 comments · Fixed by #1193

Comments

@mxsxs2
Copy link

mxsxs2 commented Dec 7, 2020

💥 Regression Report

Linting of TFunction fails since the last update. The t function should accept its key parameter to be string | string []. However, it seems to expect to be never.

K seems to resolve to never at

Last working version

Worked up to version: 11.7.4

Stopped working in version: 11.8.0

To Reproduce

Steps to reproduce the behavior:

import React from 'react'
import { useTranslation } from 'react-i18next/*'

const MyComponent= ()=>{
    const {t} = useTranslation()
    return <span>{t('hello')}</span>
}

export default MyComponent

Expected behavior

I expect to not have Argument of type 'string' is not assignable to parameter of type 'never'.ts(2769) at line 6 for t('hello')

Full error:

error TS2769: No overload matches this call.
Overload 1 of 2, '(key: never, options?: string | TOptions | undefined): never', gave the following error.
Argument of type 'string' is not assignable to parameter of type 'never'.
Overload 2 of 2, '(key: never, defaultValue?: string | undefined, options?: string | TOptions | undefined): never', gave the following error.
Argument of type 'string' is not assignable to parameter of type 'never'.

Your Environment

  • runtime version: node 14.15.1
  • i18next version: 11.8.0
  • TypeScript version: 4.1.2
  • os: Windows with Ubuntu WSL2
@paulrostorp
Copy link

same here:
Screenshot 2020-12-07 at 14 52 58

@adrai
Copy link
Member

adrai commented Dec 7, 2020

@pedrodurek can you have a look at this?

@JeroenReumkens
Copy link

JeroenReumkens commented Dec 7, 2020

Same here, was looking into failing builds for the past 30 mins. This only seems to be the case in the 4.1 typings, 4.0.x is fine. At least it helped me find out our CI uses a different TS version compared to my local environment 😅

Edit: Can confirm our CI works fine with 4.0.x again, until this issue is resolved.

@pedrodurek
Copy link
Member

pedrodurek commented Dec 7, 2020

sure 👀 @adrai

@adrai adrai linked a pull request Dec 7, 2020 that will close this issue
4 tasks
@goldmont
Copy link

goldmont commented Dec 7, 2020

Same here.

@pedrodurek
Copy link
Member

pedrodurek commented Dec 7, 2020

Hey guys, I'm still investigating it, but I managed to simulate the error using CRA. When I performed the tests for the first time, CRA wasn't supporting typescript 4.1 properly (facebook/create-react-app#9960), so I used pure webpack (https://github.com/pedrodurek/i18n-type-tests), which works perfectly.

I'll let you guys know as soon as I sort that out.

@adrai in the meantime, what do you suggest?
1 - I can create a PR disabling TS 4.1, which will fallback to the old type definitions.
2 - We can rollback the changes.
3 - Just wait until I find out the reason.

@adrai
Copy link
Member

adrai commented Dec 7, 2020

I'm not a TS user...
others have to say 🤷‍♂️
in the meantime option 3 😉

@tigerabrodi
Copy link
Contributor

If any help is needed, you know where to find me 🐅 🎉 😃

@JeroenReumkens
Copy link

@pedrodurek I'm using NextJS. They do support TS 4.1.
As an answer to your other question; Why is a separate typing needed for TS 4.1? I've actually never seen a library do this. So would there be a downside to using the default types? I tried to take a look as the 4.1 typings to spot any meaningful differences, but I found them a lot harder to decipher 😅

@JeroenReumkens
Copy link

Oh! I took a look at your example and noticed that the error does NOT occur when you pass a namespace to useTranslation. My implementation currently does not pass a namespace, which results in the error.

@pedrodurek
Copy link
Member

Hey guys, I managed to fix the issue. I'm gonna add two examples (with and without namespaces) under examples folder and open a PR.

@pedrodurek
Copy link
Member

It should be fixed now

@vtrphan
Copy link

vtrphan commented Dec 8, 2020

Hi, i'm still getting this error

No overload matches this call.
  Overload 1 of 2, '(key: Normalize<{ formattedDate: string; app: { title: string; }; loginPage: { titleUnauthenticated: string; titleAuthenticated: string; subheaderUnauthenticated: string; subheaderAuthenticated: string; ... 6 more ...; forgotPwdBtnLabel: string; }; homePage: { ...; }; nmvsUserPage: { ...; }; }>, options?: string | ... 1 more ... | undefined): string | ... 15 more ... | { ...; }', gave the following error.
    Argument of type 'string' is not assignable to parameter of type 'Normalize<{ formattedDate: string; app: { title: string; }; loginPage: { titleUnauthenticated: string; titleAuthenticated: string; subheaderUnauthenticated: string; subheaderAuthenticated: string; ... 6 more ...; forgotPwdBtnLabel: string; }; homePage: { ...; }; nmvsUserPage: { ...; }; }>'.
  Overload 2 of 2, '(key: Normalize<{ formattedDate: string; app: { title: string; }; loginPage: { titleUnauthenticated: string; titleAuthenticated: string; subheaderUnauthenticated: string; subheaderAuthenticated: string; ... 6 more ...; forgotPwdBtnLabel: string; }; homePage: { ...; }; nmvsUserPage: { ...; }; }>, defaultValue?: string | undefined, options?: string | ... 1 more ... | undefined): string | ... 15 more ... | { ...; }', gave the following error.
    Argument of type 'string' is not assignable to parameter of type 'Normalize<{ formattedDate: string; app: { title: string; }; loginPage: { titleUnauthenticated: string; titleAuthenticated: string; subheaderUnauthenticated: string; subheaderAuthenticated: string; ... 6 more ...; forgotPwdBtnLabel: string; }; homePage: { ...; }; nmvsUserPage: { ...; }; }>'.  TS2769

    55 |                        <ListItemIcon>{importanceIcon(sortingPriority)}</ListItemIcon>
    56 |                        <ListItemText
  > 57 |                                primary={t(translations.homePage.newsItem.primary, {....

Please advise

@pedrodurek
Copy link
Member

Hey @vtrphan, since it's not related to this issue, I'd advice you to open another one and provide more info. But in a nutshell, I can see that you're passing an object as key to the t function instead of a string.

@vtrphan
Copy link

vtrphan commented Dec 8, 2020

@pedrodurek Yes i'm using the approach/setup from react-boilerplate
https://github.com/react-boilerplate/react-boilerplate-cra-template
to make use of Intellisense (passing in an object instead of a string).
This used to work with react-i18next v11.7 and below, only report error in 11.8 and up

@pedrodurek
Copy link
Member

pedrodurek commented Dec 8, 2020

That's weird because even in the v11.7, it was only accepting string or TemplateStringsArray as key. Have you checked one of the examples here? We're using the last CRA version.
https://github.com/i18next/react-i18next/tree/master/example/react-typescript4.1

@adrianolsk
Copy link

@vtrphan I may be wrong, but looks like you are not using the i18n doing this t(translations.homePage.newsItem.primary, I think it's probably using the value from your object because it is not finding your key and using it as a fallback.
In my understanding you should be using like this t('homePage.newsItem.primary')

@vtrphan
Copy link

vtrphan commented Dec 8, 2020

@adrianolsk Hi, it is this approach/setup that i'm following:
https://cansahin.gitbook.io/react-boilerplate-cra-template/building-blocks/i18n
They're converting the translation json into object to take advantage of the intellisense feature

@mxsxs2
Copy link
Author

mxsxs2 commented Dec 9, 2020

Hey guys, I managed to fix the issue. I'm gonna add two examples (with and without namespaces) under examples folder and open a PR.

The PR fixed my issue. Thanks!

@jamuhl jamuhl closed this as completed Dec 9, 2020
@joelpoloney
Copy link

joelpoloney commented Dec 29, 2020

Has anyone run into the situation where you're swapping out the "base" of a key? I'm trying to use TypeScript template strings and this used to work but now requires extra TypeScript casting:

Before:

const i18nBase = isSubmitted ? 'path.to.something.resubmit' : 'path.to.something.submit'
const title = t(`${i18nBase}.title`)

Interpreting the string in this manor fails to compile with errors like No overload matches this call.. I can fix this by casting to a const with:

const i18nBase = isSubmitted ? 'path.to.something.resubmit' : 'path.to.something.submit'
const title = t(`${i18nBase}.title` as const)

but it feels like something that I shouldn't have to do. Is there a better way to do this? We exercise this pattern quite frequently in our codebase, especially for re-usable components which might be in different states and I was hoping to avoid having to rewrite them all.

Thanks!

@pedrodurek
Copy link
Member

Hey @joelpoloney, I believe I answered your question here: #1193 (comment)

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 a pull request may close this issue.