i18next / react-i18next

Internationalization for react done right. Using the i18next i18n ecosystem.
https://react.i18next.com
MIT License
9.29k stars 1.03k forks source link

TS error with multiple namespace when passing the namespace as an option to t #1422

Closed danielo515 closed 1 year ago

danielo515 commented 2 years ago

๐Ÿ› Bug Report

I am referring to this comment:

https://github.com/i18next/react-i18next/issues/1396#issuecomment-961345092

@pedrodurek have you updated the TS docs to reflect that? I don't see any reference to that.

Also, I have a similar but slightly different scenario than on #1396 , and I want to know if it is supposed to be supported or not.

function App() {
  const { t } = useTranslation();
  return (
    <div className="App">
        <p>{t("key.on.ns1")}</p> {/*No TS error */}
        <p>{t("otherKey.present.inNs2", { ns: "ns2" })}</p> {/* TS error */}
    </div>
  );
}

As you can see, if I don't provide the namespace the tFunction is not able to understand that in the second case it should compare against the keys of ns2 and it fails, because it only compares agains keys on ns1. I can workaround this using to different t functions configuring each one for each namespace, so this works fine:

function App() {
  const { t } = useTranslation();
  const { t:t2 } = useTranslation('ns2');
  return (
    <div className="App">
        <p>{t("key.on.ns1")}</p> {/*No TS error */}
        <p>{t("otherKey.present.inNs2")}</p> {/*No TS error */}
    </div>
  );
}

To my understanding, and reading the docs, if you don't provide any namespace to the useTranslation function you can access any namespace using the { ns } option, but seems that such feature does not apply to TS?

Also, I see that is it not possible to provide more than one namespace and then use the { ns } option. If you provide two namespaces like useTranslation(['ns1','ns2']) the only way to use a namespace is with t('ns2:theKey'). Anything else will not typecheck. Is that expected?

Your Environment

pedrodurek commented 2 years ago

Hey @danielo515, interesting ๐Ÿค” , I did not know about the ns option or don't remember ๐Ÿ˜… . Until we move the types to the i18next repo, it may be a little harder to support it here, I'd also need to measure compilation time. For now, I'd recommend using the following approach:

const { t } = useTranslation(['ns1', 'ns2']);
t('ns1:key.on.ns1')
t('ns2:otherKey.present.inNs2')

I haven't updated the document yet.

danielo515 commented 2 years ago

Hello @pedrodurek , I got that option from this places: https://react.i18next.com/latest/usetranslation-hook#loading-namespaces https://react.i18next.com/guides/multiple-translation-files

And I think some other place I don't remember

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

danielo515 commented 2 years ago

Quite aggressive stale bot... Can this be unflagged?

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

danielo515 commented 2 years ago

Should I comment every 7 days to prevent this being closed artificially?

sebastienlabine commented 2 years ago

this. Also should support the fact that the first element in array is the default type so we only specify the namespace for the second namespace.

vricop commented 2 years ago

This is happening to me as well. My default space is 'common', if I try to change my default space but still have access to the 'common' namespace I got TypeScript errors. While the app translates everything properly tough.

I can access my namespaces like this:

t('common:signInButton')

But not like this

t('signInButton', { ns: 'common' })}

The react-i18next docs state this is the recommended way:

https://react.i18next.com/latest/usetranslation-hook#usetranslation-params

import { useTranslation } from 'react-i18next'
import { EmailBox } from '../common/EmailBox'
import { LanguageSelector } from '../common/LanguageSelector'
import { Logo } from '../common/Logo'
import {
  ButtonGroup,
  Content,
  Cta,
  Header,
  SignInButton,
  TagLine,
  Text,
  Title,
  TopBar,
} from './HeroHeader.styled'

