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

middleware redirect doesn't change the url on router.refresh() - App Router #65970

Closed
Saman-Safaei-Dev opened this issue May 20, 2024 · 2 comments · Fixed by #65999
Closed

middleware redirect doesn't change the url on router.refresh() - App Router #65970

Saman-Safaei-Dev opened this issue May 20, 2024 · 2 comments · Fixed by #65999
Labels
bug Issue was opened via the bug report template. locked Middleware Related to Next.js Middleware Navigation Related to Next.js linking (e.g., <Link>) and navigation.

Comments

@Saman-Safaei-Dev
Copy link

Link to the code that reproduces this issue

https://github.com/Saman-Safaei-Dev/next-bug

To Reproduce

1- create a middleware to redirect from "/" to "/dashboard" with a condition ( for example when a cookie exists )
2- navigate to "/" and make this condition true, then call router.refresh()
3- you are in dashboard page but look at the url

Current vs. Expected behavior

current: the url remains the "/"
expected: the url should change to "/dashboard"

Provide environment information

Operating System:
  Platform: Windows 11
  Arch: x64
  Version: 23H2
Binaries:
  Node: 18.20.2
  npm: 10.5.0
Relevant Packages:
  next: 14.2.3
  next-intl: 3.10.0
  react: 18.3.1
  react-dom: 18.3.1
  typescript: 5.4.3

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

Middleware, Navigation

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

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

Additional context

No response

@Saman-Safaei-Dev Saman-Safaei-Dev added the bug Issue was opened via the bug report template. label May 20, 2024
@github-actions github-actions bot added Middleware Related to Next.js Middleware Navigation Related to Next.js linking (e.g., <Link>) and navigation. labels May 20, 2024
@gaojude
Copy link
Contributor

gaojude commented May 20, 2024

Should be fixed by #65999

ztanner added a commit that referenced this issue May 21, 2024
Fixes #65970

----

The browser update happens at
https://github.com/vercel/next.js/blob/10d5c278bc0e9eab067df7b5f3943c12ab54a6a9/packages/next/src/client/components/app-router.tsx#L685,
which sets the browser location to `canonicalUrl`. `canonicalUrl` was
correctly set at
https://github.com/vercel/next.js/blob/ab03c3261f53fcff1fb3387673cfd170a626564c/packages/next/src/client/components/router-reducer/reducers/refresh-reducer.ts#L98,
but then mistakenly overriden at
https://github.com/vercel/next.js/blob/51549d92de3069b6dc5cd1f5fcdc04d5b1cbf37a/packages/next/src/client/components/router-reducer/reducers/refresh-reducer.ts#L129.

This PR fixes that and includes an E2E test to prevent future
regression.

---------

Co-authored-by: JJ Kasper <jj@jjsweb.site>
Co-authored-by: Zack Tanner <1939140+ztanner@users.noreply.github.com>
Copy link
Contributor

github-actions bot commented Jun 5, 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 Jun 5, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 5, 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. locked Middleware Related to Next.js Middleware Navigation Related to Next.js linking (e.g., <Link>) and navigation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants