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 redirect from a page with Loading causes double redirect #59800

Closed
1 task done
harunsmrkovic opened this issue Dec 20, 2023 · 8 comments · Fixed by #63786
Closed
1 task done

Server-side redirect from a page with Loading causes double redirect #59800

harunsmrkovic opened this issue Dec 20, 2023 · 8 comments · Fixed by #63786
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked Navigation Related to Next.js linking (e.g., <Link>) and navigation.

Comments

@harunsmrkovic
Copy link

harunsmrkovic commented Dec 20, 2023

Link to the code that reproduces this issue

https://github.com/harunsmrkovic/double-redirect-nextjs-repro

To Reproduce

  1. Open /parent route
  2. Observe the timer shown on the redirected /parent/child route
Bildschirmaufnahme.2023-12-20.um.10.42.35.mov

Current vs. Expected behavior

I expected that after moving from Loading screen of Parent route and showing the child route, there will be no more redirects/refreshes done

In other words, I expected that once I land on /parent/child route that the timer will start from 0 and just increase, never going back to 0.

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 23.0.0: Fri Sep 15 14:42:57 PDT 2023; root:xnu-10002.1.13~1/RELEASE_ARM64_T8112
Binaries:
  Node: 18.18.2
  npm: 9.8.1
  Yarn: 1.22.19
  pnpm: N/A
Relevant Packages:
  next: 14.0.5-canary.19
  eslint-config-next: N/A
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 5.1.3
Next.js Config:
  output: N/A

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

App Router, Routing (next/router, next/navigation, next/link)

Additional context

I was testing only locally, in Chrome, Safari & Electron (Cypress).
EDIT: It is also reproducible in a production build in a deployed version.

Removing the loading.tsx page from the parent route resolves this issue.

NEXT-2952

@harunsmrkovic harunsmrkovic added the bug Issue was opened via the bug report template. label Dec 20, 2023
@github-actions github-actions bot added the Navigation Related to Next.js linking (e.g., <Link>) and navigation. label Dec 20, 2023
@harunsmrkovic harunsmrkovic changed the title Server-side redirect from a page that Server-side redirect from a page with Loading causes double redirect Dec 20, 2023
@LineGu
Copy link

LineGu commented Dec 25, 2023

I think it's because the strict mode of react is turned on

@harunsmrkovic
Copy link
Author

@LineGu but that mode is not relevant in production build, is it?

@tomdohnal
Copy link
Contributor

tomdohnal commented Jan 2, 2024

I've been looking into this issue as well and this is what I came to:

In the redirect function API reference (https://nextjs.org/docs/app/api-reference/functions/redirect), this is stated:

When used in a streaming context, this will insert a meta tag to emit the redirect on the client side.

This means that what happens is:

  1. The page initially loads and displays the contents of the loading.tsx file
  2. Soon after, the contents of of loading.tsx file is swapped for the contents of whatever page you'll be redirected to
  3. Bcs the <meta http-equiv="refresh" content="1;url=..."/> tag is inserted, the browser reloads the page at the URL passed to the said meta tag, essentially resulting in a browser refresh

For more context, I found this PR that initially implemented injecting the meta tags: #47207

My understanding is that using the meta tag is best practice for SEO as it indicates to search engine crawlers that the page is redirected (as you can't use HTTP status codes to indicate redirection when using streaming response).

That said, I think there should be a mechanism to opt out of this as it results in poor user experience.

@dihmeetree
Copy link

Also experiencing this as well. I noticed that permanentRedirect (https://nextjs.org/docs/app/api-reference/functions/permanentRedirect) does not cause the refresh, so i've been using that.

@steven-tey
Copy link
Contributor

@dihmeetree is permanentRedirect still working for you? I just tried on latest canary and it doesn't seem to work :(

@samcx
Copy link
Member

samcx commented Mar 6, 2024

Hmmm it just worked for me @steven-tey (using permanentRedirect). Digging into what permanentRedirect does and the comment above that may highlight the issue

@steven-tey
Copy link
Contributor

permanentRedirect still doesn't work for me, but I think it might be worth digging into the root of the issue and fix this for redirect since we might not want to do permanent 301 redirects in all cases and cause links to be cached in the user's browser

@ztanner ztanner added the linear: next Confirmed issue that is tracked by the Next.js team. label Mar 27, 2024
ztanner added a commit that referenced this issue Mar 28, 2024
### What & Why
When an RSC triggers `navigate` after the shell has already been sent to
the client, a meta tag is inserted to signal to the browser it needs to
perform an MPA navigation. This is primarily used for bot user agents,
since we wouldn't have been able to provide a proper redirect status
code (since it occurred after the initial response was sent).

However, the router would trigger a SPA navigation, while the `<meta>`
tag lagged to perform an MPA navigation, resulting in 2 navigations to
the same URL.

### How
When the client side code attempts to handle the redirect, we treat it
like an MPA navigation. This will suspend in render and trigger a
`location.push`/`location.replace` to the targeted URL. As a result,
only one of these navigation events will win.

Fixes #59800
Fixes #62463

Closes NEXT-2952
Closes NEXT-2719
Copy link
Contributor

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

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked Navigation Related to Next.js linking (e.g., <Link>) and navigation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants