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

Setting <html lang="..."> by default #20

Closed
kachkaev opened this issue Dec 1, 2018 · 44 comments
Closed

Setting <html lang="..."> by default #20

kachkaev opened this issue Dec 1, 2018 · 44 comments
Labels
question Further information is requested

Comments

@kachkaev
Copy link
Contributor

kachkaev commented Dec 1, 2018

I'm wondering if there is any way to implicitly set the lang attribute to the html tag. This is a good practice, which improves SEO and a11y.

Currently, my workaround is roughly this:

// ./pages/_document.js
import Document, { Head, Main, NextScript } from 'next/document'

export default class MyDocument extends Document {
  render() {
    return (
      <html lang={this.props.__NEXT_DATA__.props.initialLanguage}>
        <Head />
        <body>
          <Main />
          <NextScript />
        </body>
      </html>
    )
  }
}

If there is no way to inject lang automatically, shall it be mentioned in the example or plugin docs?

@isaachinman
Copy link
Contributor

Hi @kachkaev, I don't think that's a "workaround" - it seems like a reasonable solution to me.

In the NextJs paradigm, if you need to access attributes on the html tag directly, creating a _document.js component is the only way to do so.

How did you expect this to work otherwise?

It might be good practice, but I don't think it belongs in the example as it adds unnecessary complexity - a _document.js file is not necessary to get this plugin working.

We can definitely link to this issue from the README, though.

@isaachinman isaachinman added the question Further information is requested label Dec 1, 2018
@kachkaev
Copy link
Contributor Author

kachkaev commented Dec 1, 2018

How did you expect this to work otherwise?

I'm not sure, to be honest. Ideally, I would expect next-i18next to just add lang attribute via some API magic 😅 If you think that this is impossible, I'm happy to look at my "workaround" as at a "reasonable solution".

Perhaps, mentioning the importance of <html lang="..." /> in README is worth it. Feel free to close this issue whenever you feel comfortable. 😉

@isaachinman
Copy link
Contributor

Added with 16610ca.

@stephanembl
Copy link

Hi! Sorry for reopening this, I felt like it was a better idea.
I was looking into doing just what @kachkaev is trying to do, but with the exact same _document.js it doesn't work for me.

this.props.__NEXT_DATA__.props.initialLanguage is null. In fact, everything seems empty except for the namespacesRequired.

{ initialI18nStore: {}, initialLanguage: null, i18nServerInstance: null, pageProps: { namespacesRequired: [ 'common' ] }, }

@isaachinman
Copy link
Contributor

@stephanemombuleau Please provide a reproducible example (repository). Once you provide that, I can attempt to help you.

This is most likely user error as the approach above works just if you add it to the simple example, and serialisation of __NEXT_DATA__ is how this whole package works.

@stephanembl
Copy link

stephanembl commented Feb 8, 2019

It's all good now.
Turns out nextI18NextMiddleware needs to be given to the server before anything else (in my case, routesHandler, a handler from next-routes).

This works:

server.use(nextI18NextMiddleware(nextI18next));
server.use(routesHandler);

This gives an empty object like above:

server.use(routesHandler);
server.use(nextI18NextMiddleware(nextI18next));

@felixmosh
Copy link
Contributor

felixmosh commented Feb 25, 2019

This is my solution, use the lng from request object, and pass it as a prop to Document component.

import { lngFromReq } from 'next-i18next/dist/commonjs/utils';

export default class MyDocument extends Document<IDocumentProps> {
  public static async getInitialProps(ctx) {
    const initialProps = await Document.getInitialProps(ctx);
    const lng = lngFromReq(ctx.req);
    const additionalProps = {
      isRTL: RTL_LANGS.some(currentLng => currentLng === lng),
      lng,
    };

    return { ...initialProps, ...additionalProps };
  }

  public render() {
    const { isRTL, lng } = this.props;
    return (
      <html lang={lng} dir={isRTL ? 'rtl' : 'ltr'}>
        <Head>
        </Head>
        <body>
          <Main />
          <NextScript />
        </body>
      </html>
    );
  }
}

@isaachinman
Copy link
Contributor

@felixmosh Just wanted to say that there are cases where that will not work. We have a util called lng-from-req that performs some logic on the req object to figure out what language to render. You might want to import it and use it yourself.

@felixmosh
Copy link
Contributor

@isaachinman , I've updated my snippet, thanx

@felixmosh
Copy link
Contributor

felixmosh commented Mar 4, 2019

@isaachinman, just wanted to ping you, since the time I've started using lngFromReq I see in the prod build an error TypeError: Cannot read property 'options' of undefined that comes from that function.
I've revalidated, it comes from that function.

Looks that it related to that destructuring:

