vercel / next.js

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

Next.js issue with middleware/catch-all/rewrites/getInitialProps when client side navigation occurs #50212

Open mabasic opened 1 year ago

mabasic commented 1 year ago

Verify canary release

Provide environment information

❯ npx next info

    Operating System:
      Platform: linux
      Arch: x64
      Version: #20-Ubuntu SMP PREEMPT_DYNAMIC Thu Apr  6 07:48:48 UTC 2023
    Binaries:
      Node: 18.16.0
      npm: 9.5.1
      Yarn: 1.22.19
      pnpm: N/A
    Relevant packages:
      next: 13.4.4-canary.2
      eslint-config-next: 13.4.3
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 5.0.4

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

Data fetching (gS(S)P, getInitialProps), 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/mabasic/nextjs-middleware-catch-all-issue

To Reproduce

Next.js issue with middleware/catch-all/rewrites/getInitialProps when client side navigation occurs

Must be built with npm run build and started with npm run start for the issue to appear. It works in development mode with npm run dev.

Reproduction steps

next.config.js:

/** @type {import('next').NextConfig} */
const nextConfig = {
  reactStrictMode: true,
  async rewrites() {
    return [
      {
        source: '/:nodeSlugs*/test2',
        destination: '/test/:nodeSlugs*?slug=test2',
      },
    ]
  }
}

module.exports = nextConfig

Existence of middleware.ts file. Can be an empty file to reproduce the issue.

src/middleware.ts:

export function middleware() {
}

Catch-all page route eg. src/pages/test/[...slugs].js.

Must have getInitialProps for this issue to appear. It works when server rendering, but does not work on client side navigation.

src/pages/test/[...slugs].tsx:

import Link from 'next/link';
import { NextPageContext } from 'next';

export default function Test() {
    return (
        <p>
            <Link href="/something1/test2">something1/test2</Link> -
            <Link href="/something2/test2">something2/test2</Link>
        </p>
    )
}

Test.getInitialProps = async (ctx: NextPageContext) => {
    console.log({query: ctx.query})
  return { test: 'works' };
};

Issue

Open the browser on http://localhost:3000/something1/test2, and click on the link "something2/test2".

If you check the console log (in the browser) after a client side navigation event, you will not see slugs, but only slug. On server rendering this works properly. You get both slugs and slug in the console.

Solution

There are two solutions:

  1. you can remove one of: rewrite rule, middleware file, getInitialProps, or stop using catch-all
  2. explicitly pass catch-all variable slugs to the page params (see example bellow)

Example: next.config.js:

/** @type {import('next').NextConfig} */
const nextConfig = {
  reactStrictMode: true,
  async rewrites() {
    return [
      {
        source: '/:nodeSlugs*/test2',
        destination: '/test/:nodeSlugs*?slug=test2?slugs=:nodeSlugs*',
      },
    ]
  }
}

module.exports = nextConfig

Describe the Bug

Catch-all variable is not passed to context query when middleware file is present and when getInitialProps is used, and when a client-side navigation occurs.

Expected Behavior

Catch-all variable should be passed in this scenario.

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

mabasic commented 1 year ago

Affected issues:

broksonic21 commented 11 months ago

I mentioned this in my ticket, but there's a large series of items related to middleware.ts file simply existing and pages router - anyway to get eyes on it Vercel team? Big fan of NextJS, but this behavior is really impacting us.

and more...

broksonic21 commented 4 months ago

Any progress here for Pages Router and middleware.ts? I've seen even more tickets than above recently and this is blocking a big migration of ours.