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

No next/link prefetch and client-side navigation for rewritten routes with query parameters #35696

Closed
1 task done
dklymenk opened this issue Mar 29, 2022 · 9 comments
Closed
1 task done
Labels
bug Issue was opened via the bug report template. locked stale The issue has not seen recent activity.

Comments

@dklymenk
Copy link

Verify canary release

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

Provide environment information

    Operating System:
      Platform: linux
      Arch: x64
      Version: #1 SMP PREEMPT Mon, 21 Mar 2022 22:59:40 +0000
    Binaries:
      Node: 16.13.1
      npm: 8.1.2
      Yarn: 1.22.17
      pnpm: N/A
    Relevant packages:
      next: 12.1.3-canary.0
      react: 17.0.2
      react-dom: 17.0.2

What browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

next start

Describe the Bug

If you have an optional catch-all route and use rewrites to convert query parameters into route parameters, links that have those query parameters don't automatically prefetch the props json even when the json is pregenerated at built time.

Expected Behavior

Prefetch and client-side navigation should work for links/routes with query params the same way as for any other internal link. No page reload, etc.

To Reproduce

I have prepared a minimal repository where the issue is reproduced. https://github.com/dklymenk/next-query-rewrite-router-issue

See my last commit to get the idea of what it takes to reproduce the issue starting from a clean template.

Here are some must-haves
  1. Delete the default index.tsx page.
  2. Create an optional catch-all page file [[...slug]] to catch our potential parameters.
  3. Add getStaticPaths and getStaticProps to enable SSG.
  4. Add some links with query params.
  5. Make sure you render the query param props on the page.
  6. Add a rewrite to next.config.js to transform query params into route params /?s=query1 to /query1, etc.

yarn build, yarn start and go to localhost:3000.

You can open dev tools on the network to see that there is no prefetch happening and if you click on any of the links with query params the page will reload (no client-side navigation).

Interestingly enough, if you remove the / on the href, the preload works (you can see json preloaded on network tab), but clicking on the link seems to load only the index.json (props for the case with no query params).

@dklymenk dklymenk added the bug Issue was opened via the bug report template. label Mar 29, 2022
@tarizer
Copy link

tarizer commented Aug 23, 2022

I am having the same issues but without a "catch-all route".

When I hover over a link for rewritten routes, the JSON is prefetched and cached without the query parameters.

When I click on the same link, the query parameters [path to the file] are added to the JSON, so it doesn't use the cached version and download the file again (200).

Example:

When I hover the link for /pages/[id]/[name] I get:
john.json with a 200 status and 304 status for the following hover.

When I click on the same link I get:
john.json?id=5&name=john with a 200 status for the same file (GET and HEAD)

If I don't use "rewrites", I get only a 200 status for HEAD and the file is not downloaded again.

I am using version 12.2.5

@msampsontruecar
Copy link

I'm seeing this issue when using getStaticProps with rewrites.
I think it was caused by this change 7584b02#diff-e26f57b24cd54570c8cfbef250f5986c291e999f33b011f6c783f230a06e35e3L157

Nextjs use to pass in ssg to getDataHref and only add the search string to the end of the data href when !ssg. But starting with that commit we always add the search string and that causes the extra query params on the end of the url. The url then doesn't match what was requested during prefetch.

@tarizer
Copy link

tarizer commented Sep 9, 2022

I'm seeing this issue when using getStaticProps with rewrites. I think it was caused by this change 7584b02#diff-e26f57b24cd54570c8cfbef250f5986c291e999f33b011f6c783f230a06e35e3L157

Nice catch. I hope they fix it soon.

@msampsontruecar
Copy link

Prefetch doesn't have this problem because when getDataHref is call for ssg prefetch the query string is not included in the url.

I'm not sure what the best solution is. Maybe you could add that ssg arg back to getDataHref.

In that commit there are some changes in packages/next/shared/lib/router/router.ts that deal with removing any path params from the query string so there are not duplicates. See changes to omitParmsFromQuery and omit. That seems like it could be related.

For the example of pages/[name] fetching john.json?name=john the name is clearly duplicate info and should be excluded. But in the case of a fetch on click is there a situation where you might want a different query param included? john.json?lastName=johnson? I don't think so but I can't tell from the above PR messages what was being solved by always including the search string even for ssg.

@dennismuench
Copy link

I have the same problem with auth redirects inside the middleware. The redirects are getting cached on prefetch and when you navigate to a cached url the entire website hangs. Sadly you cannot really turn off prefetching so you have to redesign everything in a way that a user cannot accidentally hover over a link that could lead to redirected urls... . That is really bad. Ideally the prefetch mechanism should honor the status codes in the responses. I tried with 403 for example but that in turn crashes the server. Any workaround?

@dennismuench
Copy link

For anyone stumbling upon this. It is possible to disable the entire prefetching system which currently is the only way of using redirects in middleware if you are not using the app folder:
#24437 (comment)

@nextjs-bot
Copy link
Collaborator

This issue has been automatically marked as stale due to two years of inactivity. It will be closed in 7 days unless there’s further input. If you believe this issue is still relevant, please leave a comment or provide updated details. Thank you.

@nextjs-bot nextjs-bot added the stale The issue has not seen recent activity. label Jan 13, 2025
@nextjs-bot
Copy link
Collaborator

This issue has been automatically closed due to two years of inactivity. If you’re still experiencing a similar problem or have additional details to share, please open a new issue following our current issue template. Your updated report helps us investigate and address concerns more efficiently. Thank you for your understanding!

@nextjs-bot nextjs-bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 21, 2025
Copy link
Contributor

github-actions bot commented Feb 5, 2025

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 Feb 5, 2025
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 5, 2025
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 stale The issue has not seen recent activity.
Projects
None yet
Development

No branches or pull requests

5 participants