vercel / next.js

The React Framework
https://nextjs.org
MIT License
126.56k stars 26.92k forks source link

[i18n] explicit access to /en not redirecting to / #21040

Open IskanderMustafin opened 3 years ago

IskanderMustafin commented 3 years ago

What version of Next.js are you using?

10.0.5

What version of Node.js are you using?

14.15.0

What browser are you using?

Chrome

What operating system are you using?

Windows 10

How are you deploying your application?

next dev (localhost)

Describe the Bug

According to docs when enabling i18n with following config:

module.exports = {
  i18n: {
    locales: ['en-US', 'fr', 'nl-NL'],
    defaultLocale: 'en-US',
  },
}

only available URLs should be:

/blog
/fr/blog
/nl-nl/blog

but if I try to access page explicitly specifing default locale (i.e. /en-US) then it will show page as is, without being redirected to /

Expected Behavior

It should redirect all paths prefixed with default locale to non-prefixed paths (i.e. /en to /, /en/hello to /hello etc).

I would like to get rid of duplicated pages, because it potentially can bring problems with SEO.

Also, I couldn't fix it using redirects section because it runs into infinite loop (ERR_TOO_MANY_REDIRECTS ):

 async redirects() {
            return [
                {
                    source: '/en',
                    destination: '/',
                    locale: false,
                    permanent: true,
                },
                {
                    source: '/en/:path*',
                    destination: '/:path*',
                    locale: false,
                    permanent: true,
                }
            ];
        },

To Reproduce

  1. Download and run official i18n-routing example
  2. Try to access to /en (default locale) and make sure it will show page without redirect
IskanderMustafin commented 3 years ago

Found another weird thing. If i put into redirects section this rule:

 {
                    source: '/search',
                    destination: '/search/all',
                    permanent: true,
                },

and try to access to /en/search, then it will correctly redirect to /search/all (i.e. it will remove default locale prefix)

wengtytt commented 3 years ago

I have the exactly same issue.

unixisking commented 3 years ago

I can reproduce it too, it doesn't redirect to the defaultLocale when we specify it in the URL.

mi-na-bot commented 3 years ago

I have reproduced this issue as well, including the infinite redirects. It is unfortunate that the behavior of next does not match the docs.

haggen commented 3 years ago

I wanted the exact opposite; no "default" routes, only prefixed ones, so any visit to /search would redirect to /en-us/search, in case my default locale was en-us.

mercteil commented 3 years ago

Absolutely the same issue, which is definitely bad for SEO and also for legacy routes redirects.

Lets say I have the i18n config:

  i18n: {
    locales: ['en', 'de'],
    defaultLocale: 'en',
  },

In cases of a legacy website which had a route something like /en/my-awesome-products?id=100 and I relaunch my application with a new routing like /products/:id this redirect does not work and I am forced to 404 all my backlink juice instead of 302 redirect it:

 {
      source: '/en/my-awesome-products...etc',
      destination: '/products/:id',
      permanent: true,
  },
  1. Nextjs MUST allow redirects of /${locale}... to any destination
  2. Nextjs MUST, if no redirects hit, redirect /${defaultLocale}... to /...
dimisus commented 3 years ago

Came across the following discussion and the sandbox example mentioned works exactly as I would expect it for the case of defaultLocale auto redirect.

https://github.com/vercel/next.js/discussions/19482

dimisus commented 3 years ago

Well, the NextJs internal redirection is somehow too fishy to use in production. I could not make it work properly. Actually I wanted to remove my custom server but I will not be able to because of the redirects huddle.

My workaround:

// custom server (should be something like fastify and not express, but you get the point)

const next = require('next');
const express = require('express');
const { redirectedRoutes301 } = require('./middlewares');

const port = parseInt(process.env.PORT, 10) || 3000;
const dev = process.env.NEXT_ENV !== 'production';
const app = next({ dev });
const routesHandler = app.getRequestHandler();

app
  .prepare()
  .then(() => {
    const server = express();

    server.use(redirectedRoutes301);

    server.get('*', (req, res) => routesHandler(req, res));

    server.listen(port, (err) => {
      if (err) throw err;
      process.stdout.write(`> nextjs server running on port: ${port}\n`);
    });
  })
  .catch((ex) => {
    process.stderr.write(ex.stack);
    process.exit(1);
  });

// Redirects with proper regular expressions for normal human beings // and not this path-to-regex hell which is unnecessary for redirects since you do not want // the request to go on any further than this redirect or match any file route // (this is what rewrites are basically for)

