React-i18next: NextJs Sample: The page refresh after it fully loaded

[shakesBeardZ]

Hi guys, in the recent days I've noticed that my app(based on nextJS sample) get refreshed everytime when it fully loaded, and I also checked that in the sample and it's the same thing so anyone has experienced that? and how to get rid of it

[isaachinman]

It looks like you have language subpaths enabled, and are being redirected to the en locale. Can you provide a reproducible demo repo, perhaps?

[isaachinman]

@shakesBeardZ Please let us know if you want to proceed with trying to debug this.

[shakesBeardZ]

@isaachinman the problem exist in nextJs sample and I think it's because of the router replace
Router.replace(correctedPath, correctedPath, { shallow: true });
even though shollow is true but still redresh the page after the url in the adress bar changes , I mean it's loads the specific language and render it them do th refresh

and when you put in production I think it's desirable from UX prespective.
so if have any suggestion on how to get rid of it.

[isaachinman]

I still don't clearly understand the problem. Perhaps instead of GIFs, can you write how I can reproduce the problem, step by step?

[Nelrohd]

I have the same problem and I use in my app:

module.exports = {
    translation: {
      // default / fallback language
      defaultLanguage: 'fr',
      localesPath: './static/locales/',

      // needed for serverside preload
      allLanguages: ['en', 'fr'],

      // optional settings needed for subpath (/de/page1) handling
      enableSubpaths: false,
      subpathsOnNonDefaultLanguageOnly: false, // only redirect to /lng/ if not default language
    },
  };

I will try to reproduce the problem with the demo

[Nelrohd]

In fact, just download the current nextjs demo, do npm i && npm run dev and you can see the problem that @shakesBeardZ describes.

Note: if I use the link provided by the readme in examples/nextjs, I get the demo for the 7.x.x of react-i18next.

[Nelrohd]

More info for reproducing the problem:

1. Disable subpath in config.js -> enableSubpaths: false,
2. Fix bug when ctx.req is undefined in pages/_app

static async getInitialProps({ ctx }) {
    return {
      initialLanguage: (ctx.req && ctx.req.language) ?
        ctx.req.language : translation.defaultLanguage
    };
  };

1. Replace all the <Link> with <a>, so:

<Link href="/">
      <a>{t('link.gotoPage1')}</a>
</Link>

Becomes

<a href="/">{t('link.gotoPage1')}</a>

1. npm run dev and click any links, you will notice a quick flash

[Nelrohd]

Note 2: I completely removed everything in lib/languagePathCorrection.js based on @shakesBeardZ assumption but the bug is still there

import config from '../config';
import i18n from '../i18n';

const { defaultLanguage, allLanguages, subpathsOnNonDefaultLanguageOnly } = config.translation;

export default (currentRoute, currentLanguage = i18n.languages[0]) => {
  return currentRoute;
};

Anyway, I don't think it changes much since enableSubpaths is false.

[Nelrohd]

@isaachinman you can clearly see the flash when the page loads then reloads. If I remove react-i18next, my pages load as usual, without the flash. Notes that the problem does not exist in the version 7 of react-i18next. Probably something to fix in the NamespacesConsumer component.

[jamuhl]

might be related to: https://github.com/i18next/react-i18next/issues/600#issuecomment-435369595 point 1)

[isaachinman]

This is a genuine issue. Unsurprisingly, if I revert to 73e33b8, the issue goes away. We need to look at commit history since then and debug. I do not have time this weekend, but can take a deeper looker into this early next week. Apologies to everyone in the meantime - it's a work in progress.

[isaachinman]

If anyone has time to do a git bisect on this and find the bad commit before Monday, that would be incredibly helpful.

[revskill10]

You can look at my commit here , and the issue might have gone.
I have no issue till now with apollo and react-i18next.

[isaachinman]

I'm not sure why @revskill10's commits were merged in the first place. They contained several critical bugs... the app didn't even run. @jamuhl Any idea what happened there? I haven't been aware of these recent changes to the NextJs example source.

I do not think it's a good idea to be persisting app state in localStorage or cookies in this example. Same rationale as with RN - persistence belongs in user land.

[revskill10]

@isaachinman The PR is to resolve this issue (or non-issue), as i also marked it as non-issue in the first place.
We both had a discussion there about whether or not using cookie to persist locale. Your points are valid.

[jamuhl]

@isaachinman persistance is part of i18next language detection plugin...that comes for browser and node.js. But the provided PR sets cookies additional - which should be done in detector itself without pulling in cookie dependency

[jamuhl]

for browser : https://github.com/i18next/i18next-browser-languageDetector#detector-options

express: https://github.com/i18next/i18next-express-middleware#language-detection

cookie caching can be enabled on both - which persists lng on languageChange

[isaachinman]

@jamuhl Agree about keeping that within the language detection plugin.

[isaachinman]

So, let's get back to the actual topic here. This refresh issue seems to be introduced as a regression with 3c20461, which was a bug fix for another issue.

The problem is that the initialLanguage logic was rerunning on the client, where the req is never available, and was being reset to the defaultLanguage every time. Pretty straightforward bug. The real issue here is a misunderstanding - the initialLanguage logic belongs in a _document.js, _not_ an _app.js, because we only want to run that server side. We can then pass the value into _app.js via pageProps.

I've already created #608 to fix this bug.

That being said - @jamuhl if you are fine with it, let's leave the NextJs example source completely untouched until I can take a deep look at it next week when I am back in the office. I wasn't aware that this example was going to get so much use, and I think a fresh rewrite might be the best approach.

[isaachinman]

609

[jamuhl]

@isaachinman fully agree...seems next.js is right now top hot topic nr. 1 here - so i stand 200% behind your decisions

[Nelrohd]

@isaachinman Thank you for the patch #608 I implemented your solution and I don't see anymore the flash.

[Nelrohd]

@isaachinman In fact it does not work since you can't pass props to App component that way
https://github.com/i18next/react-i18next/pull/608/files#diff-3d33fc272b5b5d8cd4d515bd7939050dR19
So your fix works because initialLanguage is undefined in App

We should use the new feature enhancedApp used to pass props from _document to _app https://github.com/zeit/next.js/pull/4762/files#diff-ac08f8853956c0ef691e75a44d94cb03R5

[Nelrohd]

This is my _document, however it does not fix the flash

import React from 'react';
import Document, { Head, Main, NextScript } from 'next/document';
import { translation } from '../i18n-config';
import flush from 'styled-jsx/server';

export default class extends Document {
  static async getInitialProps(ctx) {
    const { req } = ctx;
    let initialLanguage = req.language ? req.language : translation.defaultLanguage;

    const enhanceApp = Component => (props) => <Component {...props} initialLanguage={initialLanguage} />
    const page = ctx.renderPage({ enhanceApp });
    const styles = flush();

    return {
      ...page,
      styles,
      initialLanguage,
    };
  }

  render() {
    const { initialLanguage } = this.props;

    return (
      <html lang={initialLanguage}>
        <Head />
        <body>
          <Main />
          <NextScript />
        </body>
      </html>
    );
  }
}

[Nelrohd]

If I remove wait={process.browser} from NamespacesConsumer, no more flash but of course we can see the text changing to the initial language translations.

[isaachinman]

Hi @Nelrohd. What makes you think you can't pass props into the NextJs Main? In my testing, intialLanguage in pageProps was definitely defined, but it could have been coming from the language detector plugin.

Perhaps you can join the discussion in #609? We'd rather do a solid rewrite rather than keep patching the existing solution.

[isaachinman]

Fixed with #613.