const {
    allLanguages,
    defaultLanguage,
    fallbackLng
  } = req.i18n.options;

Any idea why?

@isaachinman
Copy link
Contributor

@felixmosh In what cases is req.i18n undefined? Do you have a full reproducible example?

@felixmosh
Copy link
Contributor

It reproduces here: https://github.com/felixmosh/next-18next-bug

@isaachinman
Copy link
Contributor

@felixmosh Several problems:

  1. You're not even consuming your custom server.js file.
  2. You're not setting the default export of your i18n.js file to the actual class instance. This means your import in your server.js file will be undefined.

@felixmosh
Copy link
Contributor

felixmosh commented Mar 5, 2019

@isaachinman sorry,
I've updated the example, and it still occurs, [TypeError: Cannot read property 'options' of undefined].

just run npm run build && npm run start and you will see it in the console.

@felixmosh
Copy link
Contributor

felixmosh commented Mar 5, 2019

@isaachinman , I've found the root cause, looks like requests for static files (/static/, /_next/) are not having i18n on the req object.
So, for now I changed it to be:

const lng = ctx.req.i18n ? lngFromReq(ctx.req) : ctx.req.lng;

Thanx for the help 🤩

@isaachinman
Copy link
Contributor

I've found the root cause, looks like requests for static files (/static/, /_next/) are not having i18n on the req object.

Yes, those routes are ignored by default. But they shouldn't running getInitialProps on your _document.js. Why is that happening?

@felixmosh
Copy link
Contributor

felixmosh commented Mar 5, 2019

I'm not sure :/
But it is reproduces in the repo above, the weirdest think is that in that repo build -> start fails on that error, but in my project it is not fails, just print to the console.

@isaachinman
Copy link
Contributor

isaachinman commented Mar 5, 2019

@felixmosh Thanks for being persistent on this one - it's a genuine issue. We're ignoring the config.ignoreRoutes routes here, but obviously if you call the lngFromReq util yourself, it's not going to work.

I've created a very simple fix for this with #215 and will merge/release it soon.

@felixmosh
Copy link
Contributor

Thank you man!
You are awesome 😎

@dmitry-yudakov
Copy link

It seems initialLanguage is now in props.initialProps and it works with

<Html lang={this.props.__NEXT_DATA__.props.initialProps.initialLanguage}>

I'm using

  "next": "^8.0.3",
  "next-i18next": "^0.36.4",

@felixmosh
Copy link
Contributor

It seems initialLanguage is now in props.initialProps and it works with

<Html lang={this.props.__NEXT_DATA__.props.initialProps.initialLanguage}>

I'm using

  "next": "^8.0.3",
  "next-i18next": "^0.36.4",

Looks like you are using an internal prop __NEXT_DATA__, no?

@isaachinman
Copy link
Contributor

@felixmosh It's a relatively common pattern for better or worse.

@pantchox
Copy link

pantchox commented Nov 1, 2019

I just wanted to set the language because it affected my CSS in mobile view, making the page width moveable which was looking very bad (even when i set the view port tag of course)
to quickly fix it and set it I used the componentDidMount to inject the language to the browser.
For me it was more for CSS issue rather than language conventions.

    componentDidMount() {
        // inject language attribute for html tag, use _document.js in the future
        const htmlEl = document.getElementsByTagName('html');
        if (htmlEl && htmlEl.length > 0) {
            htmlEl[0].setAttribute('lang','en');
        }
    }

this works perfectly since componentDidMount is called only on client side.

@Vadorequest
Copy link

What's the recommended way to do that now, with Next 9.2?

I was already using a "Head" component which uses next/head from within, but it doesn't seem to work together with _document.

image

Also, I don't quite like the fact that this workaround removes the usage of Html and uses html instead.

Couldn't an option be made available from next/head? I believe that could be a better alternative to provide properties we wand to add to the <html> tag, without messing up with _document or use complex logic relying on the private API. (such as this.props.__NEXT_DATA__.props.initialProps.initialLanguage)

@Vadorequest
Copy link

Also, <html lang={this.props.__NEXT_DATA__.props.initialLanguage}> doesn't seem to work. It's not an incompatibility with my head.tsx file, if I just use <html lang="fr"> it works fine.

@felixmosh
Copy link
Contributor

Also, <html lang={this.props.__NEXT_DATA__.props.initialLanguage}> doesn't seem to work. It's not an incompatibility with my head.tsx file, if I just use <html lang="fr"> it works fine.

Did you tried this approach? It works for me for long time, without relaying on any private api's

@Vadorequest
Copy link

Thanks, I actually ended up writing my own:

import { NextPageContext } from 'next';
import Document, { DocumentProps, Head, Main, NextScript } from 'next/document';
import get from 'lodash.get';

