vercel / next.js

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

Server side locale always default in dynamic routes when a middleware is present #54217

Closed XavierLasierra closed 1 month ago

XavierLasierra commented 1 year ago

Verify canary release

Provide environment information

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 22.5.0: Thu Jun  8 22:22:20 PDT 2023; root:xnu-8796.121.3~7/RELEASE_ARM64_T6000
Binaries:
  Node: 18.8.0
  npm: 8.18.0
  Yarn: 1.22.19
  pnpm: N/A
Relevant Packages:
  next: 13.4.18-canary.0
  eslint-config-next: N/A
  react: 18.2.0
  react-dom: 18.2.0
  typescript: N/A
Next.js Config:
  output: N/A

Which area(s) of Next.js are affected? (leave empty if unsure)

Middleware / Edge (API routes, runtime), Routing (next/router, next/navigation, next/link)

Link to the code that reproduces this issue or a replay of the bug

https://github.com/XavierLasierra/next-dynamic-routes-bug

To Reproduce

  1. Clone repository, install dependencies and start the server
  2. Go to the main page with a locale other than the default (ex /en)
  3. Navigate to a dynamic route using next/link
  4. The locale on the server side will be always the default one only when doing client navigation

Describe the Bug

When doing client side navigation to a dynamic route having a middleware.js file, the locale will always be the default one.

export const getServerSideProps = async ({ locale }) => {
  return {
    props: {
      locale, // Always default with client side navigation
    },
  };
};

Expected Behavior

The locale will be consistent in dynamic routes.

Which browser are you using? (if relevant)

Chrome 115.0.5790.170

How are you deploying your application? (if relevant)

No response

lkappeler commented 1 year ago

We are facing the same issue using a global middleware file on the project. The locale passed to the getServerSideProps is the default locale.

We are using the locale for a subsequent request inside the getServerSideProps function. As a temporary workaround, I parsed the locale from the invoke path header so we can use it for our internal request. I thought it might be handy for someone.

const isValidLanguage = (value: string | null): value is Language => {
  if (!value) return false;

  return languages.includes(value as Language);
};

export const parseLocaleWithHeaderFallback = ({
  req,
  locale,
}: {
  req: IncomingMessage;
  locale: string;
}): Language => {
  if (locale !== 'default' && isValidLanguage(locale)) return locale;

  const headerValue = req.headers['x-invoke-path'];
  const [xInvokePath] = Array.isArray(headerValue)
    ? headerValue
    : [headerValue];
  const [parsedLocale] = xInvokePath
    ? xInvokePath.substring(1).split('/')
    : [null];

  if (isValidLanguage(parsedLocale)) return parsedLocale;

  Sentry.captureException(new Error('Could not derive locale from request header'), {
    extra: { req },
  });
  return 'de';
};

next.config.js

...
  i18n: {
    locales: ["default", "en", "de"],
    defaultLocale: "default",
  },
  ...
avitslv commented 1 year ago

Having the same issue when middleware file present. Problem is since v13.4.13.
We have multi locale setup. Navigating pages with next/link getting 404 page not found. The page is in en locale. In middleware req.nextUrl.locale is en, but in getStaticProps the context.locale is the default locale - e.g.de.

michalstrzelecki commented 1 year ago

We are facing similar issue - https://github.com/aralroca/next-translate/issues/1131

lauripiispanen commented 1 year ago

git bisect reveals commit 1398de9977b89c9c4717d26e213b52bc63ccbe7e to be the culprit. Not sure how helpful it is in pinpointing the actual cause since it's such a big merge commit, but it would seem that middleware URL normalization is leaking downstream and breaks locale handling for internal _next requests.

(edit: removed suggested workaround which did not avail the issue)

oleggrishechkin commented 1 year ago

looks like I have the same issue. I use middleware for default locale prefix and my locale inside getStaticProps is 'default' on client-side navigation. If I open a page by direct link locale is correct. I found that this issue starts from 13.4.13 version.

emilioschepis commented 1 year ago

