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

revalidatePath() not working as expected for Parallel Routes, or for Parallel + Intercepting Routes (modal) #54173

Closed
1 task done
magoz opened this issue Aug 17, 2023 · 32 comments · Fixed by #59585
Closed
1 task done
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

@magoz
Copy link

magoz commented Aug 17, 2023

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
      Platform: darwin
      Arch: x64
      Version: Darwin Kernel Version 21.6.0: Thu Jun  8 23:57:12 PDT 2023; root:xnu-8020.240.18.701.6~1/RELEASE_X86_64
    Binaries:
      Node: 20.5.1
      npm: 9.8.0
      Yarn: 1.22.19
      pnpm: 8.6.12
    Relevant Packages:
      next: 13.4.17
      eslint-config-next: N/A
      react: 18.2.0
      react-dom: 18.2.0
      typescript: N/A
    Next.js Config:
      output: N/A

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

App Router

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

https://github.com/magoz/revalidatepath-parallel-routes-bug

To Reproduce

Click on Parallel+Intercepting Modal.

Describe the Bug

When clicking on the Trigger Server Action button, the server action is triggered just by fetching a new random number and passing it to a client component.

It works perfectly for the Home Page, and a Leaf Page.

But for a Page that uses Parallel + Intercepting Routes (modal), revalidatePath('/parallel-intercepting') seems to fetch the new data, and the component appears to receive the new props (see client console), but it doesn't re-render.

For a page that uses Parallel Routes (modal without Intercepting), works as expected on hard navigation, but on soft navigation we get the same issue than the Parallel + Intercepting Routes case.

Expected Behavior

revalidatePath() to work as expected for Parallel Routes, and for Parallel + Intercepting Routes.

Which browser are you using? (if relevant)

Chrome 115.0.5790.170

How are you deploying your application? (if relevant)

pnpm dev

NEXT-1706

@magoz magoz added the bug Issue was opened via the bug report template. label Aug 17, 2023
@kght6123
Copy link

kght6123 commented Sep 7, 2023

I too was plagued by this bug.
I temporarily added the following settings to work around it, but the cache is no longer meaningful.

export const dynamic = "force-dynamic";

@adelegard
Copy link

@kght6123 that doesn't seem to fix it for me. Tried with @magoz example too - and doesn't work in there either.

@grzegorzpokorski
Copy link

grzegorzpokorski commented Sep 23, 2023

Hi, I also have a problem with revalidatePath invoked in server action which is executed on the page which is available in intercepted and pararel route. I also execute server action with revalidatePath inside 'startTransition' function from useTransition hook. In folowing reproduction link you can notice that value of flag that tells you whether there is a pending transition is once set to true, but when data is successfully fetched inside server action this flag from useTransition does not inform about it - insted inform that this function is still pending.
Invoking revalidatePath in server action in pararell + intercepted route cause "demage" of navigation - when you want to change route or go backward of your browsing history to close intercepted "modal", intercepted route does not hide etc. Please check this reproduction of bugs. which I notice.

@grzegorzpokorski
Copy link

Hi, I also have a problem with revalidatePath invoked in server action which is executed on the page which is available in intercepted and pararel route. I also execute server action with revalidatePath inside 'startTransition' function from useTransition hook. In folowing reproduction link you can notice that value of flag that tells you whether there is a pending transition is once set to true, but when data is successfully fetched inside server action this flag from useTransition does not inform about it - insted inform that this function is still pending. Invoking revalidatePath in server action in pararell + intercepted route cause "demage" of navigation - when you want to change route or go backward of your browsing history to close intercepted "modal", intercepted route does not hide etc. Please check this reproduction of bugs. which I notice.

It also occurs when in server action I use cookies() from next/headers

@adelegard
Copy link

I also find that this same problem exists for revalidateTag.

@grzegorzpokorski
Copy link

#55911

@kght6123
Copy link

@adelegard sorry. It certainly doesn't seem to be working.

@ats1999
Copy link

ats1999 commented Oct 26, 2023

revalidation is breaking in all way!

kght6123 added a commit to kght6123/ors that referenced this issue Oct 29, 2023
@adelegard
Copy link

Any update on this? This bug continues to cause us pain and having to do weird hacky things to work around it.

@tinleym
Copy link

tinleym commented Oct 30, 2023

