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

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

Open
1 task done
meszaros-lajos-gyorgy opened this issue Nov 9, 2023 · 44 comments
Labels
bug Issue was opened via the bug report template.

Comments

@meszaros-lajos-gyorgy
Copy link

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 #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

  • I verified that the issue exists in the latest Next.js 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 meszaros-lajos-gyorgy added the bug Issue was opened via the bug report template. label Nov 9, 2023
@meszaros-lajos-gyorgy
Copy link
Author

Still not working in 14.0.3

@meszaros-lajos-gyorgy
Copy link
Author

The issue is still present in 14.0.4-canary.18

@mathiasrscom
Copy link

mathiasrscom commented Dec 2, 2023

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
Copy link
Author

Version 14.0.4-canary.39 still broken

@meszaros-lajos-gyorgy
Copy link
Author

meszaros-lajos-gyorgy commented Dec 4, 2023

probably the same issue or very similar: #59217

@muhaimincs
Copy link
Contributor

any work around?

@jmarbutt
Copy link

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

@meszaros-lajos-gyorgy
Copy link
Author

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
Copy link

pinoniq commented Dec 12, 2023

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

@PaskalevStoyan
Copy link

PaskalevStoyan commented Dec 13, 2023 via email

@charandeep7
Copy link

It's still not working

@carolinevsboliveira
Copy link

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

@logemann
Copy link

logemann commented Dec 25, 2023

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
Copy link
Author

meszaros-lajos-gyorgy commented Dec 29, 2023

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
Copy link

mr-14 commented Jan 5, 2024

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
Copy link
Author

Still broken in v14.0.5-canary.43

@innobrightcafe
Copy link

wow, same issue. cant use with condition

@AndreyKornyusko
Copy link

still not working

@logemann
Copy link

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

@guscsales
Copy link

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
Copy link

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
Copy link
Author

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

@iamgmd
Copy link

iamgmd commented Feb 12, 2024

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

@adamchipperfield
Copy link

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

@iamgmd
Copy link

iamgmd commented Feb 14, 2024

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
Copy link

iamgmd commented Feb 14, 2024

@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
Copy link
Author

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.

@cantdocpp
Copy link

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
Copy link

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

This comment was marked as resolved.

@ootkin
Copy link

ootkin commented Mar 1, 2024

Same here with routing groups and next 14.1.0

@smithmanny
Copy link

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
Copy link

sgalanb commented Mar 8, 2024

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
Copy link

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

@guscsales
Copy link

@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
Copy link

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
Copy link

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
Copy link

TomJD commented Mar 24, 2024

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
Copy link

NoelOmo commented Apr 9, 2024

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

kimsohyuncans added a commit to humblFINANCE/humblFINANCE-frontend that referenced this issue Apr 14, 2024
the issue already discussed in this GH issue
vercel/next.js#58263

 not sure whether its
bug or the  new major architecture on next 14
@thedevdavid
Copy link

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
Copy link

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

This comment has been minimized.

@serhiihaniuk
Copy link

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue was opened via the bug report template.
Projects
None yet
Development

No branches or pull requests