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

router.refresh() causes intercepted route to switch to the intercepting route/modal #60844

Closed
1 task done
xvvvyz opened this issue Jan 18, 2024 · 4 comments · Fixed by #63263
Closed
1 task done

router.refresh() causes intercepted route to switch to the intercepting route/modal #60844

xvvvyz opened this issue Jan 18, 2024 · 4 comments · Fixed by #63263
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

@xvvvyz
Copy link

xvvvyz commented Jan 18, 2024

Link to the code that reproduces this issue

https://github.com/vercel/nextgram

To Reproduce

  1. Add code to router.refresh() on the intercepted route:
diff --git a/app/photos/[id]/page.tsx b/app/photos/[id]/page.tsx
index 4085ee7..45b6027 100644
--- a/app/photos/[id]/page.tsx
+++ b/app/photos/[id]/page.tsx
@@ -1,7 +1,18 @@
+'use client';
+
+import { useRouter } from 'next/navigation';
+
 export default function PhotoPage({
   params: { id },
 }: {
   params: { id: string };
 }) {
-  return <div className="card">{id}</div>;
+  const router = useRouter();
+
+  return (
+    <div className="card">
+      {id}
+      <button onClick={router.refresh}>Refresh</button>
+    </div>
+  );
 }
  1. Click the refresh button.

Current vs. Expected behavior

I expect the route to stay the same on router.refresh().

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.2.0: Wed Nov 15 21:53:34 PST 2023; root:xnu-10002.61.3~2/RELEASE_ARM64_T8103
Binaries:
  Node: 21.6.0
  npm: 10.2.4
  Yarn: N/A
  pnpm: N/A
Relevant Packages:
  next: 14.0.5-canary.67
  eslint-config-next: N/A
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 5.3.3
Next.js Config:
  output: N/A

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

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

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

next dev (local), next build (local), next start (local), Vercel (Deployed)

Additional context

Safari.mp4

NEXT-2737

@xvvvyz xvvvyz added the bug Issue was opened via the bug report template. label Jan 18, 2024
@github-actions github-actions bot added the Navigation Related to Next.js linking (e.g., <Link>) and navigation. label Jan 18, 2024
@damnsamn
Copy link

This remains true of Next 14.1.

Additionally, this issue also occurs after a Fast Refresh caused by a file save while on the intercepted route.

@ghost
Copy link

ghost commented Jan 19, 2024

Adding it to the bag of appRouter hard vs soft navigation on parallel routes? (ie: #60815 and #60814)

@denexapp
Copy link

denexapp commented Jan 30, 2024

I'm just so frustrated with intercepted routes... I think i've ran into another bug, but i'm not sure if it's a bug or it's me being dumb, and i don't even know if it makes sense to report it.

If anyone has implemented authentication modals with intercepting routes and server actions, please share!

@samcx samcx added the linear: next Confirmed issue that is tracked by the Next.js team. label Mar 7, 2024
ztanner added a commit that referenced this issue Mar 19, 2024
…63263)

### What
Actions that revalidate the router state by kicking off a refetch (such
as `router.refresh()` or dev fast refresh) would incorrectly trigger an
interception route if one matched the current URL, or in the case of
already being on an intercepted route, would trigger the full detail
page instead.

### Why
Interception rewrites use the `nextUrl` header to determine if the
requested path should be rewritten to a different path. We currently
forward that header indiscriminately, which means that if you were on a
non-intercepted route and called `router.refresh()`, the UI would change
to the intercepted content instead (since it would treat it the same as
a soft-navigation).

### How
This updates various reducers to only forward the `nextUrl` header if
there's an interception route present in the tree. If there is, we want
to refresh its data, rather than the data for the underlying page. The
reverse is also true: if we were on the "full" page, and triggered a
`router.refresh()`, we won't forward `nextUrl` meaning it won't fetch
the interception data.

In order to determine if an interception route is present in the tree, I
had to add a new segment type for dynamic interception routes, as by the
time they reach the client they are stripped of their interception
marker.

**Note: There are a series of bugs related to `router.refresh` with
parallel/interception routes, such as the previous page/slot content
disappearing when triggering a refresh. This does not address all of
those cases, but I'm working through them!**

Fixes #60844
Fixes #62470
Closes NEXT-2737
Copy link
Contributor

github-actions bot commented Apr 3, 2024

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 added the locked label Apr 3, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 3, 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.

4 participants