aralroca / next-translate

Next.js plugin + i18n API for Next.js 🌍 - Load page translations and use them in an easy way!
MIT License
2.64k stars 207 forks source link

Problem with Page Transitions #513

Open josegutierro opened 3 years ago

josegutierro commented 3 years ago

Hello,

First, thank you for this amazing library.

I'm experimenting this issue with any page transitions library eg. next-page-transitions framer-motion etc. Problem is that translation namespace is changed before animation finish:

<PageTransition timeout={1500} classNames='page-transition'>
  <Component {...pageProps} key={router.route} />
</PageTransition>

Note*: I set 1500 of delay to make more evident de issue, even without timeout effect is the same.

chrome-capture (1)

Is there a way to wait animation finish before load the namespace translation?

Thank you

haydencleary commented 3 years ago

@josegutierro I had the same issue, I found the following code to work better: const { t } = useTranslation(); t("home:hero.title")

instead of

const { t } = useTranslation("home"); t("hero.title")

josegutierro commented 3 years ago

Hi @haydencleary

Thank you for your feedback but still the same behaviour with const { t } = useTranslation(); t("home:hero.title") 😰

haydencleary commented 3 years ago

@josegutierro my bad! Just remembered the real fix that worked for me. I found this wrapper online that allows the translations to persist. You should just need to import this hook instead of next-translate, hope it helps.

import { useMemo } from "react";
import useNextTranslation from "next-translate/useTranslation";

/*
    This wrapper hook is so the translations persist through the page animations.
    Otherwise the route is considered different and next-translate will change the translation file before the animation has finished.
*/

export default function useTranslation(namespace, options) {
    const { t, lang } = useNextTranslation(namespace, options);
    const T = useMemo(() => t, [lang]);
    return { t: T, lang };
}
josegutierro commented 3 years ago

Hello @haydencleary

this works! Thank you so much, you made my day! 🎉

aralroca commented 3 years ago

@josegutierro @haydencleary I did a PR to include this inside the useTranslation hook, this way you do not need a workaround.

aralroca commented 3 years ago

@josegutierro @haydencleary already pre-released on 1.0.6-canary.3 😊 If you can confirm that it works, that would be great

haydencleary commented 3 years ago

Hey @aralroca thanks for the PR! I just tested and it doesn't seem to work if I simply replace the work around 😕 It does work however if I add the translations to both the origin page and the destination page namespaces. This wasn't the case before, I only had the translations in the origin page namespace previously.

josegutierro commented 3 years ago

Thank you @aralroca I will test it asap and I'll come back to you.

aralroca commented 3 years ago

Hey @aralroca thanks for the PR! I just tested and it doesn't seem to work if I simply replace the work around 😕 It does work however if I add the translations to both the origin page and the destination page namespaces. This wasn't the case before, I only had the translations in the origin page namespace previously.

@haydencleary thanks for the feedback. Do you have any idea why this could happen? If you look at the PR code I used a useMemo in the t, similar to your workaround.

haydencleary commented 3 years ago

@aralroca I just went over your PR and it's basically identical to the work around 😅 The only thing that appears to be different is that I destructure the returned result from the library and then return the extracted t in the useMemo function. Perhaps there's something to try there, but I don't see how it is any different to be honest. Also I noticed you import useMemo in src/I18nProvider.tsx but it isn't used.

aralroca commented 3 years ago

Also I noticed you import useMemo in src/I18nProvider.tsx but it isn't used.

This was a mistake, this import can be removed!

aralroca commented 3 years ago

I think we can revert the PR, because it also does not solve the problem it was trying to solve and introduces this other one: https://github.com/vinissimus/next-translate/issues/604

hengsok commented 2 years ago

@aralroca any updates on this? If I do like @haydencleary, the warning like below will be gone. When using page transition and we navigate from a page that use the string "more-examples:go-to-home" to a page that doesn't, we got the error below.

[Warning] [next-translate] "more-examples:go-to-home" is missing in current namespace configuration. Try adding "go-to-home" to the namespace "more-examples".

aralroca commented 2 years ago

Try this prerelease https://github.com/vinissimus/next-translate/releases/tag/1.3.6-canary.1

hengsok commented 2 years ago

@aralroca yes sure! Thanks so much! next-translate is way way better than next-i18next. Next-i18next kept re-render the _app.tsx for some reason in my setup. But next-translate works nicely without that issue.

the getT function is also very beautiful. Thanks for your contribution to open source.

hengsok commented 2 years ago

@josegutierro my bad! Just remembered the real fix that worked for me. I found this wrapper online that allows the translations to persist. You should just need to import this hook instead of next-translate, hope it helps.

import { useMemo } from "react";
import useNextTranslation from "next-translate/useTranslation";

/*
  This wrapper hook is so the translations persist through the page animations.
  Otherwise the route is considered different and next-translate will change the translation file before the animation has finished.
*/

export default function useTranslation(namespace, options) {
  const { t, lang } = useNextTranslation(namespace, options);
  const T = useMemo(() => t, [lang]);
  return { t: T, lang };
}

@aralroca strangely it is like what @haydencleary mentioned above, your implementation does not fix the issue but if I follow what @haydencleary suggested in the quoted, it works fine and no more Warning.

I remembered above you said adding useMemo adds some other issues.

So what if you really add useMemo to future versions and can we wrap another round of useMemo? Any ideas? :D

StLyn4 commented 2 years ago

I don't know how things are in next-page-transitions, but framer-motion does not save context states in AnimatePresence. As a result, useTranslation receives new data in the old component. The solution above will start to behave strangely if both the page and its language change at the same time. Re-providing helped me. It's more of a workaround, but still...

In _app.js:

import { useContext } from 'react';
import I18nContext from 'next-translate/context';

function App({ Component, pageProps, router }) {
  const preservedI18nContext = useContext(I18nContext);

  return (
    <AnimatePresence mode="wait">
      <motion.div key={router.pathname + (router.locale ?? '')} ...>
        <I18nContext.Provider value={preservedI18nContext}>
          <Component {...pageProps} />
        </I18nContext.Provider>
      </motion.div>
    </AnimatePresence>
  );
}

export default App;
alessiofrittoli commented 1 year ago

I solved this issue by adding the locale code in the key prop of <Component /> in _app.tsx. Then in the page component I'm using a wrapper component which wraps all the page content and it is responsible of the page transition on mount and unmount. My issue:

./pages/_app.tsx

import type { ReactElement, ReactNode } from 'react'
import type { AppProps } from 'next/app'
import type { NextPage, NextPageContext } from 'next'
import { Router } from 'next/router'
import { appWithTranslation } from 'next-i18next'
import { AnimatePresence } from 'framer-motion'
import Site from '../lib/Generals/Site'
import ErrorBoundary from '../components/HOC/ErrorBoundary'
import Providers from '../components/HOC/Providers'
import Layout from '../components/Layout/Layout'
import DocumentHead from '../components/General/DocumentHead'
import ProviderConsumers from '../components/HOC/ProviderConsumers'
import '../styles/globals.scss'

/**
 * Custom page layout.
 * 
 * @param page    The page Component.
 * @returns            The page Component wrapped by its layout.
 */
export type GetLayout = ( page: ReactElement ) => ReactNode
export type NextPageWithLayout<P = {}, IP = P> = NextPage<P, IP> & {
    getLayout?: GetLayout
}

type AppPropsWithLayout = AppProps & {
    Component: NextPageWithLayout
}
export type NextPageContextRouter = NextPageContext &
{
    router: Router
}
const App: NextPage<AppPropsWithLayout> = (
    { Component, pageProps, router }
): JSX.Element => {

    /**
     * Load default `Layout` if no one has been provided.
     * 
     */
    const getLayout = Component.getLayout || ( page => <Layout>{ page }</Layout> )

    return (
        <ErrorBoundary>
            <Providers>
                <ProviderConsumers />
                {
                    getLayout(
                        <>
                            <DocumentHead
                                appColor={ Site.appColor }
                                canonical={ Site.canonical }
                            />
                            <AnimatePresence
                                mode='wait'
                            >
                                <Component
                                    { ...pageProps }
                                    key={ router.route + router.locale }
                                    router={ router }
                                />
                            </AnimatePresence>
                        </>
                    )
                }
            </Providers>
        </ErrorBoundary>
    )

}

export default appWithTranslation( App )

./hooks/useTranslation.ts

import { useMemo } from 'react'
import { useTranslation as useNextTranslation } from 'next-i18next'
import type { DefaultNamespace } from 'next-i18next'
import type { Namespace, KeyPrefix, UseTranslationOptions } from 'react-i18next'

/**
 * This wrapper hook is so the translations persist through the page animations.
 * Otherwise the route is considered different and next-translate will change the translation file before the animation has finished.
 * 
 * @link    [GitHub issue](https://github.com/i18next/next-i18next/issues/1767#issuecomment-1091769453)
 * 
 * @param   ns      The namespace(s) to use.
 * @param   options Translation options.
 * @returns         An object containing the T function and a i18n instance.
 */
const useTranslation = <
    N extends Namespace = DefaultNamespace,
    TKPrefix extends KeyPrefix<N> = undefined
>(
    ns?     : N | Readonly<N>,
    options?    : UseTranslationOptions<TKPrefix>,
) => {
    const { t, i18n }   = useNextTranslation( ns, options )
    const TFunction     = useMemo( () => t, [ i18n ] )

    return { t: TFunction, i18n }
}

export default useTranslation

Hope this can help someone.

juancamiloqhz commented 1 year ago

Thank you for sharing your solution, but I found that Namespace and KeyPrefix is exported in "i18next" but not on "react-i18next". So the imports working for me are the following.

import { useMemo } from "react";
import { useTranslation as useNextTranslation } from "next-i18next";
import type { DefaultNamespace } from "next-i18next";
import type { UseTranslationOptions } from "react-i18next";
import type { Namespace, KeyPrefix } from "i18next";
aralroca commented 1 year ago

@StLyn4 exposing the internal context fix this issue or we need more things to do on this issue? thanks

StLyn4 commented 1 year ago

@StLyn4 exposing the internal context fix this issue or we need more things to do on this issue? thanks

As I said earlier, this change allowed us to use the workaround described above. By the way, it was used even earlier in our production thanks to a small fork. So far we haven't seen any problems. At least nobody talked about them (I can't exactly determine the size of the projects audience).

It would be nice to try to implement something at library level, but the problem is that the context must be "grabbed" above the component which makes the page transition and "propagated" below that very component. Actually, I personally don't see the adequate way to solve this problem (not saying there isn't one at all)

aralroca commented 1 year ago

Ok thanks @StLyn4, I don't know it neither. I leave the issue open, if someone finds a proposal to fix this feel free to PR 😊