[go: nahoru, domu]

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

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

Open
1 task done
XavierLasierra opened this issue Aug 18, 2023 · 74 comments
Open
1 task done
Labels
bug Issue was opened via the bug report template. Navigation Related to Next.js linking (e.g., <Link>) and navigation. Runtime Related to Node.js or Edge Runtime with Next.js.

Comments

@XavierLasierra
Copy link

Verify canary release

  • I verified that the issue exists in the latest Next.js 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
    },
  };
};
  • This is not happening for static routes.
  • This is not happening for dynamic routes with no middleware.js file.

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

@XavierLasierra XavierLasierra added the bug Issue was opened via the bug report template. label Aug 18, 2023
@github-actions github-actions bot added Runtime Related to Node.js or Edge Runtime with Next.js. Navigation Related to Next.js linking (e.g., <Link>) and navigation. labels Aug 18, 2023
@lkappeler
Copy link

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
Copy link
avitslv commented Aug 25, 2023

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
Copy link

We are facing similar issue - aralroca/next-translate#1131

@lauripiispanen
Copy link
lauripiispanen commented Aug 30, 2023

git bisect reveals commit 1398de9 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
Copy link

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
Copy link

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
Copy link

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
Copy link

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
Copy link
raRaRa commented Sep 9, 2023

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
Copy link
raRaRa commented Sep 9, 2023

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
Copy link
raRaRa commented Sep 9, 2023

I wonder if this PR fixes it: #54357

@vinnycodes
Copy link
vinnycodes commented Sep 15, 2023

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:
#53988 (comment)

@raRaRa
Copy link
raRaRa commented Sep 19, 2023

Still broken in 13.5.1

@SanderRuusmaa
Copy link
SanderRuusmaa commented Sep 20, 2023

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
Copy link
gijsbotje commented Sep 21, 2023

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
Copy link
raRaRa commented Sep 21, 2023

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
Copy link

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

@jeremymarc

This comment has been minimized.

@Zerebokep
Copy link

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

@thany
Copy link
thany commented Sep 28, 2023

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
Copy link

@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
Copy link
raRaRa commented Oct 5, 2023

Still an issue in 13.5.4 - reverting to 13.4.12 works.

@avitslv
Copy link
avitslv commented Oct 10, 2023

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.

@raRaRa
Copy link
raRaRa commented Dec 15, 2023

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

@angelovdev
Copy link

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

@florianmutter
Copy link

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

@TrevorThomp
Copy link

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

@gabrielf
Copy link
gabrielf commented Jan 25, 2024

I don't experience the "Server side locale always default in dynamic routes when a middleware is present" error when using next 14.1.0 in the minimal repo that reproduces the error: https://github.com/XavierLasierra/next-dynamic-routes-bug

But I do experience the redirect issue mentioned above unless I set localeDetection: false in the i18n-config. There is already another issue for this particular problem though: #55648

I've performed this test:

  1. Clone the repo above
  2. Upgrade to next 14.1.0
  3. Set localeDetection: false in next.config.js
  4. Visit each locale (default, en, de) and each page (/, /dynamic/1, /static) both via link and reload

And all pages seem to show the correct locale.

Maybe someone who still experiences this error, even with next 14.1.0, can update the minimal repo to reproduce the error?

@balthazur
Copy link

I just tried 14.1.0. Client side navigation is working now without reload. I use localeDetection and I don't experience the redirect issue.

However, the locale in getStaticProps is always the default locale.

@ninjarogue
Copy link
ninjarogue commented Feb 15, 2024

We are facing the same issue with 14.0.4.

@MartinDavi
Copy link

@leerob This issue has a reproduction, what needs to happen to get a response from Vercel?

@Griha3212
Copy link

Can't update our project higher than 13.4.12 cause of this localeDetection/middleware problems(((

@evtaranov
Copy link

The problem is still relevant, and it seems that the development team has no plans to fix it.

@smatjes
Copy link
smatjes commented Mar 4, 2024