export function HeroHeader() {
  /*
    'home' is now my default space, so no need to specify anything now, then 'common' isn't
    the default namespace anymore (as per this component context).
  */
  const { t } = useTranslation(['home', 'common'])

  return (
    <Header>
      <TopBar>
        <Logo />
        <ButtonGroup>
          <LanguageSelector />
          <SignInButton href="/signin">
            {/* It works: but got TypeScript error 'No overload matches this call.' */}
            {t('signInButton', { ns: 'common' })}
            {/* This also works but I got no errors */}
            {t('common:signInButton')}
          </SignInButton>
        </ButtonGroup>
      </TopBar>
      <Content>
        <Text>
          {/* Same here, TypeScript error */}
          <Title>{t('title')}</Title>
          {/* No errors... */}
          <Title>{t('home:title')}</Title>
          <TagLine>{t('tagline')}</TagLine>
          <Cta>{t('cta')}</Cta>
        </Text>
        <EmailBox />
      </Content>
    </Header>
  )
}
vricop commented 2 years ago

If I don't change my default space everything works, but I have to specify all keys for the 'home' namespace. Which kinda sucks...

import { useTranslation } from 'react-i18next'
import { EmailBox } from '../common/EmailBox'
import { LanguageSelector } from '../common/LanguageSelector'
import { Logo } from '../common/Logo'
import {
  ButtonGroup,
  Content,
  Cta,
  Header,
  SignInButton,
  TagLine,
  Text,
  Title,
  TopBar,
} from './HeroHeader.styled'

export function HeroHeader() {
  const { t } = useTranslation()

  return (
    <Header>
      <TopBar>
        <Logo />
        <ButtonGroup>
          <LanguageSelector />
          <SignInButton href="/signin">{t('signInButton')}</SignInButton>
        </ButtonGroup>
      </TopBar>
      <Content>
        <Text>
          <Title>{t('title', { ns: 'home' })}</Title>
          <TagLine>{t('tagline', { ns: 'home' })}</TagLine>
          <Cta>{t('cta', { ns: 'home' })}</Cta>
        </Text>
        <EmailBox />
      </Content>
    </Header>
  )
}

I just wanted to save some typing, as the most used namespace in this component is 'home', not 'common'.

volodymyr-strilets-mindcurv commented 2 years ago

Same issue here, the TS type-safe feature should be updated

example:

const { t } = useTranslation(['common', 'home'])

t('key') 
// โŒ TS error: No overload matches this call (but the 'common' namespace should be applied here by default)

t('common:key') 
// โœ… works fine

t('key', { ns: 'common' }) 
// โŒ the key 'common' should be type-safe, but it doesn't
// โŒ 'key' still has "No overload matches this call" error

@pedrodurek I know you are good in this type of issues ๐Ÿ˜‰

JarSeal commented 2 years ago

Same thing for me. Also the t('common:key') namespace syntax won't work with keys that have spaces:

t('common:Product name')
// โŒ Does not work, output is 'common:Product name'

However, this does not cause any TS errors (I think it should since it is just spitting out an unknown key and unknown / keys with typos should not build).

I came up with a workaround that is typesafe (not sure if I should use it though):

const t1 = useTranslation('ns1').t
const t2 = useTranslation('ns2').t
vricop commented 2 years ago

Same thing for me. Also the t('common:key') namespace syntax won't work with keys that have spaces:

t('common:Product name')
// โŒ Does not work, output is 'common:Product name'

However, this does not cause any TS errors (I think it should since it is just spitting out an unknown key and unknown / keys with typos should not build).

I came up with a workaround that is typesafe (not sure if I should use it though):

const t1 = useTranslation('ns1').t
const t2 = useTranslation('ns2').t

Follow JSON naming standards and you won't face those issues.

{
  "productName": "..."
}
t('common:productName')
JarSeal commented 2 years ago

Yes, I'm aware that using camelCase, snake_case, or some other non-spacing naming convention would help with this issue, but there are some great benefits to using english translation strings as keys:

I think JSON has several "standards" (or naming conventions) and allowing spaces and other special characters in keys is one of them.

vricop commented 2 years ago

Yes, I'm aware that using camelCase, snake_case, or some other non-spacing naming convention would help with this issue, but there are some great benefits to using english translation strings as keys:

* the code is way more readable / semantic

* the non-default languages will have a fallback text (in english) if the translation is missing

* the translators have a complete english translation ("Owners' car" vs. "ownersCar") always with the file, which makes the translation job easier if dealing only with the JSON files

I think JSON has several "standards" (or naming conventions) and allowing spaces and other special characters in keys is one of them.

