vercel / next.js

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

rewrite rules are causing a page refresh (on Vercel) #52296

Open cloetensbrecht opened 1 year ago

cloetensbrecht commented 1 year ago

Verify canary release

Provide environment information

Operating System:
      Platform: linux
      Arch: x64
      Version: #1 SMP Fri Apr 2 22:23:49 UTC 2021
    Binaries:
      Node: 18.15.0
      npm: 9.6.7
      Yarn: 1.22.11
      pnpm: N/A
    Relevant Packages:
      next: 13.4.9-canary.2
      eslint-config-next: 13.4.8
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 5.1.6
    Next.js Config:
      output: N/A

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

No response

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

git@github.com:codeurs/next-translations.git

To Reproduce

I'm not able to replicate the bug locally. The bug only appears when the app is deployed on Vercel: https://next-translations.vercel.app

To run the app locally:

git clone git@github.com:codeurs/next-translations.git
cd next-translations
yarn
yarn build
yarn start

Describe the Bug

  1. Navigate to https://next-translations.vercel.app/en
  2. Open the inspector (F12) and open the network tab
  3. Record the network log (Ctrl + E)
  4. Click the apple link within the navigation bar It will navigate you to /en/fruit/apple
  5. Within the network log you will notice that there are two 404 error responses. Their filenames are something similar to
    • appel?_rsc=5huma This actually is the NL version of the route: /nl/fruit/appel (followed by ?_rsc=5huma)
    • pomme?_rsc=5huma This actually is the FR version of the route: /fr/fruit/pomme (followed by ?_rsc=5huma)
      1. Click the FR language at the top right corner to change the language of the app to FR. It will navigate you to /fr/fruit/pomme and it will trigger a full page refresh Within the network log you will notice that there are five 404 error responses. Their filenames are something similar to
    • pomme?_rsc=ghn7z This is the current route, followed by ?_rsc=ghn7z
    • banane?_rsc=ghn7z
    • raisin?_rsc=ghn7z
    • carotte?_rsc=ghn7z These are all the other FR navigation links
    • appel?_rsc=ghn7z This is the NL translation of the current route

rewrite-redraw

The app routes in the code are all written in English, the translations are rewrites to these English paths (next.config.js). The default EN routes are all working and not resulting in 404 responses:

But all the rewrited routes are resulting in 404 responses that are causing a page redraw:

source route locale destination route
[locale]/fruit/appel NL [locale]/fruit/apple
[locale]/fruit/pomme FR [locale]/fruit/apple
[locale]/fruit/banaan NL [locale]/fruit/banana
[locale]/fruit/banane FR [locale]/fruit/banana
[locale]/fruit/druif NL [locale]/fruit/grape
[locale]/fruit/raisin FR [locale]/fruit/grape
[locale]/groenten/wortel NL [locale]/vegetables/carrot
[locale]/légumes/carotte FR [locale]/vegetables/carrot

Expected Behavior

I would expect that there is no full page refresh and that the rewritten routes are not resulting in 404 errors while fetching their data.

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

Vercel

marco-digennaro commented 1 year ago

any workaround for this?

cloetensbrecht commented 1 year ago

any workaround for this?

Unfortunately I haven't found a workaround yet.

williamli commented 1 year ago

Thank you for sharing the detailed explanation of the issue affecting your Next.js application's client-side navigation (and prefetching). This problem is caused by the use of translated page paths and attempts to use rewrites in your next.config.js file to map custom translated routes to another route.

CleanShot 2023-09-13 at 12 14 44@2x

Solution 1 (not ideal): In your rewrites configuration in next.config.js, you would need to add additional rewrites to map translated paths used by server components to the correct English-based paths. For example, you would need to handle mappings like /fr/fruit/pomme?_rsc=3kpdw to /en/fruit/apple?_rsc=6e4kl. This solution can be challenging due to the dynamic nature of the endpoints and may not be the most straightforward approach.

Solution 2 (might not be ideal for you): Consider not using translated page paths. Instead of /fr/fruit/pomme and /nl/fruit/pomme, use /fr/fruit/apple and /nl/fruit/apple. This approach simplifies the routing but may not align with your localization strategy.

Solution 3 (recommended solution): Avoid using rewrites in your configuration, as this can add a layer of complexity and potentially impact system performance and hosting costs. Instead, you can set up a page at /app/[locale]/fruit/[type]/ to handle all the fruit-related content and translations within this page. This approach simplifies the routing and allows you to load the relevant content based on the locale.

When using this solution, you can use generateStaticParams to generate all the necessary path structures based on the values of [locale] and [type] at build time (i.e. so you will have static pages /fr/fruit/pomme, /nl/fruit/pomme, /en/fruit/apple etc ).

Solution 3 is recommended as it avoids the complexities introduced by rewrites and maintains a more efficient routing structure. It aligns with best practices for handling translations in Next.js.

cloetensbrecht commented 1 year ago

Hi @williamli ,

Thanks for having a look at this issue and for proposing the 3 solutions.

The third solution will fit indeed for the 'fruit' route, but I'm not sure it will be the best solution for the /vegetables route as it's changing for every language:

I suppose we will need something like this?

But actually all these vegatables routes are just aliasses. That's why we would like to work with url rewrites. In this case we can forward them to the correct vegetables route where we can handle the correct language. /en/vegetables/carrot = /nl/groenten/wortel = /fr/légumes/carotte

Another solution could be to catch all routes in a single [[...slug]] route; but this makes everything a little dirty and will result in a bad overview of the different routes.

According to me, the best solution would be to fix the 404 errors when handling the _rsc search params.

williamli commented 1 year ago

The third solution will fit indeed for the 'fruit' route, but I'm not sure it will be the best solution for the /vegetables route as it's changing for every language

I think /[locale]/[[..slug]] is the best solution for you that Next.js currently offers. Next.js's file-based routing was not designed to handle translations the way your app requires but using /[locale]/[[..slug]] is currently the best way to achieve your desired outcome. You are also welcomed to file a feature request with the Next.js team.

According to me, the best solution would be to fix the 404 errors when handling the _rsc search params.

If you pick the custom rewrite routes, you will also need to find ways to use the rewrite rules to also rewrite _rsc urls.