module.exports = [
  {
    source: /^\/en(.*)$/i,
    destination: '/$1',
  },
  {
    source: /(?:\/vin\?q=)(.*)$/i,
    destination: '/search?keyword=$1',
  },
  {
    source: /\/{2,}/i,
    destination: '/',
  },
  .........

// Redirects middleware

const redirectLocation = (url, redirectRule) => {
  return url.replace(redirectRule.source, redirectRule.destination);
};

const getRedirectedUrl = (urlToRedirect) => {
  const redirectRule = redirects.find(({ source }) => source.test(urlToRedirect));

  if (redirectRule) {
    // match redirects recursively (if any appear recursively)
    return getRedirectedUrl(redirectLocation(urlToRedirect, redirectRule));
  }

  return urlToRedirect;
};

exports.redirectedRoutes301 = (req, res, next) => {
  const { url = '' } = req || {};

  const redirectedUrl = getRedirectedUrl(url);

  if (url !== redirectedUrl) {
    res.redirect(301, redirectedUrl);
  } else {
    next();
  }
};

EDIT: Acutally if one uses a proper CDN like Cloudflare the custom server could be moved to a worker if the business logic is request agnostic or only decorates the req/res

zanzlender commented 2 years ago

I don't know if this has been resolved yet, but if anyone still has problems with it, I found that replacing the redirect url given in the documentation using Next 12 middelware solves this problem.

I replaced this from the docs: image

With this: image

request.nextUrl.href --> returns whole url request.nextUrl.pathname --> returns only path

Fyi NEXT_PUBLIC_HOST is just http://localhost:3000

zanzlender commented 2 years ago

And another little addition, in the statement request.nextUrl.local === "default", shouldn't it be !==. Because you would want to change the url only if the locale it isn't a default one. If it is then it doesn't get the /en/ or some other prefixes.

adroste commented 2 years ago

@zanzlender Your solution neither works nor makes sense. You are just redirecting /en to /hr.

I found a hacky solution with a custom middleware that redirects /locale* to /*. E.g. (en as default locale):

// pages/_middleware.ts

import type { NextMiddleware } from 'next/server';
import { NextRequest, NextResponse } from 'next/server';

export const middleware: NextMiddleware = (request: NextRequest) => {
  // redirect default locale to base path (e.g. /en/page -> /page)
  const { defaultLocale, href } = request.nextUrl;
  const detectedLocale = request.nextUrl[Object.getOwnPropertySymbols(request.nextUrl)[0]]
    ?.locale?.path?.detectedLocale;
  if (detectedLocale === defaultLocale) 
    return NextResponse.redirect(href);
};

Unfortunately, this uses the internal / private data structure of the nextUrl object. Therefore you should pin your next dependency in your package.json as internal data structures are subject to change without requiring prior notice to the user. So be sure to properly test this piece of code every time you update next js.


Why does it work?

request.nextUrl.href will always be the full url without the default locale, e.g.:

Conclusion: Redirecting to request.nextUrl.href will always strip the default locale from the url.

The tricky part now is to write a condition that will detect the default locale in the request url. As the next js parser strips the default locale from all of its publicly available properties, the only way I found to detect the default locale in the request url was by accessing its internal object that is referenced by a Symbol. It will contain a property locale.path.detectedLocale that is undefined when there is no locale in the request url, e.g.:

So if we now check if locale.path.detectedLocale is equal to the default locale we can deduce that the request url contains the unwanted /en.

kennethlynne commented 2 years ago

I have deployed this example to https://i18n-routing-six-gilt.vercel.app and can verify that the url https://i18n-routing-six-gilt.vercel.app/en takes you to a valid page, and that is in conflict with the documentation that clearly states that the default locale does not have a prefix.

image

I expect this can cause problems for SEO as one will get penalized for having duplicated content.

kennethlynne commented 2 years ago

If you make sure to set the canonical url you avoid SEO issues. Example: https://rishimohan.me/blog/nextjs-canonical-tag

shestakov-vladyslav commented 2 years ago

I do need this feature

shestakov-vladyslav commented 2 years ago

Solution above didn't work completely for me, so I have enhanced it. It was causing endless redirect when surfing between pages.

const { defaultLocale, href } = request.nextUrl;

const detectedLocale = request.nextUrl[Object.getOwnPropertySymbols(request.nextUrl)[0]]
    ?.locale?.path?.detectedLocale;

const headers = request.nextUrl[Object.getOwnPropertySymbols(request.nextUrl)[0]].options.headers;

const hasEN = !headers?.referer ? false : headers?.referer.indexOf('/en/') !== -1;

if (detectedLocale === defaultLocale && hasEN) 
    return NextResponse.redirect(href);
PhilippSchreiber commented 2 years ago

I have another example for this. Default locale is de. If you go to https://vercel-middleware-index-404.vercel.app/de/ you get a 200 back and get redirected to https://vercel-middleware-index-404.vercel.app/ in the frontend instead of a 308 and a server side redirect. Very bad for SEO.

Repo is here: https://github.com/PhilippSchreiber/vercel-middleware-index-404

CaptainN commented 1 year ago

I think what's happening with this:

         async redirects() {
            return [
                {
                    source: '/en',
                    destination: '/',
                    locale: false,
                    permanent: true,
                },
                {
                    source: '/en/:path*',
                    destination: '/:path*',
                    locale: false,
                    permanent: true,
                }
            ];
        },

Is that the locale flag is allowing the source to match including the locale prefix /en/:path but it's not stopping it from matching the default locale, when it's not present in the URL/:path. Seems like an easy miss.

WhidRubeld commented 1 year ago

Temporary hack in NGINX configuration to fix this issue.

    location /en {
        return 301 /;
    }

    location ~ '^/en/(?<rest_uri>.*)' {
        return 301 /$rest_uri$is_args$args;
    }
pixelmultiplo commented 1 year ago

So what, no solution and all multilanguage next.js websites will suck at seo forever?

ben-markato commented 1 year ago

I am having the same issue.

mercteil commented 1 year ago

I am having the same issue.

We ended up redirecting at DNS level...

franknoel commented 8 months ago

I still have this issue in Next.js v14.1.0, weird that the documentation is saying the opposite.

cc @leerob @timneutkens

CaptainN commented 8 months ago

The problem is that the issue is subtle and difficult to understand. It's still a problem, but it's hard to convince the engineers.

It's been long enough that I'd have to really dig to be able to adequately explain the issue. But to be clear - the last time I looked at this it wasn't "difficult" to write rewrite rules in nextjs - it was flat out impossible.

JiRadu commented 4 months ago

We're having the same issue Should we configure it from cloudflare?