import { LOCALE_FR, resolveBestCountryCodes } from '../utils/locale';

/**
 * XXX Is only rendered on the server side and not on the client side
 *
 * Used to inject <html lang=""> tag
 *
 * See https://github.com/zeit/next.js/#custom-document
 */
class TFPDocument extends Document<Props> {
  static async getInitialProps(ctx) {
    const initialProps = await Document.getInitialProps(ctx);
    const { req, res }: NextPageContext = ctx;

    const bestCountryCodes = resolveBestCountryCodes(req, LOCALE_FR);
    const lang = get(bestCountryCodes, '[0]', 'en').toLowerCase();

    return { ...initialProps, lang };
  }

  render() {
    return (
      <html lang={this.props.lang}>
        <Head />
        <body>
          <Main />
          <NextScript />
        </body>
      </html>
    );
  }
}

type Props = {
  lang: string
} & DocumentProps

export default TFPDocument;

But mine is based on proprietary resolveBestCountryCodes. Does lngFromReq returns a value like fr or fr-FR?

@felixmosh
Copy link
Contributor

felixmosh commented Nov 7, 2019

@Vadorequest it returns 'fr'.

@felixmosh
Copy link
Contributor

felixmosh commented Nov 26, 2019

I've found out that i18next-express-middleware that is applied by next-i18next-middleware is attaching language & languageDirection on res.locals.

So the code may look like:

export default class MyDocument extends Document {
  static async getInitialProps(ctx) {
    const initialProps = await Document.getInitialProps(ctx);
    const {
      res: { locals },
    } = ctx;
    const additionalProps = {
      languageDirection: locals.languageDirection,
      language: locals.language,
    };

    return { ...initialProps, ...additionalProps };
  }

  render() {
    const { languageDirection, language } = this.props;
    return (
      <html lang={language} dir={languageDirection}>
        <Head />
        <body>
          <Main />
          <NextScript />
        </body>
      </html>
    );
  }
}

@AntwanSherif

This comment has been minimized.

@martpie
Copy link

martpie commented Mar 30, 2020

How do these solutions handle language changes? Do I have to setup lang={} manually after using changeLanguage? Or is it something I can ignore and screen readers won't mind?

edit: apparently, you have to do it manually:

// Change <html>'s language on languageChanged event
if (typeof window !== undefined) {
  NextI18NextInstance.i18n.on('languageChanged', function(lang) {
    const html = document.querySelector('html');

    if (html) html.setAttribute('lang', lang);
  });
}

@isaachinman
Copy link
Contributor

isaachinman commented Mar 30, 2020

@martpie Hm, that's an interesting point. I think we should pull that logic into the languageChanged wrapper that we've got in place. Would you be willing to create a PR for that? It'd save a lot of other users having to do this.

@martpie
Copy link

martpie commented Mar 30, 2020

Sure, I can do that

@eric-burel
Copy link
Contributor

eric-burel commented Jun 18, 2020

@felixmosh your solution does not seem to work anymore for me, specifically when I build the app next run build && next run start, using latest "next-i18next": "5.0.0-beta.2". It do not rely on Express anymore.

res.locals is not set during document getInitialProps call.

It works in development mode though, very weird.

Edit: it seems to depend to the availability of res.locals, if it's not already defined both i18n-express-middleware and the new version i18n-http-middleware won't create it themselves.

@felixmosh
Copy link
Contributor

@eric-burel I still see the assignment on res.locals so it should be available.
Maybe for some reason you don't have res.locals at build (minification / module order issue) and therefore you don't enter that if.

Try to debug it :]

@eric-burel
Copy link
Contributor

Yep the problem is exactly there, if res.locals is not defined this part is jumped. It is not assigned because res.locals is not there in the first place. But as far as I understand this is Express stuff so it has no guarantee to be there with the new http generic version.

@eric-burel
Copy link
Contributor

eric-burel commented Jun 18, 2020

It worked when we used a custom Express server but it has no reason to work now with a generic approach.

Edit: if googlers have the same issue, a quickfix is to add this code in _app getInitialProps:

  if (appContext.ctx && appContext.ctx.res && !appContext.ctx.res.locals) {
    appContext.ctx.res.locals = {};
  }

@eric-burel
Copy link
Contributor

eric-burel commented Jun 18, 2020

Ok so a more robust code is as follow:

// i18next-http-middleware is in charge of enhancing the req object
interface IncomingMessageWithI18n extends IncomingMessage {
  language?: string;
  i18n: any;
}
export const i18nPropsFromCtx = (
  ctx: NextPageContext
): Partial<HtmlLanguageProps> => {
  if (!(ctx && ctx.req && (ctx.req as IncomingMessageWithI18n).language))
    return {};
  const req = ctx.req as IncomingMessageWithI18n;
  return {
    lang: req.language,
    dir: req.i18n && req.i18n.dir(req.language),
  };
};

