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.

Sign up for GitHub

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

Jump to bottom

Hash is removed when linking from app router to pages router #56212

Closed
1 task done
blurrah opened this issue Sep 29, 2023 · 2 comments · Fixed by #56223
Closed
1 task done

Hash is removed when linking from app router to pages router #56212

blurrah opened this issue Sep 29, 2023 · 2 comments · Fixed by #56223
Labels
bug Issue was opened via the bug report template. locked Navigation Related to Next.js linking (e.g., <Link>) and navigation.

Comments

@blurrah
Copy link
Contributor

blurrah commented Sep 29, 2023

Link to the code that reproduces this issue

https://github.com/blurrah/nextjs-app-page-router-hash-issue-reproduction

To Reproduce

  1. Start the application in development mode
  2. Click on the issue link

Current vs. Expected behavior

Following the steps from the previous section I expect the hash in the link to be available, but it's being removed when switching from the app router to the pages router. It does however keep the hash when switching from pages router to the app router.

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 22.5.0: Thu Jun  8 22:22:20 PDT 2023; root:xnu-8796.121.3~7/RELEASE_ARM64_T6000
Binaries:
  Node: 20.5.1
  npm: 9.8.0
  Yarn: 1.22.19
  pnpm: 8.5.0
Relevant Packages:
  next: 13.5.4-canary.7
  eslint-config-next: N/A
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 5.1.3
Next.js Config:
  output: N/A

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

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

Additional context

I see this happening on a production site running 13.4.12 and on the latest canary.
@blurrah blurrah added the bug Issue was opened via the bug report template. label Sep 29, 2023
@github-actions github-actions bot added the Navigation Related to Next.js linking (e.g., <Link>) and navigation. label Sep 29, 2023
@blurrah
Copy link
Contributor Author

blurrah commented Sep 29, 2023

Did a quick check where the culprit lies:

The url without hash is being used to get the correct prefetchValues from prefetchCache in the navigateReducer. That data is then being used as the external url when switching to the pages router. Will try to send a PR with the fix and a test case

blurrah added a commit to blurrah/next.js that referenced this issue Sep 29, 2023
@blurrah
fix(next): keep hash when navigating from app to pages
6db6e8b 
Fixes vercel#56212
@blurrah blurrah mentioned this issue Sep 29, 2023
Merged
ijjk added a commit that referenced this issue Oct 2, 2023
@blurrah @ztanner @ijjk
fix(next/client): keep hash when navigating from app to pages router (#…
a2f9ef5 
…56223)

### What?

Fixes the pages router not receiving a hash when being linked from the
app router.

### Why?

The hash being removed breaks sites that rely on it for client side
features.

### How?

The hash gets omitted from the URL when used as a key for the
preflightCache. Once it's clear that it's an external URL and that it's
not empty we can use the initial href to send the hash as well.

Not completely sure if there's an edge case that might break, I added an
extra check for when the hash is only used to scroll the page.

This might need an additional test case just for
`navigate-reducer.test.tsx`.

Fixes #56212

---------

Co-authored-by: Zack Tanner <[email protected]>
Co-authored-by: JJ Kasper <[email protected]>
@github-actions
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 Oct 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
bug Issue was opened via the bug report template. locked Navigation Related to Next.js linking (e.g., <Link>) and navigation.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(next/client): keep hash when navigating from app to pages router blurrah/next.js
1 participant
@blurrah