Same issue here. We'll revert to 13.4.12 for the time being, let us know if we can provide any more info to help troubleshooting.

lx-maxhaslehner commented 1 year ago

We also ran into a similar problem when updating Next.js which broke our internationalization strategy. We´ll also keep using 13.4.12 for the moment.

SanderRuusmaa commented 1 year ago

Yup, after updating from 13.4.12 to 13.4.13+, the internationalization strategy breaks. getServerSideProps gets "default" as the passed locale.

Hopefully this gets looked into soon.

raRaRa commented 1 year ago

Wow, this bug was annoying me today. Downgrading to 13.14.12 fixed it. I use two languages, 'is' and 'en', where 'en' is the default one. If I open /is/ then the page opens in Icelandic. If I open it dynamically with Link, then it uses the default locale. Refreshing the page makes it work. I wonder if this is also a bug on prod?

raRaRa commented 1 year ago

In my case I have a middleware with a matcher of specific URLs. The middleware is not even run for the pages where this issue happens. But removing the middleware fixes it.

raRaRa commented 1 year ago

I wonder if this PR fixes it: https://github.com/vercel/next.js/pull/54357

vinnycodes commented 1 year ago

Any updates on this issue? We're finding this issue on all of our pages that use getServerSideProps.

According to nextjs docs, "default" should be added for prefixing locale. Now "default" is what is being passed to all of our link components that do no explicitly state what the locale is.

This resolved it for us: https://github.com/vercel/next.js/issues/53988#issuecomment-1680201847

raRaRa commented 1 year ago

Still broken in 13.5.1

SanderRuusmaa commented 1 year ago

Upgrading to 13.5.1 breaks the internationalization in a new way. The app gets stuck in a redirect loop when trying to navigate to the app's root ("/" or "/en" or "/et").

Seems to me that the automatic locale prefixing is still broken and 13.4.12 is still the last working version for me. Either some logic was changed and the Docs is not updated or it's just bugging out still.

//next.config.js (by the book, according to docs)
const nextConfig = {
  reactStrictMode: true,

  i18n: {
    locales: ['default', 'en', 'et'],
    defaultLocale: 'default',
  },

}

module.exports = nextConfig
//middleware.ts (by the book, according to docs)
import { NextRequest, NextResponse } from 'next/server'

const PUBLIC_FILE = /\.(.*)$/

export async function middleware(req: NextRequest) {
    if (
        req.nextUrl.pathname.startsWith('/_next') ||
        req.nextUrl.pathname.includes('/api/') ||
        PUBLIC_FILE.test(req.nextUrl.pathname)
    ) {
        return
    }

        // automatic locale prefixing
    if (req.nextUrl.locale === 'default') {
        const locale = req.cookies.get('NEXT_LOCALE')?.value || 'en'

        return NextResponse.redirect(
            new URL(`/${locale}${req.nextUrl.pathname}${req.nextUrl.search}`, req.url)
        )
    }
}

network tab when routing to "/": image

gijsbotje commented 1 year ago

Facing the same issue. Upgraded to 13.5.2 from 13.3.1. Server always has the default locale when routing via a NextLink. Reloading the page does give the correct locale.

raRaRa commented 1 year ago

It's really worrying that this has been an issue for such a long time without Vercel taking any action. This breaks so many websites that support multiple locales + have middleware file present (very common as well).

danielkoller commented 1 year ago

We have the same issue as well. Downgrading to 13.3.1 for now.

jeremymarc commented 1 year ago

same issue here...

Zerebokep commented 1 year ago

Same issue here, upgrading > 13.4.19 breaks the app with the 307 redirect loop.

thany commented 1 year ago

Same issue. I couldn't fathom this to be a bug, since it seems to basic and so easily (regression-)testable at Vercel's end.

So I started this discussion. No responses yet, at the time of writing, unfortunately.

I spent another 2 hours stepping through internal Next.js code, and I couldn't find any place where the locale definitely gets reset to default. Even knowing the middleware mechanism is the culprit, it still didn't help to figure out how, where, and why this is happening.

I'm also very interested to how this could've slipped through QA at Vercel. Since i18n seems like a pretty important feature, I would assume it goes through some sort of testing. Maybe not good enough then.

gijsbotje commented 1 year ago

@thany I would also assume such a feature has the proper tests in place. It might be because the app router doesn't provide out of the box i18n routing that this slipped through. I believe the app router is the main focus, which is quite clear as no one from vercel or the next team has replied about this issue yet.

raRaRa commented 1 year ago

Still an issue in 13.5.4 - reverting to 13.4.12 works.

avitslv commented 12 months ago

Having the same issue when middleware file present. Problem is since v13.4.13. We have multi locale setup. Navigating pages with next/link getting 404 page not found. The page is in en locale. In middleware req.nextUrl.locale is en, but in getStaticProps the context.locale is the default locale - e.g.de.

We updated project to use next v13.5.4, and now both in middleware and page getStaticPaths getting the correct locale values, not always the default locale.

rhkdgns95 commented 12 months ago

The same issue of infinite redirects occurred, so I downgraded from version v13.5.4-> v13.4.12.

Is there a solution?

raRaRa commented 11 months ago

The same issue of infinite redirects occurred, so I downgraded from version v13.5.4-> v13.4.12.

Is there a solution?

Unfortunately not, unless you remove the middleware file.

odincov commented 11 months ago

v13.5.4 works for here as expected... Make sure you updated properly (check your lock file for the correct next version presence only), remove .next directory before running again, update your middleware file to contain the new config bit with a matcher.

raRaRa commented 11 months ago

v13.5.4 works for here as expected... Make sure you updated properly (check your lock file for the correct next version presence only), remove .next directory before running again, update your middleware file to contain the new config bit with a matcher.

Doesn't work for me. Do you have a middleware present in your pages folder and are you using the pages folder? All client-side routing returns the default locale for me in 13.5.4. I've deleted ./.next & ./node_modules, etc. Still no luck.

odincov commented 11 months ago

@raRaRa middleware has to be in the same directory as pages, yes i am using pages and the middleware file is present...

Check your paths, make sure locale is outside of params { params: { slug }, locale } etc... Good luck.

raRaRa commented 11 months ago

That's precisely how I do it.

