vercel / next.js

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

[NEXT-325] NextJS13 - AppDir - Server redirect happens as fetch not browser redirection #43605

Closed schemburkar closed 1 year ago

schemburkar commented 1 year ago

Verify canary release

Provide environment information

Operating System:
  Platform: win32
  Arch: x64
  Version: Windows 10 Home Single Language
Binaries:
  Node: 18.5.0
  npm: N/A
  Yarn: N/A
  pnpm: N/A
Relevant packages:
  next: 13.0.6-canary.3
  eslint-config-next: 13.0.2
  react: 18.2.0
  react-dom: 18.2.0

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

App directory (appDir: true)

Link to reproduction - Issues with a link to complete (but minimal) reproduction code will be addressed faster

https://github.com/vercel/next.js/files/10133225/next-redirect-demo.zip

To Reproduce

Build & run the sample

next-redirect-demo.zip

yarn install
yarn dev

Navigate to /test/A this should do below:

The server redirect is done via fetch and not browser redirection.

image

Describe the Bug

The app has below pages

If user enters /external in url, the page correctly does a 302 redirect to external site. This is expected behaviour.

If user enters /test/A in the url, the page also returns a 302, but the redirection happens via fetch. This is not expected behaviour.

Expected Behaviour: The redirection should be browser redirection and not done via fetch.

To reproduce the bug

Navigate to /test/A this should do below:

Expected Behavior

The route /test/A should be browser redirection and not done via fetch.

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

NEXT-325

Fredkiss3 commented 1 year ago

I think if you want to redirect to an external domain, you could use middleware.ts, as next redirect function is more for redirecting between your routes and server components.

schemburkar commented 1 year ago

@Fredkiss3 I have a different opinion. If a redirect is being sent from a page with a 302 status code, it should always be a browser redirect. Not handled via xhr/fetch. It should not matter if its a external domain.

Also there is conflicting behaviour if the /external page is hit via url vs being called by another redirect from app dir. That means this is a bug in either case.

boehlerlukas commented 1 year ago

@schemburkar I was expecting a server side 302 redirect as well.

31i0t commented 1 year ago

Same issue here. The problem manifests similarly with a CORS error. I believe this is unexpected behavior, no? The redirect() docs say the URL can be relative or absolute. Why use an absolute URL for anything that isn't external?

31i0t commented 1 year ago

Workaround: app/settings/page.tsx

import Redirect from "./Redirect"

export default function SettingsPage() {
...
- redirect('https://google.com')
+ return <Redirect />
...
}

app/settings/Redirect.tsx

'use client'

import { useEffect } from 'react'

export default function Redirect() {
  useEffect(() => {
    window.location.replace('https://google.com')
  })
  return null
}
lucchev commented 1 year ago

Same issue here with middleware sending an external redirection.

import { NextURL } from 'next/dist/server/web/next-url';
import { NextRequest, NextResponse } from 'next/server';

export function middleware(request: NextRequest) {
  const externalBaseURL = 'https://...';
  return NextResponse.redirect(new NextURL(request.nextUrl.pathname, externalBaseURL));
}

export const config = {
  matcher: '/test'
};

It's interpreted by the Next client as a fetch, not a browser redirection. So same CORS error...

And I see no way to override Next fetch with mode:"no-cors"

LGmatrix13 commented 1 year ago

Been experiencing this issue still in 13.1. @boehlerlukas’s recommendation for a 302 server side redirect seems like a good idea…

DaleLJefferson commented 1 year ago

Yes this is really annoying, since the redirect is in a server component I was expecting to see a server side redirect.

This workaround https://github.com/vercel/next.js/issues/43605#issuecomment-1342410758 worked but it's not very satisfying.

morgangallant commented 1 year ago

Can confirm that the above-mentioned workaround did work, was having an issue where redirecting to another route (i.e. from /dashboard -> /login) wouldn't render the contents of /login properly, would just show the layout and blank otherwise. Not sure if this is immediately related to this issue or not, but certainly a similar fix — seems very reasonable that redirect() should be implemented as a 302 or something similar to it.

ChristianIvicevic commented 1 year ago

I am experiencing another minor issue with this behavior myself. In my root page I just have the following:

import { unstable_getServerSession } from 'next-auth';
import { redirect } from 'next/navigation';
import { authOptions } from 'pages/api/auth/[...nextauth]';

const Page = async () => {
  const session = await unstable_getServerSession(authOptions);
  redirect(session !== null ? '/lobby' : '/login');
};

export default Page;

The Vercel dashboard of my application shows a white screen and no preview which may be due to a lack of a browser-based redirect. Ironically the redirect works as expected with a classic index file at pages/index.tsx that contains the following code:

import type { GetServerSideProps } from 'next';
import { unstable_getServerSession } from 'next-auth';
import { authOptions } from 'pages/api/auth/[...nextauth]';

const Page = () => null;

export default Page;

export const getServerSideProps: GetServerSideProps = async ({ req, res }) => {
  const session = await unstable_getServerSession(req, res, authOptions);
  return { redirect: { destination: session !== null ? '/lobby' : '/login', permanent: false } };
};

Unlike the use of redirect() the old approach via gSSP takes the environment into consideration.

timneutkens commented 1 year ago

Had a look into this one, it happens because there is a redirect() call to a url that ends up redirecting, fetch() doesn't give us much to deal with this case because you can't get the location header when setting redirect: 'manual' on fetch otherwise this case would have an easy solution. What we'll have to do instead is always forcing a browser navigation when the fetch for the RSC payload fails.

Update: opened a PR here: #46674

github-actions[bot] commented 1 year ago

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.