Given that parallel routes are the recommended way to render modals (https://nextjs.org/docs/app/building-your-application/routing/parallel-routes#modals), this bug breaks the recommended way to submit forms in modals.

@adelegard
Copy link

Given that parallel routes are the recommended way to render modals (https://nextjs.org/docs/app/building-your-application/routing/parallel-routes#modals), this bug breaks the recommended way to submit forms in modals.

Totally. I don't understand how the community isn't freaking out about this tbh.

@balazsorban44 balazsorban44 added linear: next Confirmed issue that is tracked by the Next.js team. Navigation Related to Next.js linking (e.g., <Link>) and navigation. labels Nov 2, 2023
@master4o
Copy link

seems like a main issue now, cuz block using forms in modals

@Xananax
Copy link

Xananax commented Nov 23, 2023

Is there any workaround to this?

export const dynamic = "force-dynamic"; does not work in server components.

I am not really going to bring anything new, but I'll explain my setup below, in case someone else ends up here and wants to know if their bug is similar to the one explained here.

My setup
  • auth/[slug]/page.tsx loads the logged in user, then loads login/signup pages
  • @modal/(.)auth/[slug]/page.tsx does the same in a modal

This seems to be the setup recommended in the docs. It breaks in weird ways.

At first, everything seems to work as intended. I am receiving the form errors I send back from my action:
image

They do get displayed
image

I have a console.log that reads the formState result, and it shows up:
image

This is all working as expected


However if the form submits (data is validated), then the form stops updating.

I do receive the proper response from the server:
image

But the form stops updating, no console.log shows up.

It seems the form is incapable of updating after these lines in my server action:

  const { error } = await supabase.auth.signUp({
    email,
    password,
    options: {
      emailRedirectTo,
    },
  });

If I return anything before those lines, everything works; anything after those lines, nothing happens.

I'm using Supabase SSR, and it modifies cookies.

Manually adding a revalidatePath("/auth/signup") didn't change anything


It really looks like the recommended way doesn't work for user authentication, despite that being both an example in the docs, and the main motivation to use parallel routes to my eyes.

If anyone has a workaround, I'd be happy to know.

@master4o
Copy link

master4o commented Nov 23, 2023

Is there any workaround to this?

export const dynamic = "force-dynamic"; does not work in server components.

I am not really going to bring anything new, but I'll explain my setup below, in case someone else ends up here and wants to know if their bug is similar to the one explained here.

My setup

  • auth/[slug]/page.tsx loads the logged in user, then loads login/signup pages
  • @modal/(.)auth/[slug]/page.tsx does the same in a modal

This seems to be the setup recommended in the docs. It breaks in weird ways.

At first, everything seems to work as intended. I am receiving the form errors I send back from my action: image

They do get displayed image

I have a console.log that reads the formState result, and it shows up: image

This is all working as expected

However if the form submits (data is validated), then the form stops updating.

I do receive the proper response from the server: image

But the form stops updating, no console.log shows up.

It seems the form is incapable of updating after these lines in my server action:

  const { error } = await supabase.auth.signUp({
    email,
    password,
    options: {
      emailRedirectTo,
    },
  });

If I return anything before those lines, everything works; anything after those lines, nothing happens.

I'm using Supabase SSR, and it modifies cookies.

Manually adding a revalidatePath("/auth/signup") didn't change anything

It really looks like the recommended way doesn't work for user authentication, despite that being both an example in the docs, and the main motivation to use parallel routes to my eyes.

If anyone has a workaround, I'd be happy to know.

I used bad, but working idea.

  1. implement useFormState.
  2. In Server Action return object with message: 'ok', if response status.ok
  3. if (loginState?.message === "ok" && sentCode) {
    setAuthCookie({ token: loginState.token });
    in modal component if mesage in state from useFormState === 'ok', invoke second separated server action only with cookies logic and then router.back()

It works for me, but it is very dirty solution. Need to wait issue resolve :(

@Xananax
Copy link

Xananax commented Nov 24, 2023

Oh wow, that's pretty smart! But yea, it doesn't feel good.

@dclark27
Copy link

dclark27 commented Dec 7, 2023

Also need a fix to this! Our forms in parallel routes are breaking our navigation.

Screen.Recording.2023-12-07.at.12.47.47.PM.mov

related: #58201
related: #58180

@tsnieman
Copy link

tsnieman commented Dec 8, 2023

I'm experiencing this bug and it presents pretty much exactly like @dclark27's video above.

This is a pretty huge bug with some very key features of nextjs — who can we tag to look into this?

tsnieman added a commit to tsnieman/nextjs-parallel-route-pain that referenced this issue Dec 8, 2023
@tsnieman
Copy link

tsnieman commented Dec 8, 2023

Here's as minimal reproduction as I could make:
🐛 https://github.com/tsnieman/nextjs-parallel-route-pain/
it's currently using next@14.0.5-canary.2

Please let me know if there's more that i can provide. this bug is a real pain that my team doesn't have a workaround for. EDIT — the best workaround i've found is to use something like router.push(`?ts=${Date.now()}`) — and be sure that you dont use router.refresh(), revalidatePath, etc. The key is to coerce the page into updating rather than explicitly asking the page to update.

and here's a video of the bug in action:

Kapture.2023-12-08.at.12.43.41.mp4

tsnieman added a commit to tsnieman/nextjs-parallel-route-pain that referenced this issue Dec 9, 2023
tsnieman added a commit to tsnieman/nextjs-parallel-route-pain that referenced this issue Dec 9, 2023
tsnieman added a commit to tsnieman/nextjs-parallel-route-pain that referenced this issue Dec 9, 2023
@esemczak
Copy link

This issue absolutely persists and can be easily reproduced. Can we have at least acknowledgement from the team that they are aware and working on it? Parallel routes are completely useless in current state.

@numman-ali
Copy link

There is a commit that went in recently by @ztanner (thank you!) which mentions the nextUrl in the header can cause issues with server actions
#59175

Perhaps a solution in the same area could be found for revalidateTag and and revalidatePath? @ztanner do you have any thoughts?

@feedthejim
Copy link
Contributor

we're definitely investigating :)

@numman-ali
Copy link

we're definitely investigating :)

Thank you @feedthejim 🙏
I'm happy to help in any way, just shout and I'm there

ztanner added a commit that referenced this issue Dec 15, 2023
…59585)

### What?
There are a bunch of different bugs caused by the same underlying issue,
but the common thread is that performing any sort of router cache update
(either through `router.refresh()`, `revalidatePath()`, or `redirect()`)
inside of a parallel route would break the router preventing subsequent
actions, and not resolve any pending state such as from `useFormState`.

### Why?
`applyPatch` is responsible for taking an update response from the
server and merging it into the client router cache. However, there's
specific bailout logic to skip over applying the patch to a
`__DEFAULT__` segment (which corresponds with a `default.tsx` page).
When the router detects a cache node that is expected to be rendered on
the page but contains no data, the router will trigger a lazy fetch to
retrieve the data that's expected to be there
([ref](https://github.com/vercel/next.js/blob/5adacb69126e0fd7dff7ebd45278c0dfd42f6116/packages/next/src/client/components/layout-router.tsx#L359-L370))
and then update the router cache once the data resolves
([ref](https://github.com/vercel/next.js/blob/5adacb69126e0fd7dff7ebd45278c0dfd42f6116/packages/next/src/client/components/layout-router.tsx#L399-L404)).

This is causing the router to get stuck in a loop: it'll fetch the data
for the cache node, send the data to the router reducer to merge it into
the existing cache nodes, skip merging that data in for `__DEFAULT__`
segments, and repeat.

### How?
We currently assign `__DEFAULT__` to have `notFound()` behavior when
there isn't a `default.tsx` component for a particular segment. This
makes it so that when loading a page that renders a slot without slot
content / a `default`, it 404s. But when performing a client-side
navigation, the intended behavior is different: we keep whatever was in
the `default` slots place, until the user refreshes the page, which
would then 404.

However, this logic is incorrect when triggering any of the above
mentioned cache node revalidation strategies: if we always skip applying
to the `__DEFAULT__` segment, slots will never properly handle reducer
actions that rely on making changes to their cache nodes.

This splits these different `applyPatch` functions: one that will apply
to the full tree, and another that'll apply to everything except the
default segments with the existing bailout condition.

Fixes #54173
Fixes #58772
Fixes #54723
Fixes #57665

Closes NEXT-1706
Closes NEXT-1815
Closes NEXT-1812
@numman-ali
Copy link

Hey @ztanner thanks for putting the latest commit in, really appreciated 🙏
I've pulled the latest canary that contains your changes but still doesn't seem to working.
I've tested with router.refresh client side and a revalidateTag/Path on a server action and both times the 404 page is called
Any thoughts on what might be happening?

@feedthejim
Copy link
Contributor

@numman-ali do you have a reproduction?

@sinafath
Copy link

sinafath commented Dec 20, 2023

yeah the issue still exists in 14.0.5-canary.18 @feedthejim

@bosheski
Copy link

Issue still exists in 14.0.5-canary.22 @feedthejim

@jeremy-code
Copy link

@feedthejim

I am not certain whether this is the same issue @bosheski, @sinafath, and @numman-ali are experiencing, but this is my minimal reproduction of an issue with intercepting routes on 14.0.5-canary.29: jeremy-code/next-intercepting-bug.

next-intercepting-bug

The server action calls revalidatePath("/"). When there is no intercepting route, running the server action from the default page works as intended. When there is an intercepting route, running the server action from either the modal or the default page renders the 404 page.

Note that removing app/[slug]/page.tsx results in the same behavior, I included it to show that the 404 page it returns is not the result of the server rendering the default page at the non-intercepted route.

I imagine the expected behavior would be for the current context's data to be reloaded, but not the route, or that the modal remains rendered rather than the non-intercepted page.

Let me know if there's anything I need to clarify.

@stefan-joseph
Copy link

The most recent update fixed the issue on the default page for the form, however, the bug still persists when submitting the form from a modal using parallel routing.

@jasonbert
Copy link

jasonbert commented Jan 1, 2024

EDIT: Looks like this might be the same issue as #58715


I think this behaviour may be related to the issues described here. I've got a modal displaying a blog post coming from a parallel/intercepted route, which has a "favourite" (heart) toggle button that calls a server action and revalidates the cache by tag. When "closing" the modal (router.back()), the route changes but doesn't remove the modal.

  1. Trigger modal/parallel/intercepted route by clicking on blog link (next/link).
    image

  2. Click on "favourite" toggle button, triggers server action and revalidates by tag.
    image

  3. Click on "close" (X) button to close the modal (router.back()). The route has changed, but the modal hasn't been removed.
    image

Modal Component

'use client'

import { useRouter } from "next/navigation";
import React, { MouseEventHandler, PropsWithChildren, useCallback, useEffect, useRef } from 'react';
import { Heading2 } from '../heading';
import styles from './index.module.scss';

interface ModalProps extends PropsWithChildren {
    header?: string | React.ReactNode
    footer?: React.ReactNode
    onClose?: () => void
}

export default function Modal({ children, header, footer, onClose }: ModalProps) {
    const overlay = useRef(null);

    const handleClose = useCallback(() => {
        if (onClose) onClose();
    }, [onClose]);

    const handleClick: MouseEventHandler = useCallback((e) => {
        if (onClose && e.target === overlay.current) {
            onClose();
        }
    }, [onClose]);

    const handleKeyDown = useCallback((e: KeyboardEvent) => {
        if (onClose && e.key === "Escape") onClose();
    }, [onClose]);

    useEffect(() => {
        document.addEventListener("keydown", handleKeyDown);
        return () => document.removeEventListener("keydown", handleKeyDown);
    }, [handleKeyDown]);

    return (
        <div ref={overlay} className={styles.Modal} onClick={handleClick}>
            <div className={styles.Modal__container}>
                <div className={styles.Modal__header}>
                    {header &&
                        <div className={styles.Modal__headerContent}>
                            {typeof header === 'string' ?
                                <Heading2>{header}</Heading2> :
                                header
                            }
                        </div>
                    }
                    <div className={styles.Modal__headerClose}>
                        <button onClick={handleClose} className={styles.Modal__headerCloseButton}>X</button>
                    </div>
                </div>
                <div className={styles.Modal__content}>
                    {children}
                </div>
                {footer &&
                    <div className={styles.Modal__footer}>
                        <div className={styles.Modal__footerContent}>
                            {footer}
                        </div>
                    </div>
                }
            </div>
        </div>
    )
}

export function RouterModal(props: ModalProps) {
    const router = useRouter();
    const handleClose = useCallback(() => {
        if (props.onClose) {
            props.onClose();
        }

        router.back();
    }, [router]);

    return <Modal {...props} onClose={handleClose} />
}

Action

'use server'

export async function toggleBlogFavoriteAction(blogId: string) {
    await toggleBlogFavorite(blogId);

    revalidateTag(BLOG_FAVORITES_CACHE_TAG);
}

Component

'use client';

import { startTransition, useOptimistic } from "react";
import { PrimaryButton } from "~f/framework/button";
import { toggleBlogFavoriteAction } from "./blogAction";

export default function BlogFavoriteToggleButton({ blogId, isFavorite }: { blogId: string, isFavorite: boolean }) {
    const [optimisticIsFavorite, setOptimisticIsFavorite] = useOptimistic(isFavorite);

    async function toggle() {
        startTransition(() => {
            setOptimisticIsFavorite(!optimisticIsFavorite);
        });

        await toggleBlogFavoriteAction(blogId);
    };

    return (
        <PrimaryButton onClick={toggle} style={{ minWidth: 50 }}>{optimisticIsFavorite ? '❤️' : '🖤'}</PrimaryButton>
    )
}

Source: https://github.com/scorado-ltd/scorado-examples-nextjs-cosmos
Next.js: 14.0.4

@jeremy-code
Copy link

jeremy-code commented Jan 3, 2024

I have discovered where the 404 page is coming from that I mentioned in my previous comment. It looks like Next.js is trying to load the default component (in my case, app/default.tsx) at the root. If there is any function at all, it now loads the modal (but not app/page.tsx). If default.tsx does return a component, it will load that in addition to the modal. The server action continues to run like before, but without any errors. I have updated my reproduction repository to demonstrate what I mean at jeremy-code/next-intercepting-bug.

For anyone wanting a quick fix, you can just use redirect to get to the original page. For me, it only rendered the root layout and not other layouts, so I tried manually importing each page + layout and their props, but it was a bit messy since Default didn't receive any props. I suspect the POST request having the header next-url': '/children/modal' may be involved in some way, but not sure.

Here is a gif of what I am referring to:
CleanShot 2024-01-03 at 14 22 43

agustints pushed a commit to agustints/next.js that referenced this issue Jan 6, 2024
…ercel#59585)

### What?
There are a bunch of different bugs caused by the same underlying issue,
but the common thread is that performing any sort of router cache update
(either through `router.refresh()`, `revalidatePath()`, or `redirect()`)
inside of a parallel route would break the router preventing subsequent
actions, and not resolve any pending state such as from `useFormState`.

### Why?
`applyPatch` is responsible for taking an update response from the
server and merging it into the client router cache. However, there's
specific bailout logic to skip over applying the patch to a
`__DEFAULT__` segment (which corresponds with a `default.tsx` page).
When the router detects a cache node that is expected to be rendered on
the page but contains no data, the router will trigger a lazy fetch to
retrieve the data that's expected to be there
([ref](https://github.com/vercel/next.js/blob/5adacb69126e0fd7dff7ebd45278c0dfd42f6116/packages/next/src/client/components/layout-router.tsx#L359-L370))
and then update the router cache once the data resolves
([ref](https://github.com/vercel/next.js/blob/5adacb69126e0fd7dff7ebd45278c0dfd42f6116/packages/next/src/client/components/layout-router.tsx#L399-L404)).

This is causing the router to get stuck in a loop: it'll fetch the data
for the cache node, send the data to the router reducer to merge it into
the existing cache nodes, skip merging that data in for `__DEFAULT__`
segments, and repeat.

### How?
We currently assign `__DEFAULT__` to have `notFound()` behavior when
there isn't a `default.tsx` component for a particular segment. This
makes it so that when loading a page that renders a slot without slot
content / a `default`, it 404s. But when performing a client-side
navigation, the intended behavior is different: we keep whatever was in
the `default` slots place, until the user refreshes the page, which
would then 404.

However, this logic is incorrect when triggering any of the above
mentioned cache node revalidation strategies: if we always skip applying
to the `__DEFAULT__` segment, slots will never properly handle reducer
actions that rely on making changes to their cache nodes.

This splits these different `applyPatch` functions: one that will apply
to the full tree, and another that'll apply to everything except the
default segments with the existing bailout condition.

Fixes vercel#54173
Fixes vercel#58772
Fixes vercel#54723
Fixes vercel#57665

Closes NEXT-1706
Closes NEXT-1815
Closes NEXT-1812
@stefcot
Copy link

stefcot commented Jan 10, 2024

I have quite a complicated folder structure on a project I am working on and indeed, the commit seems to work: revalidatePath refreshes data from the parallel route containing the modal. Hope it will last! Many thanks.

However, the canary version causes errors impacting the project CI involving version mismatch with storybook and as a consequence, I'm not able to deploy.

kumar-kanujia added a commit to kumar-kanujia/geek-chat that referenced this issue Jan 16, 2024
Parllel route not working with revalidte or redirect
sol: temp fix not revalidate for parllel route
vercel/next.js#54173
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 Jan 26, 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.