export async function getStaticProps({
  params,
  locale,
  previewData,
}: GetStaticPropsContext) {

Locale defaults to en.

  i18n: {
    locales: ["en", "is"],
    localeDetection: false,
    defaultLocale: "en",
  },

If I navigate to some URL with Nextjs <Link> on client-side, such as /is/test, locale becomes en in my getStaticProps, but if I refresh that page I get it as is (correct). So client-side navigation doesn't work on a locale different than the defaultLocale, but it works if I refresh the page (server-side navigation).

The middleware doesn't even run for those paths, it just needs to be present for this bug to appear. If I remove the middleware file it works. I tried console logging inside my middleware and it doesn't run for the path I tried. It only runs for certain paths which the matcher is configured to.

Please note that this only seems to happen on the development server (next dev), not production.

mkataja commented 11 months ago

The same issue of infinite redirects occurred, so I downgraded from version v13.5.4-> v13.4.12.

Is there a solution?

Try with localeDetection: false. In our case 13.5.5 seems to work correctly but only with that option set. Otherwise we get a redirect loop.

raRaRa commented 11 months ago

Still broken in 13.5.5.

rhkdgns95 commented 11 months ago

@mkataja

Try with localeDetection: false. In our case 13.5.5 seems to work correctly but only with that option set. Otherwise we get a redirect loop.

This is still an unresolved issue (v13.5.5)

What is unusual is that loops do not occur in local development, but issues do occur in the environment deployed after building.

SanderRuusmaa commented 11 months ago

The same issue of infinite redirects occurred, so I downgraded from version v13.5.4-> v13.4.12. Is there a solution?

Try with localeDetection: false. In our case 13.5.5 seems to work correctly but only with that option set. Otherwise we get a redirect loop.

This fixes the redirect issue for us as well. But if the localeDetection is "true", then it keeps getting into the redirect loop.

This means that automatic locale prefixing with localeDetection is still broken in version 13.5.6... sigh

raRaRa commented 11 months ago

Does anyone know if this affects the App Router? I'm already thinking about migrating after watching Nextjs conf.

gijsbotje commented 11 months ago

@raRaRa As the app router doesn't have built in localized routes, it should not affect it. You'll need to create your own middleware to handle localization.

oleggrishechkin commented 11 months ago

Looks like this issue was resolved in v14+.

SvetlioSS commented 11 months ago

Not working for me with v14.0.1

soulchild commented 10 months ago

@leerob Do you guys even care about the fact you're introducing app-breaking bugs with minor-/patch-level upgrades affecting real users or are you too busy riding the app-router hype train? I for one will not consider Next.js for upcoming projects anymore, just because no client is willing to pay for countless hours of chasing down bugs that come with every upgrade of the underlying framework. No wonder people hate on the JS ecosystem if all they ever tried was Next.js. Express and plain React have served me well for a decade now with almost no surprises when upgrading. This right here is ridiculous.

pixelass commented 10 months ago

Let's please keep in mind that next.js and vercel have been pushing web applications to the next level and the ecosystem offers a wide range of tools to make development of next-gen technology very comfortable.

If one is not happy with changes that's understandable and I have also stuck to pages directory for production and am not happy about the hype around tailwind just because it solves ONE problem. But if you're hating on the rough transition to the future, you're really fighting windmills.

BTW: yes this bug is stupid and should be fixed ASAP

soulchild commented 10 months ago

I'm not here to judge whether Vercel's vision for the future is the right move. Let's just say that the idea of RSCs, server actions and the app router in general is ... controversial.

I just don't want my app to break almost every single time I'm applying patch/minor upgrades to Next.js AND the issues that crop up are not even acknowledged.

Vercel's official story is: Hey, you might wanna try our new app-router if you like, but it's fine if you stick with the pages router. When in reality it feels more like: Dude, you're still using the old pages router? lol Come on, switch already. There is streaming and backend code right there in your frontend code. How can you possibly miss out on that? 😄

Mattgic commented 10 months ago

I confirm the issue is still here on latest 14.0.3 version. It's a serious problem for us as we can't upgrade Next anymore, and won't be able to migrate to App Router anytime soon.

soulchild commented 10 months ago

Also, it took us days to even find out what the problem was: Our client reported the redirect loop and I was unable to reproduce it - neither on the production system nor on my local machine. I initially wrote it off as some reverse proxy fuckup on the client's side. Turns out, I had my locale set to the app's default locale which prevented the problem from showing up on my machine. 😞

pixelass commented 10 months ago

Ok. So we found the bug immediately because we test our apps. This way we were able to act fast, make a decision and deploy a workaround.

balthazur commented 10 months ago

Facing the same problem, getStaticProps returns the default locale in dynamic routes with middleware file present (auth).

balthazur commented 9 months ago

Bug still present in v14.0.5-canary.5

joaogarin commented 9 months ago

Still facing this bug, its really worrying that no one in vercel seems to care about this one. This has been broken for many months and there is really no path forward since there is no real workaround.

Try with localeDetection: false. In our case 13.5.5 seems to work correctly but only with that option set

This doesn't really help in my case it seems. I still get really weird redirects even with this option set. We are pilling security vulnearbilities and other issues like https://github.com/vercel/next.js/issues/51263 which have been fixed but we can't update.

raRaRa commented 9 months ago

I would just do all the locale functionality manually, which will also help you migrate to app router in the future.

angelovdev commented 9 months ago

@leerob it has been more than 4 months since this issue was reported and there has been no communication from the team whatsoever.

florianmutter commented 9 months ago

I guess someone using a payed vercel product needs to open a support ticket or something to increase priority

TrevorThomp commented 9 months ago

Would love to see an update on this so we can upgrade to v14 for now we are pinned to v13.