i18next / next-i18next

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

react-i18next:: You will need to pass in an i18next instance by using initReactI18next when running build when there is translation in top level layout #1917

Closed bryanltobing closed 1 year ago

bryanltobing commented 2 years ago

🐛 Bug Report

image

This is only happening when I run build script and my layout call useTranslation hook

To Reproduce

A small repo to reproduce

This is the only place I call useTranslation hook

// My _app.tsx
import "../styles/globals.css";
import type { AppProps } from "next/app";
import { appWithTranslation, useTranslation } from "next-i18next";
import React from "react";

function MyApp({ Component, pageProps }: AppProps) {
  return (
    <Layout>
      <Component {...pageProps} />
    </Layout>
  );
}

const Layout: React.FC<{ children: React.ReactNode }> = ({ children }) => {
  const { t } = useTranslation();

  return (
    <div>
      <div>{t("hello-world")}</div>
      {children}
    </div>
  );
};

export default appWithTranslation(MyApp);

my only page is pages/index.tsx

and I already called the serverSideTranslation

export async function getStaticProps({ locale }: any) {
  return {
    props: {
      ...(await serverSideTranslations(locale, ["common"])),
    },
  };
}

the translation is working, and I believe as mentioned here https://github.com/i18next/next-i18next/issues/1840#issuecomment-1145614232 image

the only issue is that there are warnings happen when running build

Expected behavior

There's no warning that says react-i18next:: You will need to pass in an i18next instance by using initReactI18next when running build script

Your Environment

adrai commented 2 years ago

Try to move the Layout component to a different file.

adrai commented 2 years ago

serverSideTranslations should be in the same file where useTranslation is used, isn't it?

bryanltobing commented 2 years ago

Try to move the Layout component to a different file.

I used it in a different file when the warning happens. I moved it to _app.tsx just to make it easier to visualize, and I'm not sure there's a difference, it's just a component definition.

serverSideTranslations should be in the same file where useTranslation is used, isn't it?

not necessarily I think https://github.com/i18next/next-i18next/issues/1840#issuecomment-1145614232 since nextjs layout can't call serverSideTranslations in its own file

how next handle layout https://nextjs.org/docs/basic-features/layouts

adrai commented 2 years ago

Then probably we have to live with that warning... The problem is, during build, the Layout component is rendered also if i18next is not initialized yet and not ready...

If you do this, you'll see what I mean. But I honestly have no idea how to prevent this in next.js, sorry.

const { t, ready } = useTranslation()
console.log(ready)
console.log(t('hello-world'))
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.

yuuk commented 2 years ago

Did you resolve this problem?

bryanltobing commented 2 years ago

not stale

bryanltobing commented 2 years ago

@yuuk I didn't

martinjlowm commented 2 years ago

I'm experiencing the same thing. For me it seems like an issue with bundling (by debugging stack traces) where the useTranslate context loses its reference. In this scenario, the provider is created in the environment of the Node.js process (non-bundle) and some translation attempts made afterwards are successful within that same environment. Suddenly, React tries to render a bundled version of the module and the knowledge about the context is lost.

I happen to have layout components (in which this problem occurs) in a separate package and I think it's simply a Webpack misconfiguration on my end.

edit: I upgraded Next.js from 10 to 12 and that resolved it for me. I now use Webpack 5 as well.

martinjlowm commented 2 years ago

Figured out why. It's because react-i18next is listed as a regular dependency and not a peer dependency. Using pnpm/yarn 2/3 introduces this issue. I can see from previous issues that it seems to be intended, but I would disagree. Forking the project for now to fix it in my case.

adrai commented 2 years ago

Maybe just defining react-i18next in your dependencies will work for you? Then there's no need for a fork.

martinjlowm commented 2 years ago

Already have, however, the module resolution in a symlink setup does not function well in this case.

diff --git a/package.json b/package.json
index 02388f7e443548dc6e428429719346293099412c..cc1169a110b2b1cb53e81d78837cfec7a69570a3 100644
--- a/package.json
+++ b/package.json
@@ -130,12 +130,12 @@
     "core-js": "^3",
     "hoist-non-react-statics": "^3.3.2",
     "i18next": "^21.9.1",