Just want to chime in and add that we also face this issue in a similar setup. Also blocking us from updating to > 13.4.12.

@florianmutter
Copy link

Might be fixed by #62606

@gabrielf
Copy link
gabrielf commented Apr 3, 2024

I think this issue should be closed.

The bug as stated in the original issue: "Server side locale always default in dynamic routes when a middleware is present" using the original reproduction repo but updated to the latest next (14.1.4) does not show the bug.

With next 14.1.4 the endless redirect mentioned in the thread is also gone. I tried adding a Static Site Generation (SSG) page using getStaticProps also mentioned in the thread but it also got the correct locale both on client side navigation and on page reload.

There are other bugs related to i18n such as:

But the particular bug discussed in this thread seem to be solved.

I suggest that anyone that still sees this issue update the original reproduction repository https://github.com/XavierLasierra/next-dynamic-routes-bug or create a new, minimal one, that demonstrates the issue and submits a new issue or updates this one if you think its the same issue.

Please remember that router (app/pages), middleware (present or matching/not present or matching), runtime (node/edge), i18n (on/off), server (npm run dev/npm run build + npm start/vercel/other) can all affect the behavior. If you see this problem in your app then there may be something in it that differs from the minimal reproduction.

@Mattgic
Copy link
Mattgic commented Apr 9, 2024

Problem actually still occurs when middleware has a config object with a matcher array. I updated the reproduction repo with such a middleware file and last Next version (14.1.4), the issue still exists.
https://github.com/Mattgic/next-dynamic-routes-bug/blob/main/src/middleware.js

export const config = { matcher: ["/foo"] };
export async function middleware(req) {}

Having found this, I am going to try replacing the config matcher with conditions inside the middleware (https://nextjs.org/docs/app/building-your-application/routing/middleware#conditional-statements), it should do the trick.

@Thevyn

This comment has been minimized.

@gabrielf
Copy link

@Mattgic I tested your updated repo and can confirm that with the additional middleware config the bug is still present. Here is an updated bug report using the new template:

Link to the code that reproduces this issue

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

To Reproduce

  1. Clone repository, run npm i && npm run dev
  2. Visit http://localhost:3000/en
  3. Click the link Dynamic 1

Current vs. Expected behavior

As I click the link I expect the locale to be en but it is default.

Reloading the page correctly shows the locale as en.

Provide environment information

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 23.4.0: Fri Mar 15 00:12:49 PDT 2024; root:xnu-10063.101.17~1/RELEASE_ARM64_T6020
  Available memory (MB): 16384
  Available CPU cores: 12
Binaries:
  Node: 18.18.2
  npm: 9.8.1
  Yarn: 1.22.22
  pnpm: 8.15.5
Relevant Packages:
  next: 14.3.0-canary.9 // Latest available version is detected (14.3.0-canary.9).
  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) are affected? (Select all that apply)

Internationalization (i18n), Middleware / Edge (runtime), Routing (next/router, next/link)

Which stage(s) are affected? (Select all that apply)

next dev (local), next build (local), next start (local)

Additional context

The bug seems to be triggered by pages router + i18n + middlware present with matcher config + dynamic route.

The example repo demonstrates the bug using a Server Side Rendered page but the bug is also present for a Static Site Generated page using getStaticPaths + getStaticProps.

@omgovich
Copy link
omgovich commented Jun 13, 2024

We have the same issue on our project (Next.js v14.2.4).
A middleware with matcher makes getXProps receive a default locale when a user is doing client-side navigation.

Almost a year since the bug appeared, why is nobody fixing it?

P.S. I have reported the bug to "Pro Support" using my corporate Vercel account, but they said they can't help me with my Next.js problems.

@omgovich
Copy link
omgovich commented Jun 14, 2024

I am going to try replacing the config matcher with conditions inside the middleware (https://nextjs.org/docs/app/building-your-application/routing/middleware#conditional-statements), it should do the trick.

I have tried the same, but enabling middleware for all URLs breaks next.config.js rewrites (in docs it is said that middleware has higher priority than rewrites).

So you'll have to implement rewrites inside of your middleware. Here is how I did that (hope it'll help):

