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

Shallow routing does not work #1023

Closed
medihack opened this issue Mar 4, 2021 · 15 comments
Closed

Shallow routing does not work #1023

medihack opened this issue Mar 4, 2021 · 15 comments

Comments

@medihack
Copy link

medihack commented Mar 4, 2021

Describe the bug

When switching the location with shallow routing option (using the Link or Router) it is not working (language does not switch).

Occurs in next-i18next version

Cloned from master branch (8.0.6).

Steps to reproduce

Simply add shallow={true} to the Link component in the example app

Expected behaviour

Change of language should also work with shallow routing

OS (please complete the following information)

  • Browser: Chrome 88.0.4324.190

Additional context

I can explain why this happens. With the current implementation of next-i18next the i18n instance is always completely replaced. The problem is that the t function of useTranslation is cached. So, if the components do not get recreated but just rerendered (as in shallow routing) the t is the old cached one even if the I18nextProvider has already a new i18n instance.

@medihack
Copy link
Author

medihack commented Mar 6, 2021

@isaachinman If i18next/react-i18next#1273 gets merged there is still a little problem why shallow routing does not work for next-i18next. When using shallow routing then getServerSideProps won't be called again and appWithTranslation always uses a stale initialLocale to create the i18n client. To fix this the Next router must be queried for the correct locale. Here is a little code sample that would fix it in appWithTranslation.tsx:

const router = useRouter() // somewhere at the top
...
// changed stuff
locale = router.locale || initialLocale;

({ i18n } = createClient({
  ...createConfig({
    ...userConfig,
    lng: locale,
  }),
  lng: locale,
  resources: initialI18nStore,
}))

@isaachinman
Copy link
Contributor

Sure, that looks reasonable.

skrivanos added a commit to skrivanos/next-i18next that referenced this issue Mar 14, 2021
* Memoize the i18n client and do not re-create it unless the route or
  locale has changed (fix i18next#1059).

* Let Next's router locale take precedence over initialLocale (fix i18next#1023).

* Bump the minimum version of react-i18next.
skrivanos added a commit to skrivanos/next-i18next that referenced this issue Mar 14, 2021
* Memoize the i18n client and do not re-create it unless the route or
  locale has changed (fix i18next#1059).

* Let Next's router locale take precedence over initialLocale (fix i18next#1023).

* Bump the minimum version of react-i18next.
skrivanos added a commit to skrivanos/next-i18next that referenced this issue Mar 14, 2021
* Memoize the i18n client and do not re-create it unless the route or
  locale has changed (fix i18next#1059).

* Let Next's router locale take precedence over initialLocale (fix i18next#1023).

* Bump the minimum version of react-i18next.
skrivanos added a commit to skrivanos/next-i18next that referenced this issue Mar 14, 2021
* Memoize the i18n client and do not re-create it unless the route or
  locale has changed (fix i18next#1059).

* Let Next's router locale take precedence over initialLocale (fix i18next#1023).

* Bump the minimum version of react-i18next.
skrivanos added a commit to skrivanos/next-i18next that referenced this issue Mar 23, 2021
* Memoize the i18n client and do not re-create it unless the route or
  locale has changed (fix i18next#1059).

* Let Next's router locale take precedence over initialLocale (fix i18next#1023).

* Bump the minimum version of react-i18next.
skrivanos added a commit to skrivanos/next-i18next that referenced this issue Mar 23, 2021
* Memoize the i18n client and do not re-create it unless the route or
  locale has changed (fix i18next#1059).

* Let Next's router locale take precedence over initialLocale (fix i18next#1023).

* Bump the minimum version of react-i18next.
@Dragomir-Ivanov

This comment has been minimized.

DanPurdy added a commit to DanPurdy/next-i18next that referenced this issue Apr 30, 2021
* Memoize the i18n client and do not re-create it unless the route or
  locale has changed (fix i18next#1059).

* Let Next's router locale take precedence over initialLocale (fix i18next#1023).

* Bump the minimum version of react-i18next.
DanPurdy added a commit to DanPurdy/next-i18next that referenced this issue May 6, 2021
* Memoize the i18n client and do not re-create it unless the route or
  locale has changed (fix i18next#1059).

* Let Next's router locale take precedence over initialLocale (fix i18next#1023).

* Bump the minimum version of react-i18next.
@csinig
Copy link

csinig commented Sep 30, 2021

Did this fix really solve completely the issue ?
I mean with the previous next/nextI18next versions, we were using i18n.changeLanguage which did not cause a complete app refresh.
Now the behaviour has changed using the router, and the whole app is re-rendering. Side effect is that some important components state is lost.
We thought shallowRouting could help, but it did not work, neither did it with this fix.
Did we miss something important, or maybe this issue isnt solved yet ?
Thanks

@isaachinman
Copy link
Contributor

@csinig In next-i18next v8 and onwards, NextJs itself determines the locale, and all locale switching should be done through NextJs, not i18n.changeLanguage. The NextJs router controls the locale, and if you have a problem with the implementation, you should raise an issue on the NextJs repo.

@csinig
Copy link

csinig commented Sep 30, 2021

@isaachinman Yes we are aware of that, implementation has changed and we already have adapted the code to use the nextJs router.
We just thought that using shallowRouting=true would help to get back the previous behavior.
If I understand this is not and won't be the case ?

@isaachinman
Copy link
Contributor

@medihack Can you confirm if this issue is now resolved?

@medihack
Copy link
Author

@isaachinman I can't say for sure as I am currently using a custom solution in the app I am creating. I could test it, but I will need some time. But just looking at the code it should work.

@isaachinman
Copy link
Contributor

Sure, feel free to investigate and let me know if we need to reopen this.

@liho00
Copy link

liho00 commented Dec 6, 2021

@isaachinman This issue still persists. Pls reopen it.

                <Link
                  href={router.asPath}
                  shallow
                  replace
                  locale={activeLocale === "zh-CN" ? "en-US" : "zh-CN"}
                >
                  <a className="text-white font-medium text-sm">
                    {activeLocale === "zh-CN" ? "中文" : "English"}
                  </a>
                </Link>

did not update the language, unless i remove the shallow.

// next-i18next.config.js

module.exports = {
  i18n: {
    locales: ["en-US", "zh-CN"],
    defaultLocale: "zh-CN",
    localeDetection: false,
  },
  keySeparator: ".",
};

"next-i18next": "^10.0.1",

@isaachinman I can't say for sure as I am currently using a custom solution in the app I am creating. I could test it, but I will need some time. But just looking at the code it should work.

Hi, Could you sharing whats the custom solution about?

@isaachinman
Copy link
Contributor

@liho00 Did router.locale change? NextJs controls the locale itself, so I'm not sure what you're facing is an issue with next-i18next.

@csinig
Copy link

csinig commented Dec 6, 2021

@isaachinman i dont think its an issue with nextjs. We faced the same issue as @liho00 and just ended up switching to next-translate witch just works fine with shallow

@isaachinman
Copy link
Contributor

isaachinman commented Dec 6, 2021

From the docs:

Shallow routing allows you to change the URL without running data fetching methods again

next-i18next depends on those data fetching methods to provide translations as props to pages. If you'd like to use shallow routing with next-i18next, you'll need to follow these guidelines for loading translations on the client side.

If anyone wants to work on a PR for shallow routing support, it would be much appreciated.

@isaachinman isaachinman reopened this Dec 6, 2021
@isaachinman
Copy link
Contributor

I suppose we can consider this a duplicate of #1532.

@PeterWadie
Copy link

Is there any workaround to fix this until it gets fixed?

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.

6 participants