That's true, there are many ways to format your JSON keys. But we're talking about JS, basically JSON stands for JavaScript Object Notation, and the standard way of naming things is camelCase then it makes sense. There's no real benefit, just the look of it. If the translation is missing in other languages other than the default one, it'll fallback to the default one by default. There's nothing wrong with ownersCar. The only important thing for translators it's inside the value not the key.

JarSeal commented 2 years ago

I have to disagree. Libraries like eslint and prettier also aim to change the "look of it". Don't you think there's a real benefit when the source is more readable and more semantic? Like in this case, showing the actual text content? Also, in my case the default language is not english, so falling back to a non-english language is not the best option here.

I also disagree that camelCase would be a standard. It is a common practice and a single convention, but not a standard. The react-i18next examples even have spaces on some of their keys so I don't think I'm completely on my own with this opinion and with the benefits. And while acknowledging the possible risks and troubles that this naming convention presents compared to eg. kebab-case or snake_case, I still truly believe, that having an actual text content as a key has real benefits and I don't think that restricting the key naming conventions to "non-spacing" keys is necessary.

jamuhl commented 2 years ago

I agree with both of you...use what ever suits you best...that's the flexibility i18next likes to give.

But - my personal preference - is not using natural language for keys out of following reasons:

C3ntraX commented 2 years ago

I have similar errors.

The access via "notification:" is not found with Typescript but is working.

 // Error no overload matched
 t("notification:error:loginFailed");

"typescript": "^4.6.3" "react-i18next": "^11.16.2",

import commonEN from "./en/common.json";
import notificationEN from "./de/notification.json";
import commonDE from "./de/common.json";
import notificationDE from "./de/notification.json";

export const defaultNS = "common";
export const resources = {
   en: {
      common: commonEN,
      notification: notificationEN,
   },
   de: {
      common: commonDE,
      notification: notificationDE,
   },
} as const;
import "react-i18next";
import { defaultNS, resources } from "../../i18n/config";

declare module "react-i18next" {
   interface CustomTypeOptions {
      defaultNS: typeof defaultNS;
      resources: typeof resources["de"];
   }
}

image

I think "notifications" should be listed too, but is not the case....

AzzouQ commented 2 years ago

In my case, I have multiple JSON files.

Let's say I have:

common.json:

"componentMenu": {
  "title": "Title",
  "action1": "Action 1",
  "action2": "Action 2",
}

and link.json:

"componentMenu": {
  "text": "Text",
  "href": "https://href"
}

In my composant, i would expect to be able to use something like

const { t } = useTranslation(['common', 'link'], { keyPrefix: 'componentMenu' });

t(''); // <- title, action1, action2, href, title

Weirdly, it can still be achieved when manually typing useTranslation:

const { t } = useTranslation<'common' | 'link', 'componentMenu'>(undefined, {
  keyPrefix: 'componentMenu',
});

<ComponentMenu title={t('title', { ns: 'common' })}>
  <ComponentMenu.Action action={t('action1', { ns: 'common' })} />
  <ComponentMenu.Action action={t('action2', { ns: 'common' })} />
  <Link href={t('href', { ns: 'link' })} label={t('text', { ns: 'link' })} />
</ComponentMenu>

I would expect that last example to be the default behavior as it works and is far more easy to work with from a dev POV.

Passing 2 ns to useTranslation in array seems to force the use of key-like common: or link:, while this is unecessary, since ns option is passed and it will also typed it like useTranslation<('common' | 'link')[]> which seems odd since I want either one of them, not an array containing both as a type.

Benrski commented 2 years ago

I found a possible solution that's working fine with react-i18next v11.

useTranslation expects ns?: N | Readonly<N>, so using as const will do the trick and namespace will be inferred correctly.

// โŒ Inferred namespace is ("ns1" | "ns2")[]
const { t } = useTranslation(['ns1', 'ns2']);
t('ns1:key.on.ns1');
t('ns2:otherKey.present.inNs2');

// โœ… Inferred namespace is Namespace<"ns1" | "ns2">
const { t } = useTranslation(['ns1', 'ns2'] as const);
t('ns1:key.on.ns1');
t('ns2:otherKey.present.inNs2');
volodymyr-strilets-mindcurv commented 2 years ago

@Benrski your solution loads all the namespaces (when you have more than ns1 and ns2), even if I specify just a couple of them

aaarichter commented 2 years ago
Screenshot 2022-09-16 at 21 33 53

I see this typing on useTranslation() when used locally. I'm not sure where I set common but this prevents me to set any additional namespace. I tried to overwrite it with defaultNS in the init but no luck :(

in another typescript project for odd reason common is not set and multiple namespace work just fine Screenshot 2022-09-16 at 22 01 56

edit

turns out I forgot the react-i18next.d.ts that looked the namespace ( https://react.i18next.com/latest/typescript )

adrai commented 2 years ago

Have you tried with the new i18next and react-i18next versions? https://www.i18next.com/overview/typescript

TheTimeWalker commented 1 year ago

@adrai It is still the same issue with react-i18next v12.0.0 and i18next 22.0.5

Lesik commented 1 year ago

I can confirm with i18next 22.0.7 and react-i18next 12.0.0 that the following does not work:

// app.en.json
{
  "Homepage": "Homepage",
  "Welcome text": "Welcome text",
  "Present in both": "Present in App"
}

// shop.en.json
{
  "Buy": "Buy",
  "Present in both": "Present in Shop"
}

// App.tsx
const { t } = useTranslation(["app", "shop"]);

<p>{t("app:Homepage")}</p> // works
<p>{t("shop:Buy")}</p> // works
<p>{t("Homepage", { ns: "app" })}</p> // type error, but should be supported
<p>{t("app:Welcome text")}</p> // no type error but doesn't show up in browser (notice the space in the translation key)

// App2.tsx
const { t } = useTranslation(["app"] as const);

<p>{t("Homepage", { ns: "app" })}</p> // works
<p>{t("Homepage", { ns: "shop" })}</p> // also works, but "shop" ns was never loaded, so should error

I think we should focus on properly typing the "ns syntax" ({ ns: "app" }) over the "namespace inside string" syntax ("app:Homepage"), because:

Also interesting trivia: t("Present in both") will return "Present in App" or "Present in Shop" depending on which namespace you specified first in the array you pass to the useTranslation hook. (As opposed to using the defaultNS specified in the config. Basically, the defaultNS simply acts as a fallback when you don't pass anything to useTranslation, but if you do, then the defaultNS doesn't have any effect.) In my opinion this is a bit dangerous, I feel like if you do specify multiple namespace to the useTranslation hook, then all calls to t() must have the ns argument passed as well, otherwise it's too unclear where the translation is coming from. If you specify only one namespace (or none, using the default, which is one), the ns argument should be illegal (never).

adrai commented 1 year ago

try with i18next v22.4.5

Lesik commented 1 year ago

@adrai Could you clarify what has changed in that version and what we should try? I don't really get it from looking at the commits before that release. Did that version add support for the { ns: "app" } syntax or the "app:..." syntax? Can we get rid of the as const or is it still necessary? Because I just tried all possible combinations of it and nothing seems to have changed.

adrai commented 1 year ago

In general the complete signature calls for the t function have been updated...t('key', { ns: 'app' }) and t('app:key') should both work

https://github.com/i18next/i18next/blob/master/test/typescript/custom-types/t.test.ts#L40

Lesik commented 1 year ago

Oh yeah, nice work! It will now also check whether key is actually part of ns. Though unfortunately if not, it doesn't underline the key but instead underlines the ns with the rather cryptic error message:

Argument of type '{ ns: string; }' is not assignable to parameter of type 'TOptionsBase & { ns: string; } & { defaultValue: string; }'. ย ย ย ย ย ย Property 'defaultValue' is missing in type '{ ns: string; }' but required in type '{ defaultValue: string; }'.


Apart from that, unfortunately it still doesn't check whether the namespace you specify to the t function is actually included in the loaded namespaces from the useTranslation hook. So you could do:

const { t } = useTranslation(["namespace1"])

return <p>{ t("key", { ns: "namespace2" }) }</p> // no error, but should be a type error because namespace2 is not loaded