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

Discrepancy in searchParams between server and client after clicking Link #64170

Closed
kachkaev opened this issue Apr 7, 2024 · 10 comments · Fixed by #65977
Closed

Discrepancy in searchParams between server and client after clicking Link #64170

kachkaev opened this issue Apr 7, 2024 · 10 comments · Fixed by #65977
Labels
bug Issue was opened via the bug report template. locked

Comments

@kachkaev
Copy link
Contributor

kachkaev commented Apr 7, 2024

Link to the code that reproduces this issue

https://github.com/kachkaev/next-js-search-params-mismatch-mwe

To Reproduce

  1. Clone reproduction repo, run pnpm install and pnpm dev (or pnpm build; pnpm start).

  2. Navigate to http://localhost:3000/?a=1&b=3 in a new tab. Observe this page (no issues so far):

  3. Click on <Link /> components at the bottom of the page, observe changes in displayed values for a and b. All good still.

    Kapture.2024-04-07.at.13.20.33.mp4
  4. Navigate to a link that takes you to /, i.e. that unsets both a and b.

Current vs. Expected behavior

Observed state

searchParams in page.tsx is equal to the original value (the one we saw on initial page load).
useSearhParams() returns correct values for a and b.

Expected state

Both sources of searchParams should be in sync (which can be observed after full page reload):

Provide environment information

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 23.4.0: Fri Mar 15 00:10:42 PDT 2024; root:xnu-10063.101.17~1/RELEASE_ARM64_T6000
  Available memory (MB): 32768
  Available CPU cores: 10
Binaries:
  Node: 20.11.1
  npm: 10.5.0
  Yarn: 4.0.0
  pnpm: 8.10.5
Relevant Packages:
  next: 14.2.0-canary.61 // Latest available version is detected (14.2.0-canary.61).
  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

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

next dev (local), next start (local)

Additional context

If all initial page query params are unset, the bug does not manifest itself. So it’s important to start with something like http://localhost:3000/?a=1&b=3 and not just http://localhost:3000.


This bug has implications on pages like e-commerce catalogs which can be filtered. Imagine someone sends you a link to

https://example.com/products?category=shoes&color=red

You’ll see filter controls in the UI and start clicking:

https://example.com/products?category=shoes&color=green

https://example.com/products?category=tshirts&color=green

https://example.com/products?category=tshirts

https://example.com/products?category=tshirts&color=red

...

As long as there is at least one query param in the URL, all works well. But as soon as you navigate to no filters, the catalog will look broken. The URL will be https://example.com/products but the content will be for ?category=shoes&color=red (the initial state of the app router). The only way to see the whole catalog will be to refresh the page.

@kachkaev kachkaev added the bug Issue was opened via the bug report template. label Apr 7, 2024
@kachkaev kachkaev changed the title Discrepancy in searchParams between server and client after clicking <Link /> Discrepancy in searchParams between server and client after clicking Link Apr 7, 2024
@kachkaev
Copy link
Contributor Author

Looks like I have found a workaround. It might be good enough for most people before the issue is fixed in Next.js itself:

kachkaev/next-js-search-params-mismatch-mwe#1

Kapture.2024-04-12.at.23.10.33.mp4

@dmaksimov
Copy link

Experiencing this issue as well.

The search params are in sync up until you navigate to the page without any search params.

@samburgers
Copy link

Discovered this issue too, and made a clickable minimal reproduction here:
https://nextjs-queryparams.vercel.app/

Source:
https://github.com/samburgers/nextjs-queryparams/blob/main/app/page.tsx

@icyJoseph
Copy link
Contributor

Ahh, we've also hit this one after a major leap from 14.1.x to 14.2.x territory.

Same issue reported here: #65030

@balazsorban44 - sorry for pinging you, but do you think you could drop some thoughts on this issue?

@icyJoseph
Copy link
Contributor

Another update, it might just be a coincidence, but changing staleTime dynamic to zero, seems to do the trick...

Perhaps the introduction of staleTimes, causes this issue?

@icyJoseph
Copy link
Contributor

icyJoseph commented May 20, 2024

@ztanner Hi, sorry for pinging you on this one, in all honesty I haven't tested this issue in the canaries released this past week, but would you mind investigating what's going on here? Is it expected behavior, or has a bug indeed been introduced?

Next.js 14.3.0-canary.70 still reproduceable

I think the links here, #64170 (comment), are pretty much all you need to see the issue.

ztanner added a commit that referenced this issue May 20, 2024
…rch params (#65977)

cc @icyJoseph @ztanner

NOTE: The canary release
[`v14.1.1-canary.51`](https://github.com/vercel/next.js/releases/tag/v14.1.1-canary.51)
and below work as expected.

### Why?

Introduced from #61535, the initial prefetch cache is set based on the
`location.pathname`.
When a page is loaded WITH the search param, the cache key does not
contain information of the search param.

The issue is when on a dynamic page reading the `searchParams` value,
the value doesn't change if navigated as:

```
/?q=foo --> /
```

The prefetch cache hits, not re-rendering, and the `searchParams` value
is not passed properly.

### How?

For the prefetch cache, add the `location.search` as well.

Since `createPrefetchCacheKey` uses
[`createHrefFromUrl`](https://github.com/vercel/next.js/blob/canary/packages/next/src/client/components/router-reducer/create-href-from-url.ts)
which includes `location.search`, I'm expecting the change won't affect
current cache key behavior.

Fixes #64170
Fixes #65030

---------

Co-authored-by: Zack Tanner <1939140+ztanner@users.noreply.github.com>
@devjiwonchoi
Copy link
Member

@kachkaev @icyJoseph Just tested next@14.3.0-canary.73, and it seems to be fixed! Please note that next@canary currently requires the react@canary as well :)

@kachkaev
Copy link
Contributor Author

This is brilliant, many thanks @devjiwonchoi 👏

@icyJoseph
Copy link
Contributor

Fantastic! Also verified this on a couple of project now. Awaiting a new release now.

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
ztanner added a commit that referenced this issue Jul 9, 2024
…rch params (#65977)

cc @icyJoseph @ztanner

NOTE: The canary release
[`v14.1.1-canary.51`](https://github.com/vercel/next.js/releases/tag/v14.1.1-canary.51)
and below work as expected.

### Why?

Introduced from #61535, the initial prefetch cache is set based on the
`location.pathname`.
When a page is loaded WITH the search param, the cache key does not
contain information of the search param.

The issue is when on a dynamic page reading the `searchParams` value,
the value doesn't change if navigated as:

```
/?q=foo --> /
```

The prefetch cache hits, not re-rendering, and the `searchParams` value
is not passed properly.

### How?

For the prefetch cache, add the `location.search` as well.

Since `createPrefetchCacheKey` uses
[`createHrefFromUrl`](https://github.com/vercel/next.js/blob/canary/packages/next/src/client/components/router-reducer/create-href-from-url.ts)
which includes `location.search`, I'm expecting the change won't affect
current cache key behavior.

Fixes #64170
Fixes #65030

---------

Co-authored-by: Zack Tanner <1939140+ztanner@users.noreply.github.com>
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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants