vercel / next.js

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

next.js 14 redirect() inside a server action can't switch between root layouts (was working in next.js 13) #58263

Open meszaros-lajos-gyorgy opened 11 months ago

meszaros-lajos-gyorgy commented 11 months ago

Link to the code that reproduces this issue

https://github.com/meszaros-lajos-gyorgy/nextjs-14-redirect-bug

To Reproduce

  1. start the development server with npm run dev
  2. open http://localhost:3000/en in your browser
  3. Click on the "search" submit button on top of the page next to the text input (it doesn't matter what you enter into the textfield, it always redirects to the same route -> http://localhost:3000/en/search/hello)

Current vs. Expected behavior

Expected result:

You should land on /en/search/hello as that is the hardcoded redirection inside the server action in services/SiteSearch.service.ts.

( This was the behavior in Next.js 13.4.12, but it also threw an error (see https://github.com/vercel/next.js/issues/53392) )

Actual result:

The server action sends back a redirect instruction among the response headers:

X-Action-Redirect: /en/search/hello

but the page does not go to that path

Verify canary release

Provide environment information

Operating System:
  Platform: linux
  Arch: x64
  Version: #37~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Mon Oct  9 15:34:04 UTC 2
Binaries:
  Node: 21.1.0
  npm: 10.2.0
  Yarn: 1.22.15
  pnpm: 6.11.0
Relevant Packages:
  next: 14.0.2
  eslint-config-next: 14.0.2
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 5.2.2
Next.js Config:
  output: N/A

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

App Router

Additional context

When I try the same but first opening http://localhost:3000/en/search/world and then try to redirect via the server action then it works without any issues.

I've also wrote down all of the infos above to the readme in my repo.

meszaros-lajos-gyorgy commented 11 months ago

Still not working in 14.0.3

meszaros-lajos-gyorgy commented 10 months ago

The issue is still present in 14.0.4-canary.18

mathiasrscom commented 10 months ago

Do we have an update on this one?

I had to do a bit of a workaround to render my login page instead of the nav as redirect does not work for me:

// Next
import { redirect } from "next/navigation"

// Queries
import { getUser } from "@/actions/queries/user/getUser"

// Auth
import { auth } from "@/auth"

// Components
import Nav from "@/components/nav"

// Pages
import LoginPage from "../(auth)/login/page"

export default async function AppLayout({
  children,
}: {
  children: React.ReactNode
}) {
  const session = auth()

  if (!session) {
    redirect("/login")
  }

  const user = session !== null ? await getUser() : null

  return <>{user ? <Nav user={user}>{children}</Nav> : <LoginPage />}</>
}
meszaros-lajos-gyorgy commented 10 months ago

Version 14.0.4-canary.39 still broken

meszaros-lajos-gyorgy commented 10 months ago

probably the same issue or very similar: https://github.com/vercel/next.js/issues/59217

muhaimincs commented 10 months ago

any work around?

jmarbutt commented 10 months ago

This whole time I thought it was me, but yes this is a pretty major issue for us.

meszaros-lajos-gyorgy commented 10 months ago

Still broken in v14.0.5-canary.6.

The possible workaround here is that the server action sends back some json data to the client and then the client can do a redirect on client side. Kinda removes the purpose of having server side actions though. Or we can always downgrade back to v13 I guess.

pinoniq commented 10 months ago

This actually does work for me for all redirects apart from redirect('/') or redirect('').

I have two root layouts in seperate groups. Where a server action from the login page does a redirect().

image

charandeep7 commented 10 months ago

It's still not working

carolinevsboliveira commented 10 months ago

I'm with the same problem. Redirect is not updating the route

logemann commented 9 months ago

Same for me. The first 3 hours i thought its about me being fairly new to NextJS but redirecting to some destination within my origin RootLayout works without issues. Its the context switching which breaks the redirect.

This one is pretty ugly because it affects one super mainstream use case "redirect after login", where its pretty normal that the login page is inside your "website" Routing Group and the target route is something like a dashboard in the "WebApp" Routing Group.

For the time being i will re-architect my loginpage to be a client comoonent which calls a handleSubmit() which calls my server action to auth and then get back JSON to handleSubmit where i will do a Route push. Works but ugly.

meszaros-lajos-gyorgy commented 9 months ago

Off topic, but I have to admit that I regret choosing Next 13 for a new project. This whole next generation of next.js kinda feels like a work-in-progress/proof-of-concept project rather than something that is production ready.

mr-14 commented 9 months ago

I've spent way too many frustrating hours on this. For me, the work around is to just have a common root layout with minimum markup.

Project structure: image

Example of /app/layout.tsx

export default function RootLayout({ children }: { children: React.ReactNode }) {
  return (
    <html lang="zh">
        <body>{children}</body>
    </html>
  )
}
meszaros-lajos-gyorgy commented 9 months ago

Still broken in v14.0.5-canary.43

innobrightcafe commented 9 months ago

wow, same issue. cant use with condition

AndreyKornyusko commented 9 months ago

still not working

logemann commented 9 months ago

because of that i reverted from a two root layout setup to one rootLayout (which basically has no layout at all).

guscsales commented 9 months ago

I've spent way too many frustrating hours on this. For me, the work around is to just have a common root layout with minimum markup.

Project structure: image

Example of /app/layout.tsx

export default function RootLayout({ children }: { children: React.ReactNode }) {
  return (
    <html lang="zh">
        <body>{children}</body>
    </html>
  )
}

This works for me as well. For next@14.1.0 when you have group routes defined in the app it's required to use an minimum layout.tsx for work.

sachinbhatnagar commented 9 months ago

I've spent way too many frustrating hours on this. For me, the work around is to just have a common root layout with minimum markup.

Project structure: image

Example of /app/layout.tsx

export default function RootLayout({ children }: { children: React.ReactNode }) {
  return (
    <html lang="zh">
        <body>{children}</body>
    </html>
  )
}

This worked! Thank you! This is such an annoying and undocumented issue. Wasted almost 3 hours before I saw this thread.

meszaros-lajos-gyorgy commented 8 months ago

Tested it on the latest canary version 14.1.1-canary.45, still broken, updated the repo for reproducing the issue.

iamgmd commented 8 months ago

Moving from NuxtJs to NextJs and got the same problem, I am starting to regret it based on this simple requirement :-(

adamchipperfield commented 8 months ago

How hasn't this got much attention? Feels like a pretty big issue.

iamgmd commented 8 months ago

Actually, I did get this working as I wanted, bear with me, I am new to NextJs and what I am doing maybe not what is intended but for me, all of my errors have gone and the redirect works as expected along with the different layouts rendering how I want them.

In my scenario I wanted 2 layouts, the anon layout and the landing layout. The only difference between those two is that I also have a Navbar in the landing layout.

@guscsales is correct, you need the root layout and the other layouts become nested layouts (I think).

So what I did is have the layout.tsx in the root of the app, with this being the root layout, it holds all of the top level stuff. (fyi, I have ShadCn also installed) and the other layouts are in route groups (anon) and (landing)

import { Montserrat as FontSans } from "next/font/google";
import "@/styles/globals.css";

const defaultUrl = process.env.VERCEL_URL
  ? `https://${process.env.VERCEL_URL}`
  : "http://localhost:3000";

export const metadata = {
  metadataBase: new URL(defaultUrl),
  title: "My NextJs and Supabase site",
  description: "The fastest way to build apps with Next.js and Supabase",
};

import { cn } from "@/lib/utils";

export const fontSans = FontSans({
  subsets: ["latin"],
  variable: "--font-sans",
});

export default function RootLayout({
  children,
}: {
  children: React.ReactNode;
}) {
  return (
    <html lang="en" suppressHydrationWarning>
      <body
        className={cn(
          "min-h-screen bg-background font-sans text-foreground antialiased",
          fontSans.variable,
        )}
      >
        {children}
      </body>
    </html>
  );
}

The (anon)/layout.tsx looks like this:-

export default function AnonLayout({
  children,
}: {
  children: React.ReactNode;
}) {
  return (
    <div className="flex min-h-screen flex-col items-center">{children}</div>
  );
}

The (landing)/layout.tsx looks like this:-

import Navbar from "@/components/Navbar";

export default function LandingLayout({
  children,
}: {
  children: React.ReactNode;
}) {
  return (
    <div className="min-h-screen bg-background font-sans text-foreground antialiased">
      <Navbar />
      {children}
    </div>
  );
}

In my (anon)/login/page.tsx I have my signin method redirect as below.

  const signIn = async (formData: FormData) => {
    "use server";

    const email = formData.get("email") as string;
    const password = formData.get("password") as string;
    const supabase = createClient();

    const { error } = await supabase.auth.signInWithPassword({
      email,
      password,
    });

    if (error) {
      return redirect("/login?message=Could not authenticate user");
    }

    return redirect("/");
  };

Apologies for the messy response, just wanted to let peeps know how I am doing this. Feel free to correct me, as I said, I am new to NextJs.

It maybe that the problem derives from the fact that you can/should only have one <html><body> so don't put them in anything else except the root layout.tsx.

iamgmd commented 8 months ago

@meszaros-lajos-gyorgy I have just amended your code slightly below and the redirect works as expected:

Add layout.tsx in src/app:-

export const metadata = {
  title: "Next.js",
  description: "Generated by Next.js",
};

export default function RootLayout({
  children,
}: {
  children: React.ReactNode;
}) {
  return (
    <html lang="en">
      <body>{children}</body>
    </html>
  );
}

Amend src/app/[locale]/layout.tsx to:

import { SiteSearch } from "@/components/SiteSearch";

export default function LocaleLayout({
  children,
}: {
  children: React.ReactNode;
}) {
  return (
    <div>
      <SiteSearch />
      {children}
    </div>
  );
}

Amend src/app/search/layout.tsx:

import { SiteSearch } from "@/components/SiteSearch";

export default function SearchLayout({
  children,
}: {
  children: React.ReactNode;
}) {
  return (
    <div>
      <SiteSearch />
      {children}
    </div>
  );
}

Amend src/app/search/[search]/page.tsx to get the params:

export default async function SearchPageEN(args: any) {
  return (
    <div>
      <h1>/en/search/{args.params.search}</h1>
    </div>
  );
}

let us know if it all works for you.

meszaros-lajos-gyorgy commented 8 months ago

I also ended up creating a fake root element called (root) with nothing more than the html and body tags inside. It's just inconvinient as you have to adjust the paths of every import in all the files inside the app folder.

Update: I've stopped using next.js altogether somewhere in mid-April and I'm just using vanilla react now. There are way too many unanswered questions and way too many issues which don't get addressed. In every release all I see are fixes for Turbopack.

cantdocpp commented 8 months ago

I do agree with comment above, that if your architecture is splitted between route groups without any root layout, then you need to have 1 root layout on your project that's used by your next app.

I think it's a bug that next cannot redirect to different layout

dorji-dev commented 8 months ago

Still not working. I have two root layouts for the info. Next version is 14.1.0. As a workaround for now, I am returning a status from the server action like {status: error | success} and based on that handling the redirection on the client side via useRouter from next/navigation.

arkantoster commented 8 months ago

I think the same problem occurs when you have just one root layout, I've asked about this on stackOverflow https://stackoverflow.com/questions/78029743/how-to-properly-use-redirect-on-nextjs-14

Using Next 14.1.0, but couldn't do any of the workarounds cited here, because i'm trying to keep the page as a server page

ootkin commented 7 months ago

Same here with routing groups and next 14.1.0

smithmanny commented 7 months ago

Having a root layout fixed my problem. My folder looks like this: app -> layout.tsx (I needed to add this root layout) app -> (auth) -> layout.tsx app -> (home) -> layout.tsx

sgalanb commented 7 months ago

Same here. I love all things Vercel and Next.js, but it's very disappointing to see that the app router still has plenty of bugs and unfinished stuff more than a year after its launch.

SpokenSound commented 7 months ago

Can't believe this is still a thing...

guscsales commented 7 months ago

@guscsales is correct, you need the root layout and the other layouts become nested layouts (I think).

Correct, Next.js 14 works now with the Nested layout, to be honest, I think this is not a bug, but a different architecture that comes from a major version.

Some important points:

By default, layouts in the folder hierarchy are nested, which means they wrap child layouts via their children prop.

Only the root layout can contain and tags.

i.e.: you must have a root layout inside "app".

This is the official documentation from Next.js: https://nextjs.org/docs/app/building-your-application/routing/pages-and-layouts#nesting-layouts

marcin-piekarski commented 7 months ago

Just got caught out by this when I tried redirecting from one page to another with both having their own layouts.

Had to add a rootLayout like others have mentioned.

satvik-1203 commented 6 months ago

Or it could be because you don't have the e.preventDefault() action, which gets triggered after redirecting, resulting in the same page refresh.

TomJD commented 6 months ago

Just came across the same issue, still happening on next 14.1.3, same implementation as this comment, when conditionally checking for authentication and trying to redirect back to login.

NoelOmo commented 6 months ago

Will try adding the root layout to my code, but what I have been doing is invoking the redirect function twice. Like literally calling it twice and it seemed to work. Just thought I would check whether there is a better way and alas :D

thedevdavid commented 6 months ago

I don't think it's because of a missing root layout in /app. The docs still say that multiple root layouts are allowed with removing the top-level layout file. https://nextjs.org/docs/app/building-your-application/routing/route-groups#creating-multiple-root-layouts

serhiihaniuk commented 6 months ago

Having a root layout fixed my problem. My folder looks like this: app -> layout.tsx (I needed to add this root layout) app -> (auth) -> layout.tsx app -> (home) -> layout.tsx

I've added layouts like this redirect downloads the page it should redirect to, but doesn't actually change it in the browser

so sad I've spent hours on this

axten commented 5 months ago

same issue here

Edit by maintainer bot: Comment was automatically minimized because it was considered unhelpful. (If you think this was by mistake, let us know). Please only comment if it adds context to the issue. If you want to express that you have the same problem, use the upvote 👍 on the issue description or subscribe to the issue for updates. Thanks!

serhiihaniuk commented 5 months ago

This layout trick helped me with actions, it works. But still doesnt if you use routes.

Had to move auth to the actions, but didnt want to use them

Coder-Devesh-Agarwal commented 5 months ago

The Layout trick works for me and also works with routes

But still I guess I will wait for this thing to become more mature before taking it into production

codePassion-dot commented 5 months ago

I'm experiencing the exact same issue, it's frustrating seeing features marked as stable and after adding them to the codebase notice that everything falls apart. Why any maintainer hasn't say anything yet about this issue ?

codePassion-dot commented 5 months ago

it seems weird that the documentation explicitly says that if you're going to use route groups to have multiple root layouts you should remove the top-level layout.js file but some people here say that adding it is actually what make the redirect works so disappointing.

BadgerBloke commented 4 months ago

It worked for me https://github.com/vercel/next.js/issues/49298#issuecomment-1537433377

RuentDev commented 4 months ago

in my case, when I put it inside of a try-catch block it is not working, but when I remove it it is working fine

alexhwoods commented 4 months ago

in my case, when I put it inside of a try-catch block it is not working, but when I remove it it is working fine

Hey @RuentDev, this is because redirect throws a Next.js specific error. It's kind of part of the "rules of redirects" that you can't put them in a try catch. (See here)

mrlectus commented 3 months ago

I've spent way too many frustrating hours on this. For me, the work around is to just have a common root layout with minimum markup. Project structure: image Example of /app/layout.tsx

export default function RootLayout({ children }: { children: React.ReactNode }) {
  return (
    <html lang="zh">
        <body>{children}</body>
    </html>
  )
}

This works for me as well. For next@14.1.0 when you have group routes defined in the app it's required to use an minimum layout.tsx for work.

Wow this solved my issue, at least they should have mentioned this in the Documentation

mrlectus commented 3 months ago

it seems weird that the documentation explicitly says that if you're going to use route groups to have multiple root layouts you should remove the top-level layout.js file but some people here say that adding it is actually what make the redirect works so disappointing.

Yeah, it was cause of what the doc said that made me remove it, now i added it back and everything works fine