-    "i18next-fs-backend": "^1.1.5",
-    "react-i18next": "^11.18.4"
+    "i18next-fs-backend": "^1.1.5"
   },
   "peerDependencies": {
     "next": ">= 10.0.0",
-    "react": ">= 16.8.0"
+    "react": ">= 16.8.0",
+    "react-i18next": "^11.18.4"
   },
   "resolutions": {
     "i18next": ">=21.8.14",

Have this patch if anyone's facing the same problem.


Well, turns out I was too quick to come to a conclusion. Pretty sure it works in development, but production builds would still fail. The server-bundle still has next-i18next references and will therefore continue to reference a different instance of react-i18next. I'm not entirely sure about PNPM internals, but even with the patch above and using https://pnpm.io/cli/patch, react-i18next would still be linked in the dependencies.

My workaround now is to transpile next-i18next and use resolve alias to force the same module instance:

const withNextTranspileModules = require('next-transpile-modules')(['next-i18next'], {
  resolveSymlinks: true,
  debug: false,
});

and

config.resolve.alias = config.resolve.alias || {};
config.resolve.alias['react-i18next'] = path.dirname(require.resolve('react-i18next/package.json'));
bojackhorseman0309 commented 2 years ago

I have the same problem but on a completely static next export project. I got the warning when running it and when building it, even though it works perfectly. I followed the SSG example and it works fine just with that error. It happens only on my header which is in my layout, exactly what happened to the OP. It obviously has to due to the fact that it is outside of <Component> but since you can't really getStaticProps on a regular component I guess that's it.

Haven't found a solution yet but not worrying for now since I don't see any bugs, but worried that something might happen in the future lol.

Using NextJS 12.2.5 and next-i18next on 12.0.0.

rikkit commented 2 years ago

I think I'm getting the same problem, with Yarn 3 and PnP. My scenario is I have a NextJS app and a component library built with react-i18next and packaged separately - same as @martinjlowm. next-i18next works fine for NextJS pages, but components from my package don't seem to find the I18NextProvider and I get react-i18next:: You will need to pass in an i18next instance by using i18nextReactModule in my console.

Unfortunately I couldn't get @martinjlowm 's workaround to work for me, I got Critical dependency: the request of a dependency is an expression from i18next-node-fs-backend (maybe related https://github.com/i18next/i18next-node-fs-backend/issues/228)

adrai commented 2 years ago

I think I'm getting the same problem, with Yarn 3 and PnP. My scenario is I have a NextJS app and a component library built with react-i18next and packaged separately - same as @martinjlowm. next-i18next works fine for NextJS pages, but components from my package don't seem to find the I18NextProvider and I get react-i18next:: You will need to pass in an i18next instance by using i18nextReactModule in my console.

Unfortunately I couldn't get @martinjlowm 's workaround to work for me, I got Critical dependency: the request of a dependency is an expression from i18next-node-fs-backend (maybe related i18next/i18next-node-fs-backend#228)

i18next-node-fs-backend is deprecated and replaced by i18next-fs-backend... make sure you have updated all i18next dependencies first

rikkit commented 2 years ago

I shouldn't have written that context in the message, it's distracting. The error came from the workaround.

@adrai can you comment on the workflow where second package uses react-i18next? How should that instance be initialised?

adrai commented 2 years ago

I shouldn't have written that context in the message, it's distracting. The error came from the workaround.

@adrai can you comment on the workflow where second package uses react-i18next? How should that instance be initialised?

Can you create a minimal reproducible example that really differs from the one above?

rikkit commented 2 years ago

It's the exact same as the original post, except that Layout is in a separate package, and using Yarn with PnP to install the package.

adrai commented 2 years ago

It's the exact same as the original post, except that Layout is in a separate package, and using Yarn with PnP to install the package.

https://github.com/i18next/next-i18next/issues/1917#issuecomment-1193339399

rikkit commented 2 years ago

So I went ahead and forked the project, changed i18next and react-i18next to be peer dependencies, and published it: @rikkilt/next-i18next

react-l18next 's I18NextProvider is now initialised as expected and the components in my library are getting translated.

It seems in order to support Yarn PnP, i18next and react-i18next need to be peer dependencies.

adrai commented 2 years ago

So I went ahead and forked the project, changed i18next and react-i18next to be peer dependencies, and published it: @rikkilt/next-i18next

react-l18next 's I18NextProvider is now initialised as expected and the components in my library are getting translated.

It seems to support Yarn PnP that's what's required - move i18next and react-i18next to be peer dependencies.

If this is ok for all, and there is no other negative impact, I'm ready to accept a PR and create a new release.

Please let me know...

rikkit commented 2 years ago

The only downside I can see is that people will have to install react-i18next and i18next separately:

yarn add next-i18next react-i18next i18next 

Otherwise there should be no functional differences (for non PnP users)

I'll make a PR soon and see if there are any docs that need to be updated in the repo 👍

adrai commented 2 years ago

The only downside I can see is that people will have to install react-i18next and i18next separately:


yarn add next-i18next react-i18next i18next 

Otherwise there should be no functional differences (for non PnP users)

I'll make a PR soon and see if there are any docs that need to be updated in the repo 👍

This will probably still be an issue in production: https://github.com/i18next/next-i18next/issues/1917#issuecomment-1234347869

rikkit commented 2 years ago

@adrai I've deployed my app with my forked package to Vercel ("production mode"), and it's working as expected. So I've sent a PR. 👍

I think the problem in the comment you linked relates to pnpm patching (which I don't know about).

adrai commented 2 years ago

@martinjlowm @bryantobing12 can you confirm?

adrai commented 2 years ago

also @kachkaev @capellini can you tell us your opinion, since https://github.com/i18next/next-i18next/issues/136 was already discussed in 2019

rikkit commented 2 years ago

Reading #136, I see this comment from @isaachinman

However, for advanced use cases, I believe that users may still need to use i18next

What cases? If there are situations where users need to install and import i18next directly, that probably means we've failed to > expose some part of the API.

The case is outlined above; using next-i18next and react-i18next in the same project with Yarn PnP.

I was thinking about it and an alternative solution could be to allow the user to provide the i18n instance, so that the instance created for react-i18next can be passed to next-i18next directly. I see there's a "global" i18n instance referred to in appWithTranslation - this function could be extended to allow passing in an i18n instance. I am not sure if I18nProvider would also need to be passed in, or if it's fine for one i18n to be shared between two separate providers.

martinjlowm commented 2 years ago

I decided to drop next-i18next altogether due to the lack of language detection support (pass on lng config on the client) and go with the custom solution I was using previously. However, it being a peer seems sane to me.

I think my findings from pnpm patching is that it's done after dependency installation, so the patch essentially doesn't do anything useful.

matthewwolfe commented 2 years ago

I've run into a similar issue with the following projects structure:

- NextJS App (uses next-i18next)
- Create React App (uses react-i18next)
- Shared Components Package (uses react-i18next)

I want to be able to import the Shared Components Package in both the CRA and NextJS apps, but using useTranslation from react-i18next directly in a NextJS app does not work. The translations never become ready = true and the NextJS logs display the following error:

react-i18next:: You will need to pass in an i18next instance by using initReactI18next
matthewwolfe commented 2 years ago

@adrai

Then probably we have to live with that warning... The problem is, during build, the Layout component is rendered also if i18next is not initialized yet and not ready...

If you do this, you'll see what I mean. But I honestly have no idea how to prevent this in next.js, sorry.

const { t, ready } = useTranslation()
console.log(ready)
console.log(t('hello-world'))

Because of the separate instance of react-i18next, ready never becomes true.

bryanltobing commented 2 years ago

I decided to drop next-i18next altogether due to the lack of language detection support (pass on lng config on the client) and go with the custom solution I was using previously. However, it being a peer seems sane to me.

@martinjlowm I did the same. what are you using now for localization internationalization in next app then?

martinjlowm commented 2 years ago

Still i18next (and react-i18next). Now, I'm just using my own initialization configuration which I used previously.

bryanltobing commented 1 year ago

@martinjlowm would you mind sharing how you initialize i18next and react-i18next in nextjs project without next-i18next ?

matthewwolfe commented 1 year ago

One potential workaround is writing a hook that checks the initialization of next-i18next and react-i18next. One should never become initialized so you can do something like:

import { useTranslation as useNextTranslation } from 'next-i18next';
import { useTranslation as useReactTranslation } from 'react-i18next';

function useTranslation() {
  const { ready: isReadyNext, ...nextProps } = useNextTranslation();
  const { ready: isReactReady, ...reactProps } = useReactTranslation();

  if (isReadyNext) {
    return { ready: isNextReady, ...nextProps };
  } else {
    return { ready: isReactReady, ...reactProps };
  }
}
rikkit commented 1 year ago

@matthewwolfe I don't think that effectively addresses the issue where a NextJS app with next-i18next uses a library with react-i18next. You would need the library to opt-in to this use case by using a custom useTranslation hook, so it'd be changing the dependency for an issue with the wrapper library.

The core problem is that there are two instances of i18n when - using the logic that next-i18next is a drop-in wrapper over react-i18next - there should only be one.

matthewwolfe commented 1 year ago

@rikkit yep that is definitely correct. The library would need to be the one providing the custom hook. Its not ideal but it seems to work. The most ideal solution is to make the dependency a peer dependency as mentioned above.

adrai commented 1 year ago

So seems there are 2 main causes for the "react-i18next:: You will need to pass in an i18next instance by using initReactI18next" warning:

1) when building the app: https://github.com/i18next/next-i18next/issues/1917#issuecomment-1193339399 2) for advanced project structure: https://github.com/i18next/next-i18next/issues/1917#issuecomment-1252500184

The proposed peerDependency solution would only address 2) and conflicts with the current drop-in wrapper approach. 1) seems not to be solvable... right?

rikkit commented 1 year ago

@adrai Do you have an opinion on my suggested alternative solution described here?

adrai commented 1 year ago

@adrai Do you have an opinion on my suggested alternative solution described here?

I don't think this will be possible without changing a lot of things... but feel free to try.

matthewwolfe commented 1 year ago

@adrai Any update on if the peer dependency PR will be merged?

adrai commented 1 year ago

@adrai Any update on if the peer dependency PR will be merged?

Personally, I don't think the peer dependency approach is adequate here...

matthewwolfe commented 1 year ago

@adrai it will not solve all of the problems mentioned in this thread, but it will specifically solve the problem of having three packages, one with next-i18next another with react-i18next and a shared package that also uses react-i18next and is used by both. Currently the re-export of react-i18next from this package is incompatible with using react-i18next in a dependency. The hook is never initialized.

matthewwolfe commented 1 year ago

The workaround I mentioned here: https://github.com/i18next/next-i18next/issues/1917#issuecomment-1256990725 works but sort of sucks.

adrai commented 1 year ago

The workaround I mentioned here: https://github.com/i18next/next-i18next/issues/1917#issuecomment-1256990725 works but sort of sucks.

Feel free to try a PR with a better solution that fulfills all needs... 🤷‍♂️

matthewwolfe commented 1 year ago

I think they are two separate issues though...

matthewwolfe commented 1 year ago

The core problem is that there are two instances of i18n when - using the logic that next-i18next is a drop-in wrapper over react-i18next - there should only be one.

stale[bot] commented 1 year 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.

rikkit commented 1 year ago

It's not stale

belgattitude commented 1 year ago

The core problem is that there are two instances of i18n when - using the logic that next-i18next is a drop-in wrapper over react-i18next - there should only be one.

The common source of this is:

To fix this, take a look at this https://github.com/bryantobing12/layout-reproduce-i18next/pull/1 (based on @bryantobing12 reproduction).

Tested with pnpm7 and yarn in node_module linker (the pnpm will be fixed by https://github.com/i18next/next-i18next/pull/1966)

The gist is

See the file changed: https://github.com/bryantobing12/layout-reproduce-i18next/pull/1/files

To make it more solid, I'll prepare a doc in there: https://github.com/i18next/next-i18next/pull/1966.

statico commented 1 year ago

Not stale. I switched from Yarn to pnpm and none of my translations work because I get this error. I've installed all the peer dependencies.

I'm mystified why changing package managers would change this behavior.