i18next / next-i18next

The easiest way to translate your NextJs apps.
https://next.i18next.com
MIT License
5.51k stars 762 forks source link

Next.js 10 #869

Closed martpie closed 3 years ago

martpie commented 3 years ago

Is your feature request related to a problem? Please describe.

Next.js is just out, and introduces i18n routing primitives

https://nextjs.org/docs/advanced-features/i18n-routing

Describe the solution you'd like

I think a lot of the features of next-i18next can be dropped, so next-i18next really just focuses on the i18next integration.

Describe alternatives you've considered

n/a

Additional context

n/a

dcporter44 commented 3 years ago

So it really doesn't look like there's an out-of-the-box way to customly handle what language Next i18n uses. It simply parses the Accept-Language and moves on.

There is an open PR to more appropriately handle regional fallbacks. For example, automatic fallback from en-GB to en. See here: https://github.com/vercel/next.js/issues/18676 This issue has been added to a Vercel milestone set to be complete early January.

Obviously, this PR doesn't solve the issue of being able to handle language fallback ourselves. But it does fall more in line with i18next's default fallback functionality.

Next delegates the locale route at the highest level - prior to even reaching the _app.js component. At the server level, you could theoretically transform the incoming request header for Accept-Language before the request hits Next, but obviously this isn't a strategy that can be used with static apps.

fromi commented 3 years ago

I have an very basic integration of i18next and next.js 10 on a project in production (https://game-park.com/ if you want to have a look).

  // in _app.tsx
  const {locale} = useRouter()
  const i18n = useMemo(() => createI18nInstance(locale), [])
  useEffect(() => {
    if (locale) {
      i18n.changeLanguage(locale).catch(error => console.error('Error while changing i18n language', error))
    }
  }, [locale])
  return (
    <I18nextProvider i18n={i18n}>
    [...]
    </I18nextProvider>
  )

with createI18nInstance.tsx:

import i18next from 'i18next'
import {initReactI18next} from 'react-i18next'
import translations from './translations.json'

export default function createI18nInstance(language = 'en') {
  const i18n = i18next.createInstance({
    lng: language,
    fallbackLng: 'en',
    resources: translations
  })
  i18n.use(initReactI18next).init().catch(error => console.error('Error during i18next instance creation', error))
  return i18n
}

By using i18next.createInstance, I am able to generate static pages at build time for each language. Of course this configuration is quite simple since I do not have XHR backend, but it might be useful to some of you.

kylekirkby commented 3 years ago

I also have used react-i18next/i18next for i18n. Next.js v10 with i18n routing doesn't support SSG so I've built a starter that doesn't use the i18n routing capabilities.

https://github.com/linaro-marketing/serverless-nextjs-i18n-ssg-ssr-starter

With this starter I can deploy to my own infrastructure using Serverless-nextjs, statically optimize pages, and also have the ability to use the SSR functionality.

I really do hope that Next.js v10 gets SSG support added soon!

dcporter44 commented 3 years ago

I wanted to see what it would be like to just use react-i18next with Next 10's i18n functionality. This is about as minimal of a working example I could create that also simulates next-i18next's namespace loading. It also allows use of the useTranslation hook rather than using providers or HOC.

next.config.js

module.exports = {
  i18n: {
    locales: ['en', 'de', 'es'],
    defaultLocale: 'en'
  }
};

i18n.js

import i18next from 'i18next';
import { initReactI18next } from 'react-i18next';

export const i18nextInit = (router, namespaces = ['default']) => {
  if (!i18next.isInitialized) {
    i18next.use(initReactI18next).init({
      lng: router.locale,

      supportedLngs: router.locales,

      fallbackLng: router.defaultLocale,

      react: {
        useSuspense: false
      },

      resources: {}
    });
  }

  if (i18next.language !== router.locale) {
    i18next.changeLanguage(router.locale);
  }

  namespaces.forEach(ns => {
    if (!i18next.hasResourceBundle(router.locale, ns)) {
      const data = require(`./content/locales/${router.locale}/${ns}.json`);
      i18next.addResourceBundle(router.locale, ns, data);
    }
  });

  i18next.setDefaultNamespace(namespaces[0]);
};

_app.js

import '../styles/globals.css';
import { i18nextInit } from '../i18n.js';
import { useRouter } from 'next/router';

export default function MyApp({ Component, pageProps }) {
  const router = useRouter();

  i18nextInit(router, pageProps.i18nNamespaces);

  return <Component {...pageProps} />;
}

index.js

import { useTranslation } from 'react-i18next';

export default function IndexPage(props) {
  const { t } = useTranslation();

  return (
    <div>
      <p>{t('welcome_msg')}</p>
      <p>{t('secondary:hello_world')}</p>
    </div>
  );
}

export async function getStaticProps() {
  return {
    props: {
      i18nNamespaces: ['default', 'secondary']
    }
  };
}
TheFullResolution commented 3 years ago

@dcporter44 I used your example and it works. I did some tweaks to support the project I am working on (we use locale like en-NL, nl-NL, but load a single translation file per language).

I hardcode the languages and extract lang code from locale string:

export const i18nextInit = (router: NextRouter, namespaces = ['common']) => {
    const lng = getLangFromLocale(router.locale!);

    if (!i18next.isInitialized) {
        i18next.use(initReactI18next).init({
            lng,
            supportedLngs: ['nl', 'en', 'be'],
            fallbackLng: 'en',
            react: {
                useSuspense: false,
            },

            resources: {},
        });
    }

    if (i18next.language !== lng) {
        i18next.changeLanguage(lng);
    }

    namespaces.forEach((ns) => {
        if (!i18next.hasResourceBundle(lng, ns)) {
            const data = require(`../../public/locales/${lng}/${ns}.json`);
            i18next.addResourceBundle(lng, ns, data);
        }
    });

    i18next.setDefaultNamespace(namespaces[0]);
};

here is a naive extract function:

export const getLangFromLocale = (currentLocale: string) => {
    return currentLocale.split(/[-_]/)[0];
};
dcporter44 commented 3 years ago

@TheFullResolution Yeah that should work. But it's important to note that Next will only pass through a locale to the router if the locale is in the locales config option in next.config.js. So if your goal is to fallback all "nl-*" locales to "nl", this implementation won't work. You would theoretically have to pass every possible "nl-" locale to the locales config option in order for it to be passed through to your i18nextInit function (or else Next will pass through the defaultLocale).

Even if you did add all "nl-*" locales to the locales config option, it's important to note that the URL would not redirect to "/nl/page". For example, if Next detects "nl-NL" from the Accept-Language header and you added "nl-NL" to your locales config array, the url would appear as "/nl-NL/page". Next will NOT redirect to "/nl/page".

TheFullResolution commented 3 years ago

@dcporter44 Yes, I am aware of it. And in the case of this project, it is even desirable, as there is a different offer showed base on the country, so we do need to have nl-NL or nl-BE, while loading just one language JSON (nl)

kachkaev commented 3 years ago

@dcporter44 interesting! What about production builds and client-side routing to a page with additional namespaces? I'm curious how this part of your solution can handle this:

    namespaces.forEach((ns) => {
        if (!i18next.hasResourceBundle(lng, ns)) {
            const data = require(`../../public/locales/${lng}/${ns}.json`); // ๐Ÿ‘ˆ ๐Ÿ‘€
            i18next.addResourceBundle(lng, ns, data);
        }
    });
mikeyrayvon commented 3 years ago

I have a working implementation with Prismic CMS based on @dcporter44's example here in case that is useful to anyone!

dcporter44 commented 3 years ago

@kachkaev @mikeyrayvon Ah, so the implementation I wrote above doesn't seem to allow Next to use its Automatic Static Optimization. This is most likely because we are dynamically importing server-side files. In my case, my locale files were in a folder called content/locales. So Next is only using SSR and not SSG in my implementation.

This behavior seems to be undocumented. The docs say that SSG will only be prevented if using getServerSideProps or getInitialProps... which I'm not even using. Regardless, I guess I can understand why dynamically loaded files prevent SSG.

I've just implemented a better solution by loading the translations in getStaticProps instead of loading translations dynamically in _app.js like I was before. Now when I run npm run build, the pages are now built as .html files. This means that they are now successfully being optimized as static pages. My build folder now actually has generated static HTML files for each language for each page which is really cool.

Anyway, here is what I've come up with instead:

next.config.js

module.exports = {
  i18n: {
    locales: ['en', 'de', 'es'],
    defaultLocale: 'en'
  }
};

i18n.js

import i18next from 'i18next';
import { initReactI18next } from 'react-i18next';

export const i18nextInit = async (router, i18nResources) => {
  if (!i18nResources) {
    return;
  }

  const { translations, namespaces } = i18nResources;

  if (!i18next.isInitialized) {
    i18next.use(initReactI18next).init({
      lng: router.locale,

      supportedLngs: router.locales,

      fallbackLng: router.defaultLocale,

      ns: namespaces,

      react: {
        useSuspense: false
      }
    });
  }

  i18next.setDefaultNamespace(namespaces[0]);

  if (i18next.language !== router.locale) {
    i18next.changeLanguage(router.locale);
  }

  namespaces.forEach(ns => {
    if (!i18next.hasResourceBundle(router.locale, ns)) {
      i18next.addResourceBundle(router.locale, ns, translations[ns]);
    }
  });
};

export async function getTranslations(locale, namespaces) {
  const translations = {};

  for (let i = 0; i < namespaces.length; i++) {
    const ns = namespaces[i];

    const data = require(`./content/locales/${locale}/${ns}.json`);

    translations[ns] = data;
  }

  return { translations, namespaces };
}

_app.js

import '../styles/globals.css';
import { i18nextInit } from '../i18n.js';
import { useRouter } from 'next/router';

export default function MyApp(props) {
  const { Component, pageProps } = props;

  const router = useRouter();

  i18nextInit(router, pageProps.i18nResources);

  return <Component {...pageProps} />;
}

index.js

import { useTranslation } from 'react-i18next';
import { getTranslations } from '../i18n';

export default function IndexPage(props) {
  const { t } = useTranslation();

  return (
    <div>
      <p>{t('welcome_msg')}</p>
      <p>{t('secondary:hello_world')}</p>
    </div>
  );
}

export async function getStaticProps({ locale }) {
  return {
    props: {
      i18nResources: await getTranslations(locale, ['default', 'secondary'])
    }
  };
}
alexandrepaiva-dev commented 3 years ago

@dcporter44 thanks for sharing the code, I'm using here and the translation is working well, but when I click on the Nextjs Link component to change the language I get a console warning (the translation change works normally though):

Warning: Cannot update a component (`Home`) while rendering a different component (`MyApp`). To locate the bad setState() call inside `MyApp`, follow the stack trace as described in https://reactjs.org/link/setstate-in-render
    at MyApp (webpack-internal:///./src/pages/_app.jsx:32:24)

My _app.jsx:

function MyApp({ Component, pageProps }) {
  const contextValue = useGlobalContextValue();

  const router = useRouter();

  i18nextInit(router, pageProps.i18nResources);

  return (
    <GlobalContext.Provider value={contextValue}>
      <Component {...pageProps} />
    </GlobalContext.Provider>
  );
}

How I'm using the Link component:

<li key={locale}>
  <Link href="/" locale={locale}>
    <a className={router.locale === locale ? styles.selected : ""}>
      {locale}
    </a>
  </Link>
</li>

It doesn't happen with normal Link component (to navigate to another page, for example), it's only when changing the language. Any idea why? Thanks!!

dcporter44 commented 3 years ago

@alexandrepaivaa Thanks for bringing this up. I'm actually seeing this too. Everything seems to still work as expected regardless of the warning. I haven't had time to dig into this yet. I'll look into it and report back here.

dcporter44 commented 3 years ago

@alexandrepaivaa So I did a little research and found this issue (#1124) on the react-i18next repo. Some users in the thread seem to be getting the same warning when using the useTranslation hook. There's not really any clear solution in the issue. But it seems to be something to do with the changeLanguage function triggering a re-render within useTranslation.

In the meantime, I was able to get rid of the warning by removing useTranslation from my code, and instead just using i18next.t. I don't believe there's any real benefit to using the useTranslation hook in our use case. I could be wrong. But i18next.t seems to work fine. Maybe someone else in this thread will have better insight into any potential shortcomings with this implementation, but it seems to work just fine and no warnings:

index.js:

import i18next from 'i18next';
import { getTranslations } from '../i18n';

export default function IndexPage(props) {
  return (
    <div>
      <p>{i18next.t('welcome_msg')}</p>
      <p>{i18next.t('secondary:hello_world')}</p>
    </div>
  );
}

export async function getStaticProps({ locale }) {
  return {
    props: {
      i18nResources: await getTranslations(locale, ['default', 'secondary'])
    }
  };
}

But I guess this actually begs a bigger question of "how badly do we even need react-i18next"? Could we not just use the i18next library by itself? I understand that react-i18next gives us access to React-specific components, hooks, and HOC's, but if the above implementation is all you need, then maybe i18next by itself is enough. Depends on your use case I guess. But I'd assume that in 90% of use-cases with Next 10, the above example with just i18next will work fine.

kachar commented 3 years ago

@alexandrepaivaa I'm hitting the same issue:

Cannot update a component (`Home`) while rendering a different component (`CustomApp`)

So far I've ended up splitting the initial changeLanguage from the dynamic change via useEffect.

The lines that I've wrapped are:

if (i18next.language !== router.locale) {
  i18next.changeLanguage(router.locale);
}

On init the i18next.language is undefined and that's okay, but when we change the url we hit the problem. I've been able to extract the initialization in a dedicated hook. You might check out the gist at:

https://gist.github.com/kachar/ee23a13dffa493179e835f3482f3b470#file-usenextlocale-ts


But then I hit a difference between the server and the client in the initial language. Needs more tweaks

Warning: Text content did not match. Server: "ะะฐั‡ะฐะปะพ" Client: "Home"
Summon528 commented 3 years ago

I'm new to i18n and this is my current solution, any opinion? It is working pretty fine for me, no warning/error in console, and next can generate static HTML for each page.

_app.tsx

import React, { useEffect, useState } from "react"
import i18next, { ResourceLanguage } from "i18next"
import { AppProps } from "next/app"
import { useRouter } from "next/router"
import { I18nextProvider } from "react-i18next"

export default function MyApp(props: AppProps): JSX.Element {
  const { Component, pageProps } = props
  const router = useRouter()
  const [instance] = useState(i18next.createInstance())
  const locale = router.locale ?? "en"
  const namespaces = ["default"].concat(pageProps?.i18nNamespaces ?? [])

  if (!instance.isInitialized) {
    instance.init({
      supportedLngs: router.locales,
      fallbackLng: router.defaultLocale,
      lng: locale,
      ns: namespaces,
      defaultNS: "default",
      react: {
        useSuspense: false,
      },
      interpolation: {
        escapeValue: false,
      },
      resources: {
        [locale]: namespaces.reduce<ResourceLanguage>(
          // eslint-disable-next-line
        (pre, ns) => ({ ...pre, [ns]: require(`../locales/${locale}/${ns}.json`) }),
          {},
        ),
      },
    })
  }

  namespaces.forEach((ns) => {
    if (!instance.hasResourceBundle(locale, ns)) {
      // eslint-disable-next-line
        instance.addResourceBundle(locale, ns, require(`../locales/${locale}/${ns}.json`))
    }
  })

  useEffect(() => {
    if (instance.language !== locale) {
      instance.changeLanguage(locale)
    }
  }, [instance, locale])

  return (
    <I18nextProvider i18n={instance}>
      <Component {...pageProps} />
    </I18nextProvider>
  )
}

page/index.tsx

import { GetStaticProps } from "next"
import { useTranslation } from "react-i18next"

export default function IndexPage() {
  const { t } = useTranslation()

  return (
    <div>
      <p>{t("welcome_msg")}</p>
      <p>{t("secondary:hello_world")}</p>
    </div>
  )
}

export const getStaticProps: GetStaticProps = async () => ({
  props: {
    i18nNamespaces: ["secondary"],
  },
})
dcporter44 commented 3 years ago

@Summon528 I've tried out your code above. I think the reason you're able to avoid the warnings is your use of useEffect with changeLanguage (which also sounds like what @kachar said he's doing). But I notice a slight delay when changing language. When I switch to a new page with a different language, the previous language's translations show for a brief moment before the new language's translations come in. If you put a console log in your _app's useEffect and a console log on the next page, you'll see that useEffect runs after rendering the page. So that explains the slight delay in language change.

I'm not too sure what your use of I18nextProvider is doing. I believe the point of the provider is to be used with the withTranslation consumer so you can access that specific i18n instance. I don't think the useTranslation hook is meant to be used along with the I18nextProvider. Your code would likely work the same without the provider. Maybe I'm wrong. Is there a reason you're using the provider here?

dcporter44 commented 3 years ago

@alexandrepaivaa @kachar @Summon528 So I believe I found a solution that allows us to use the useTranslation hook without getting the warning that @alexandrepaivaa mentioned here.

The issue is the re-render triggered within useTranslation when changeLanguage is called. One solution is to call changeLanguage in a useEffect like @kachar and @Summon528 tried. However, this causes changeLanguage to be called after the page is rendered as I mentioned in the comment above. So not an ideal solution.

Another solution is to just not use useTranslation and instead use i18next.t like I showed in this comment. But this doesn't allow us to use the useTranslation hook. I'm not sure if that's really even a problem...

But if you really want to use useTranslation, I discovered you could do it this way without warnings:

_app.js

import '../styles/globals.css';
import { i18nextInit } from '../i18n.js';
import { useRouter } from 'next/router';
import { useTranslation } from 'react-i18next';

export default function MyApp(props) {
  const { Component, pageProps } = props;

  const router = useRouter();

  i18nextInit(router, pageProps.i18nResources);

  const { t } = useTranslation();

  return <Component {...pageProps} t={t} />;
}

And then simply use the t function passed into your page via props:

index.js

import { getTranslations } from '../i18n';

export default function IndexPage({ t }) {
  return (
    <div>
      <p>{t('welcome_msg')}</p>
      <p>{t('secondary:hello_world')}</p>
    </div>
  );
}

export async function getStaticProps({ locale }) {
  return {
    props: {
      i18nResources: await getTranslations(locale, ['default', 'secondary'])
    }
  };
}

This seems to work for me. No more warnings and we're able to use useTranslation.

Let me know your thoughts.

stephent commented 3 years ago

@dcporter44 I'd understood that the benefit of the useTranslation hook was to avoid the need to use prop drilling to pass t down through your component tree.

However, instead of using useTranslation, I've exported t from i18n itself as described in https://github.com/i18next/i18next/issues/1287#issuecomment-718781261 and it seems to be working nicely with your solution here in both dev and prod (on Vercel).

Thanks for all your work on this - super helpful.

dcporter44 commented 3 years ago

@stephent Ah, yeah - good point. I didn't consider the prop drilling. Yeah, it looks like just using t from the i18n instance is the way to go.

kubanac95 commented 3 years ago

@dcporter44 Any example on how I could make this running now while still using getInitialProps method instead of getStaticProps.

Also, from the code snippets you shared I don't see how we can preload a namespace, like for eg. common for each page?

Thanks for the good work and support!

isaachinman commented 3 years ago

Quick update

Hi all โ€“ย want to check in again. Apologies for a delay in response, I have been quite busy in the past few weeks.

I want to clarify that next-i18next will definitely be proceeding with a provider-based approach to support the maximum number of use cases. If individuals want to explore other options, I encourage them to, but please let's keep discussion in this issue centred around next-i18next.

After some consideration, and some discussion with the Vercel team and various NextJs contributors, I have decided to simply release the current work on a beta branch, as I do believe it's ready for testing.

In the end, how the NextJs core handles locale detection and fallback is likely going to be an ongoing process, as the Vercel team builds out parity with the rest of the industry.

next-i18next v8 beta release

I am going to stop all active development on next-i18next v7, outside critical security issues, and start releasing a beta that supports NextJs v10 on next-i18next@8.0.0-beta.x. That source code can be found on the next-v10 branch.

You can install this via yarn add next-i18next@beta and try it out right away!

Also, I have updated this example repo to use next-i18next@8.0.0-beta.0, and it is being deployed on Vercel here.

Next steps

While we're ready for early adopters, there is still a lot of work to be done. There is a lot of test coverage to write, edge cases to account for, unexpected behaviour on the Vercel platform, etc.

If anyone is interested in helping, please do reach out.

felixmosh commented 3 years ago

Quick update

Hi all โ€“ย want to check in again. Apologies for a delay in response, I have been quite busy in the past few weeks.

I want to clarify that next-i18next will definitely be proceeding with a provider-based approach to support the maximum number of use cases. If individuals want to explore other options, I encourage them to, but please let's keep discussion in this issue centred around next-i18next.

After some consideration, and some discussion with the Vercel team and various NextJs contributors, I have decided to simply release the current work on a beta branch, as I do believe it's ready for testing.

In the end, how the NextJs core handles locale detection and fallback is likely going to be an ongoing process, as the Vercel team builds out parity with the rest of the industry.

next-i18next v8 beta release

I am going to stop all active development on next-i18next v7, outside critical security issues, and start releasing a beta that supports NextJs v10 on next-i18next@8.0.0-beta.x. That source code can be found on the next-v10 branch.

You can install this via yarn add next-i18next@beta and try it out right away!

Also, I have updated this example repo to use next-i18next@8.0.0-beta.0, and it is being deployed on Vercel here.

Next steps

While we're ready for early adopters, there is still a lot of work to be done. There is a lot of test coverage to write, edge cases to account for, unexpected behaviour on the Vercel platform, etc.

If anyone is interested in helping, please do reach out.

Thank you for the update.

I've reviewed the code, it looks GOOD! Just to clarify that I got it properly, on each page there is a need to call to serverSideTranslations and pass it as props manually, correct?

isaachinman commented 3 years ago

@felixmosh Thanks for the review!

And yes, that is correct. That is more down to Next's paradigms/patterns than anything I would have designed by choice. The fact that they rely on a named export from page-level React components leaves no opportunity to actually write HOCs anymore.

I would be hesitant to say whether it's objectively good or bad, but: especially in the case of SSG where perf is a non-issue, tree traversal to shake out translation keys would be a much better approach.

felixmosh commented 3 years ago

It can be an HOF ( High order function) which will wrap the getStaticProps / getServerProps but it is not a deal breaker

isaachinman commented 3 years ago

@felixmosh That's correct, but to be honest I'd rather steer clear of any NextJs internals at this point! I hope that the serverSideTranslations pattern is declarative and clear to users.

dcporter44 commented 3 years ago

@isaachinman Thanks for the update! Code looks great. I didn't mean to steer this issue off-topic. Since Next now handles so much of the i18n routing and locale delegation, I was trying to understand the value of using next-i18next instead of just using react-i18next or just i18next. Personally I'll end up using the beta you've just released because it's less code management on my end and I trust your experience with i18next integration. Hopefully I can help out with writing some of the tests like you've mentioned.

But I'm curious if you see any major performance benefits using the HOC-based beta you've just released versus just using react-i18next or i18next by itself (like in my basic examples above).

isaachinman commented 3 years ago

@dcporter44 In general, next-i18next should not alter perf in any way, besides the few added kb to your JS bundle. You can see from the report here that the actual size of the next-i18next@beta source code is currently ~2.2kb GZIP, and could probably be reduced even further.

janhesters commented 3 years ago

Tried the beta and works mostly well ๐Ÿ‘ Thank you! ๐Ÿ™

It looks like fallbacks don't work. So if my default language is en and I use de and I lack a translation in de it doesn't substitute in the en translation.

Also a question: Is there still a way to programatically change the locale (without using routing)?

isaachinman commented 3 years ago

@janhesters Thanks for testing!

Locale fallback

We are still setting fallbackLng in the i18next config. If fallback behaviour is somehow different between master and next-v10 branches, then I've done something wrong. Do you have time to quickly debug a bit, and see if you can identify the problem?

Changing language imperatively

Have you checked the docs?

Their imperative example is as such:

router.push('/another', '/another', { locale: 'fr' })
dcporter44 commented 3 years ago

@isaachinman I'm doing some testing. Everything seems to work great.

Do you see any reason that we can't just get rid of defaultLocale and locales as options in next-i18next.config.js, and instead just use the values provided to next.config.js? I don't see the benefit in writing it in 2 places, but maybe I'm missing something.

You could pass the entire router object to getServerSideTranslations instead of just passing in the locale. And then you could pass that router object into createConfig. In createConfig you would then have access to Next's locale, defaultLocale, and locales.

In addition, if we make the localePath property default to '/public/static/locales', we could make using next-i18next.config.js entirely optional.

I also wanted to ask, which next-i18next.config.js options are still able to be used? This would be good start for the README. For example, the use option is no longer able to be used because functions are not able to be serialized as JSON in getStaticProps. I would make a PR for this, but I'm not sure what you had in mind.

janhesters commented 3 years ago

@isaachinman

Locale fallback

I didn't check in production, only development. That is probably the reason. False alarm, sorry.

Changing language imperatively

I missed that. Thank you!

njarraud commented 3 years ago

@janhesters Thanks for testing!

Locale fallback

We are still setting fallbackLng in the i18next config. If fallback behaviour is somehow different between master and next-v10 branches, then I've done something wrong. Do you have time to quickly debug a bit, and see if you can identify the problem?

My understanding is that you use lngsToLoad in the master version to select which languages shall be kept in the store. However in the beta version it is gone and serverSideTranslations only keeps the data for initialLocale only.

Master version:

const { fallbackLng } = config
const languagesToLoad = lngsToLoad(initialLanguage, fallbackLng, config.otherLanguages)

/*
  Initialise the store with the languagesToLoad and
  necessary namespaces needed to render this specific tree
*/
languagesToLoad.forEach((lng) => {
  initialI18nStore[lng] = {}
  namespacesRequired.forEach((ns) => {
    initialI18nStore[lng][ns] = (
      (req.i18n.services.resourceStore.data[lng] || {})[ns] || {}
    )
  })
})

Beta version

namespacesRequired.forEach((ns) => {
    initialI18nStore[initialLocale][ns] = (
      (i18n.services.resourceStore.data[initialLocale] || {})[ns] || {}
    )
  })
isaachinman commented 3 years ago

@dcporter44 Actually, it occurs to me that we can go one step further and just remove locales and defaultLocale entirely.

Since next-i18next@beta only ever initialises i18next instances for a single locale at a time, we don't even need to be aware of those other values, unless I am forgetting some use case. Check out the idea, here.

This would also allow users to completely omit the next-i18next.config.js file unless they had custom/non-standard configuration to add.

For example, the use option is no longer able to be used because functions are not able to be serialized as JSON in getStaticProps. I would make a PR for this, but I'm not sure what you had in mind.

Not sure I get your point here โ€“ we are already serialising the i18next instance in next-i18next@master โ€“ this isn't a new problem, right?

TheFullResolution commented 3 years ago

Optional next-i18next.config.js is a good idea ๐Ÿ‘ I mentioned that before, but I am working on the project where we are using a different locale code for next.js (for example en-NL, nl-NL) and different for locale code for i18n-next (en, nl). I am guessing this is not a common use case, so optional next-i18next.config.js would be awesome :) Once I have some more time, I will try to implement the beta in our project

AlexeyToksarov commented 3 years ago

@isaachinman looks like it won't be possible anymore to use getInitialProps for a page if I want to use this library, correct?

TheFullResolution commented 3 years ago

@isaachinman I was testing your new implementation, interestingly locally useTranslation from 'react-i18next' is working with next-18next, not sure if it will in production

felixmosh commented 3 years ago

@isaachinman I was testing your new implementation, interestingly locally useTranslation from 'react-i18next' is working with next-18next, not sure if it will in production

It should work since the appWithTranslations wraps the app with i18nextProvider

isaachinman commented 3 years ago

@TheFullResolution Yeah, after looking further into that use case, I just don't think it's something that next-i18next is going to be able to support very well.

@AlexeyToksarov Why do you say that? You should be able to use serverSideTranslations in any of the NextJs data fetching methods.

@TheFullResolution I believe the same is true of previous versions of next-i18next, and yes, it's for the reason that @felixmosh mentioned. We just re-export useTranslation as a utility for users.

AlexeyToksarov commented 3 years ago

@isaachinman because with getInitialProps it throws the Module not found: Can't resolve 'fs' error.

isaachinman commented 3 years ago

Ah that's correct, we are no longer taking into account the fact that it could run client side as well. What is your use case for using getInitialProps instead of getServerSideProps?

AlexeyToksarov commented 3 years ago

Basically, we just have an existing app and don't really want to rewrite every getInitialProps to getServerSideProps :)

isaachinman commented 3 years ago

Migrating to NextJs v10 i18n and next-i18next@8 will require a rewrite no matter what!

AlexeyToksarov commented 3 years ago

Yeah, I understand that. I just wanted to say that previously we didn't use the i18n, so in other to use it now we need to rewrite our current data-fetching code which uses getInitialProps. But hopefully, it shouldn't be too difficult

Thanks

TheFullResolution commented 3 years ago

@isaachinman why wouldn't support the use case? we are using our own method to extract the language, so getStaticProps looks like this:

export const getStaticProps: GetStaticProps<Props> = async ({ locale }) => {
    const lng = getLangFromLocale(locale!);
    return {
        props: {
            ...(await serverSideTranslations(lng, ['common', 'home'])),
        },
    };
};

So en-NL is being changed into en by getLangFromLocale, all loads smoothly so far.

Regarding the hook, is it safe to use it then, or is it just a side-effect that can break any moment?


Sorry, I started only now looking at source code. I got confused because types are not completely updated and Typescript was screaming you do not have any exported member 'useTranslation'.

I checked and the problem seems to be the fact you export types, but not methods in types.d.ts

export {
//here we need to add useTranslation, serverSideTranslations
  I18nContext,
  withTranslation,
}

I tested the beta version, successfully build the app, and deployed it. Translations working without a problem, our language switcher which calls nextjs router router.push('/', '/', { locale:${language}-${country}}); successfully changes locale and page displays new language afterwards.

So other than outdated types I see no issues with the new version, good job! ๐Ÿ‘

isaachinman commented 3 years ago

@TheFullResolution Ah, that makes sense. If you're handling locale resolution yourself, there shouldn't be any issues.

I just want to note that I've made some changes to types and documentation, and have updated the README.

I've released these changes on next-i18next@8.0.0-beta.1, which can be installed via yarn add next-i18next@beta.

If anyone can have a look at the new README and give thoughts, and continue to test the beta, I would appreciate that. Test coverage (both unit and e2e) is the main missing piece now, before considering a production-ready release.

kachar commented 3 years ago

@isaachinman When I try to test out the beta version I'm not able to fulfill the types definition expectations:

Could not find a declaration file for module 'next-i18next/serverSideTranslations'. './frontend/node_modules/next-i18next/serverSideTranslations.js' implicitly has an 'any' type. Try npm i --save-dev @types/next-i18next if it exists or add a new declaration (.d.ts) file containing declare module 'next-i18next/serverSideTranslations';ts(7016)

Not sure why we need @types at all as the project is TS based already

isaachinman commented 3 years ago

@kachar Ah, thanks for catching that. You are correct, this package exports its own types, and there is no such thing as @types/next-i18next.

I've added a type for next-i18next/serverSideTranslations and have released next-i18next@8.0.0-beta.2. Can you please have a look and let me know if that resolves the issue for you?

kachar commented 3 years ago

@isaachinman It looks the serverSideTranslations file is being found correctly, no error is being thrown. Anyhow it doesn't show the actual types exported from the function.

I've found that the other dependency appWithTranslation cannot be found in the package.

import { appWithTranslation } from 'next-i18next'

image

TheFullResolution commented 3 years ago

Yes, I also see serverSideTranslations as a valid import however since npm included only content of dist folder with compiled javascript code export { serverSideTranslations } from './src/serverSideTranslations' only prevents import error but doesn't provide types.

Also, appWithTranslation is missing from types.d.ts export, would be enough to just add it like this:

const  appWithTranslation: AppWithTranslation

export {
  I18nContext,
  useTranslation,
  withTranslation,
  appWithTranslation
}

For the rest, I still didn't find any issues with the beta version (I checked the latest version as well)

DoctorJohn commented 3 years ago

@isaachinman there is a typo in the "Advanced Configuration" section of the updated README file. There it says the config file shall be named next-i18next-config.js. According to src/serverSideTranslations.ts it's supposed to say next-i18next.config.js. (.config.js instead of -config.js).