next.js icon indicating copy to clipboard operation
next.js copied to clipboard

Infinite page refresh with Next 12.2.4 using i18n and middleware

Open wadehammes opened this issue 2 years ago • 3 comments

Verify canary release

  • [X] 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 21.6.0: Sat Jun 18 17:07:22 PDT 2022; root:xnu-8020.140.41~1/RELEASE_ARM64_T6000
Binaries:
  Node: 16.15.0
  npm: 8.5.5
  Yarn: 1.22.19
  pnpm: N/A
Relevant packages:
  next: 12.2.4-canary.11
  eslint-config-next: 12.2.4
  react: 18.2.0
  react-dom: 18.2.0

What browser are you using? (if relevant)

Firefox

How are you deploying your application? (if relevant)

Vercel

Describe the Bug

With latest 12.2.4, I cannot navigate to any dynamic paths and my home url just refreshes over and over.

https://user-images.githubusercontent.com/222170/183095213-ad572000-e446-476a-8040-81d307b96ab4.mov

Expected Behavior

I think this started with 12.2.4-canary.11. Was fine prior to that in previous canaries after canary.4 (which fixed the dynamic path infinite reload). Also seems to be working locally and only happens on Vercel.

Link to reproduction

https://rhythm-marketing-git-update-next1222-gotrhythm.vercel.app/

To Reproduce

Create a next.js site with middleware and i18n using 12.2.4 deployed to Vercel

wadehammes avatar Aug 05 '22 14:08 wadehammes

So after some digging, the refreshes were happening due to middleware running on every Next internal. Adding this to my middleware alleviated the issue:

const nextPage = NextResponse.next();

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

if (
    pathname.startsWith("/_next") || // exclude Next.js internals
    pathname.startsWith("/api") || //  exclude all API routes
    pathname.startsWith("/static") || // exclude static files
    PUBLIC_FILE.test(pathname) // exclude all files in the public folder
  ) {
    return nextPage;
  }

I think the question stands, should this be a config flag to disable middleware running on any Next internals?

wadehammes avatar Aug 05 '22 14:08 wadehammes

Hi there!

I've noticed that the issue happens if the las part of the path is shorter than 8 chars. I guess this is why it breaks with i18n which appends the locale to the url.

On my usecase: /landings/jll-demo -> issue (8 chars) /landings/jll-demo2 -> works (9 chars)

My workaround right now is prepending a constant value to this final path like this: /landings/slug-jll-demo and removing it when doing the data fetching.

francofantini avatar Aug 09 '22 13:08 francofantini

@wadehammes the matcher config allows filtering to specific paths so that it doesn't run for _next or other static assets.

@francofantini that definitely sounds unexpected can you provide a link to a repo with a reproduction of this in the latest version of Next.js v12.2.4?

ijjk avatar Aug 09 '22 23:08 ijjk

I've narrow it down to being an issue when having more than one middleware. We are using turborepo with one "core" app that rewrites to another apps. Both the core and the other app have a middleware that check for authn.

We removed the middleware from the other app and the issue is gone now.

francofantini avatar Aug 11 '22 17:08 francofantini

@wadehammes the matcher config allows filtering to specific paths so that it doesn't run for _next or other static assets.

@ijjk If we wanted to restore the old middleware behavior of only running on files within /pages, do we need to maintain an array of all of the pages in our application for the matcher? ie: matcher: [/all, /my, /pages:goHere*]

or is there a way to just skip over the next internals without having to maintain the matcher?

edit: I've updated my matcher to be:

export const config = {
  matcher: ['/', '/:path((?!_next).*)']
}

and it seems to be working as expected.

Rangoons avatar Aug 12 '22 14:08 Rangoons

@wadehammes the matcher config allows filtering to specific paths so that it doesn't run for _next or other static assets.

@ijjk If we wanted to restore the old middleware behavior of only running on files within /pages, do we need to maintain an array of all of the pages in our application for the matcher? ie: matcher: [/all, /my, /pages:goHere*]

or is there a way to just skip over the next internals without having to maintain the matcher?

edit: I've updated my matcher to be:

export const config = {
  matcher: ['/', '/:path((?!_next).*)']
}

and it seems to be working as expected.

Just FYI, in Next.js docs there is a matcher that matches all paths except api routes, static assets and favicon https://nextjs.org/docs/advanced-features/middleware#matcher

And as far as I'm aware, middleware also didn't run previously only on files within /pages, but would also run for files inside /public folder.

marko-hologram avatar Oct 31 '22 22:10 marko-hologram

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

github-actions[bot] avatar Feb 10 '23 00:02 github-actions[bot]