import { compile, match } from 'path-to-regexp'

// Copy of the array from `next.config.js`
const REWRITES = [
  {
    source: '/:type(resume|cover-letter|cv)-templates',
    destination: '/templates/:type',
  },
  {
    source: '/@:path',
    destination: '/my/:path',
  }
]

export default async function middleware(req: NextRequest) {
  const url = req.nextUrl

  // Return static files right away
  if (url.pathname.startsWith('/_next/static') || url.pathname.startsWith('/_next/image')) {
    return NextResponse.next()
  }

  // Perform rewrites
  for (const rewrite of REWRITES) {
    const fromPath = match(rewrite.source)
    const result = fromPath(url.pathname)
    if (result) {
      const toPath = compile(rewrite.destination)
      url.pathname = toPath(result.params)
      return NextResponse.rewrite(url)
    }
  }

  // ... Put the rest of your middleware here ...

  return NextResponse.next()
}

@thansk
Copy link
thansk commented Jun 15, 2024

Almost a year since the bug appeared, why is nobody fixing it?

P.S. I have reported the bug to "Pro Support" using my corporate Vercel account, but they said they can't help me with my Next.js problems.

I thought only Next.js is abandoned but it seems the company is abandoning its customers too.

@samcx
Copy link
Member
samcx commented Jun 23, 2024

Hi everyone,

We apologize for the length it has taken to respond to this issue.

I was not able to reproduce the issue when testing the :repro: (https://github.com/XavierLasierra/next-dynamic-routes-bug) on the latest canary v15.0.0-canary.41

CleanShot.2024-06-23.at.11.07.41.mp4

Let me know if you are able to confirm this or are seeing anything differently :frog-eyes:

@tjcampbell
Copy link

@samcx please see @Mattgic's comment above. The issue still exists when the middleware contains a matcher in the config. The repro middleware doesn't contain any config which is misleading.

@Mattgic
Copy link
Mattgic commented Jun 24, 2024

I updated my repro repo to latest Next canary, bug still exists as previously said when middleware has a matcher.
https://github.com/Mattgic/next-dynamic-routes-bug

@samcx
Copy link
Member
samcx commented Jun 27, 2024

@tjcampbell @Mattgic

This is the matcher I used in particular, and I can't replicate. :frog-eyes:

export async function middleware(req) {}

export const config = { 
  matcher: [
    '/((?!api|_next/static|_next/image|favicon.ico).*)',
  ]
};

CleanShot 2024-06-27 at 18.03.07.mp4.zip

@tjcampbell
Copy link

@samcx you should be able to replicate it with the middleware CORS example, or with @Mattgic's repro.
https://nextjs.org/docs/pages/building-your-application/routing/middleware#cors

export const config = {
  matcher: '/api/:path*',
}

@samcx
Copy link
Member
samcx commented Jul 1, 2024

@tjcampbell Using @Mattgic's :repro: with that config (not the CORS example). I can :repro:! (my bad for missing it at first)

CleanShot.2024-07-01.at.13.49.20.mp4

@thansk
Copy link
thansk commented Jul 1, 2024

@tjcampbell Using @Mattgic's :repro: with that config (not the CORS example). Cannot :repro:.
CleanShot.2024-07-01.at.13.49.20.mp4

You are reproducing it. You pick English, you go to the dynamic page and it shows up as default which is wrong.

@samcx
Copy link
Member
samcx commented Jul 1, 2024

@thansk Oh my bad, I see your point, I was just looking at the URL :lol:.

Interesting, it must be related to the matcher :frog-eyes:, even though there's no /api endpoint...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue was opened via the bug report template. Navigation Related to Next.js linking (e.g., <Link>) and navigation. Runtime Related to Node.js or Edge Runtime with Next.js.
Projects
None yet
Development

No branches or pull requests