and in plain JS:

export const i18nPropsFromCtx = (ctx) => {
  if (!(ctx && ctx.req && ctx.req.language))
    return {};
  const req = ctx.req;
  return {
    lang: req.language,
    dir: req.i18n && req.i18n.dir(req.language),
  };
};

Use that in your custom _document getInitialProps call:

MyDocument.getInitialProps = async (ctx) => {
  const i18nDocumentProps = i18nPropsFromCtx(ctx);
  return {
    i18nDocumentProps,
  };
};

and then finally:

export default class MyDocument extends Document {
  render() {
    const { i18nDocumentProps } = this.props;
    return (
      <Html
        {...i18nDocumentProps}
      >
....

Thanks all for your fast answers!

@adyz
Copy link

adyz commented Jul 14, 2020

Hi @eric-burel -- awesome stuff! 👍

I'm fiddling with next.js (TS version) and next-i18next.

Your code works for me, but I am not sure where to import the IncomingMessage and some other types from your example (I've just ignored in my code for now).

1: Can you please post the full code with imports?
2: I have a button that users can click on to change the language, but that change does not apply to the document tag -- how would you change the lang and direction on the client-side?

function Header() {
  const [t, { language, changeLanguage }] = i18n.useTranslation();

  return (
    <div className={headerStyles.header}>
      <a href="#" title={t("brand-title")}>
        <Logo />
      </a>
      <button onClick={() => changeLanguage(language === "en" ? "ar" : "en")}>
        {t("change-locale")}
      </button>
    </div>
  );
}

is there any recommended way?

Thanks! 🙏

@eric-burel
Copy link
Contributor

eric-burel commented Jul 15, 2020

Yep the full code is here in our starter. IncomingMessage comes from node http which is the common base interface used by all node frameworks. Express extends it a bit with useful fields like locals, but you can't guarantee that you systematically have them in any Node app.

First, what is the precise purpose of "dir" and "lang" attributes? I wanted to set them for SEO purpose, but I must admit that I can't tell if you actually need to make them dynamic or not (maybe to have the right autocorrect language for instance?).

Since Document is smth server-rendered, you'll probably need a side-effect with a direct DOM mutation. So like const onChange = { document.querySelector("html").lang = "whatever" }. That's fake non-working code but you get my point.

@adyz
Copy link

adyz commented Jul 15, 2020

@eric-burel Thanks!

Beside SEO I use dir to flip my CSS styles for RTL languages (Arabic) so if a user changes the language from en to ar I need to update the dir on HTML and that will, in turn, flip the direction of text and other CSS logical properties.

import i18n from "components/i18n";

function Header() {
  const [t, { language, changeLanguage }] = i18n.useTranslation();

  function updateLang() {
    const newLang = language === "en" ? "ar" : "en";
    changeLanguage(newLang);
    document.documentElement.dir =  i18n.i18n.dir(newLang);
    document.documentElement.lang = newLang;
  }
  return (
    <>
      <button onClick={updateLang}>{t("change-locale")}</button>
    </>
  );
}

@marteloge
Copy link

@felixmosh your solution does not seem to work anymore for me, specifically when I build the app next run build && next run start, using latest "next-i18next": "5.0.0-beta.2". It do not rely on Express anymore.

res.locals is not set during document getInitialProps call.

It works in development mode though, very weird.

Edit: it seems to depend to the availability of res.locals, if it's not already defined both i18n-express-middleware and the new version i18n-http-middleware won't create it themselves.

I'm wondering if there is any way to implicitly set the lang attribute to the html tag. This is a good practice, which improves SEO and a11y.

Currently, my workaround is roughly this:

// ./pages/_document.js
import Document, { Head, Main, NextScript } from 'next/document'

export default class MyDocument extends Document {
  render() {
    return (
      <html lang={this.props.__NEXT_DATA__.props.initialLanguage}>
        <Head />
        <body>
          <Main />
          <NextScript />
        </body>
      </html>
    )
  }
}

If there is no way to inject lang automatically, shall it be mentioned in the example or plugin docs?

Does not seem like this works for the serverless beta version (currently testing 5.0.0-beta.4). Works when setting it the first time. When I change the language it is not reflected in the HTML lang. Does anyone have a solution for the new serverless beta version?

@pontusab
Copy link

You can use ctx.res.cachedUserLanguage in your _document.js

@isaachinman
Copy link
Contributor

The new i18n features in NextJs v10 will handle this automatically. I plan to rewrite next-i18next to leverage these features soon.

@i18next i18next locked as resolved and limited conversation to collaborators Oct 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests