next.js icon indicating copy to clipboard operation
next.js copied to clipboard

[NEXT-1141] Next router / link not working when redirecting with new search params [Regression]

Open mikebuilds opened this issue 2 years ago • 26 comments

Verify canary release

  • [X] 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.1.0: Sun Oct  9 20:19:12 PDT 2022; root:xnu-8792.41.9~3/RELEASE_ARM64_T6020
    Binaries:
      Node: 18.15.0
      npm: 9.5.0
      Yarn: N/A
      pnpm: 8.0.0
    Relevant packages:
      next: 13.4.0
      eslint-config-next: 13.3.0
      react: 18.2.0
      react-dom: 18.2.0

Which area(s) of Next.js are affected? (leave empty if unsure)

No response

Link to the code that reproduces this issue

https://codesandbox.io/p/sandbox/eloquent-https-5slx4d

To Reproduce

Navigate to the same URL with a new search param either via router.push() or the link component.

In the reproduction, wait for the initial page to load - then try and change to page 2, or 3, or use the search input.

Describe the Bug

The bug first appeared in 13.3.5-canary.2, prior versions works correctly.

Navigating to the same page with new search params no longer renders the loading.tsx. Looking at the URL / network tab - it seems that the router does kick off the fetch for the new page, but then waits for the full body response before updating the URL, or rendering the loading page.

Expected Behavior

Prior to 13.3.5-canary.2, the behaviour was working correctly:

Navigating to the same page with new search params should show the loading.tsx, if the page is slow to fetch.

Video of this working previously:

https://user-images.githubusercontent.com/3614846/236453767-429cec22-46bc-436f-8377-2a597243a866.mov

Which browser are you using? (if relevant)

N/A

How are you deploying your application? (if relevant)

N/A

NEXT-1141

mikebuilds avatar May 05 '23 12:05 mikebuilds

For reference, I think this may be the issue commit: https://github.com/vercel/next.js/commit/25ba8a74b7544dfb6b30d1b67c47b9cb5360cb4e @feedthejim

Unsure if there is background here, but I can't think of why this could potentially be desired behaviour for any component using search params.

mikebuilds avatar May 05 '23 12:05 mikebuilds

@mikebuilds can you perhaps share the code for the issue 2? It's hard to investigate because it could be a mistake in your code.

Looking into issue 1

feedthejim avatar May 05 '23 12:05 feedthejim

@feedthejim apologies, issue 2 is in fact a code issue - I changed some code when trying to debug issue 1, and accidentally introduced duplicate keys on the table rows.

Appreciate you looking into issue 1

mikebuilds avatar May 05 '23 12:05 mikebuilds

Hello there,

Issue 1 is happening to me + other issue where next fetch don't take into account the cache and revalidate options in the fetch call and its only running the fetch again after 30s approx when navigating back to the route:

{
      cache: 'no-store',
      next: {
        revalidate: 0,
      },
}

PS: Found my problem and its probably related: https://github.com/vercel/next.js/issues/49300

fprl avatar May 05 '23 13:05 fprl

Hey everyone, is there any update on this? loading.tsx is not rendering once searchParams has changed after using the example in the docs.

fprl avatar May 21 '23 20:05 fprl

What I've done to circumvent this issue is to use my own suspense boundary (with a key prop) instead of relying on the loading.tsx file's boundary.

In the Suspense's key prop I stringify the search params, that way it'll re-render (and fallback) whenever the search params change.

With this approach I was also able to exclude certain params from influencing the loading state, like the currently opened dialog.

example: https://github.com/wise-old-man/wise-old-man/blob/53bbe450f845d220895a46eee3f9da903f34674f/app-v2/src/app/names/page.tsx#L55

psikoi avatar May 30 '23 14:05 psikoi

What I've done to circumvent this issue is to use my own suspense boundary (with a key prop) instead of relying on the loading.tsx file's boundary.

In the Suspense's key prop I stringify the search params, that way it'll re-render (and fallback) whenever the search params change.

With this approach I was also able to exclude certain params from influencing the loading state, like the currently opened dialog.

example: https://github.com/wise-old-man/wise-old-man/blob/53bbe450f845d220895a46eee3f9da903f34674f/app-v2/src/app/names/page.tsx#L55

I tried this, and it's only working for me in dev but not in prod:

import { Suspense } from 'react';

import { getHouses } from '@/utils/houses';

import Listings from '../components/listings';

type Props = {
  searchParams: {
    [key: string]: string;
  };
};

export default async function SearchPage({ searchParams }: Props) {
  const key = JSON.stringify(searchParams);
  const listingsPromise = getHouses(searchParams);

  return (
    <Suspense key={key} fallback={<Listings.Skeleton />}>
      {/* @ts-expect-error Async Server Component */}
      <Listings listingsPromise={listingsPromise} />
    </Suspense>
  );
}

PS: Keep in mind I'm sending the promise and not the result so that shouldn't be a problem.

fprl avatar May 30 '23 18:05 fprl

@francoromanol have you tried to deploy it to somewhere other than vercel? I'm not sure but might be related to https://github.com/vercel/next.js/issues/51033.

there's also a similar problem mentioned in https://github.com/vercel/next.js/issues/42346 😭

chillardinho avatar Jun 19 '23 09:06 chillardinho

Guys, any news on that?

tantsur avatar Oct 06 '23 02:10 tantsur

I had this issue today and had a difficult time finding a solution until i found this thread, using a key on the Suspense Element fixes it.

Let's get this issue resolved 🙏

alepacheco avatar Oct 06 '23 13:10 alepacheco

i added the JSON.stringify(searchParams) as a key on the suspense and it worked not in production tho , it only works in dev env

MelancholYA avatar Oct 06 '23 22:10 MelancholYA

Any updates on this?

Even with a key on Suspense I'm not seeing the loading template when updating query params and pushing into the router.

jaredgalanis avatar Oct 28 '23 11:10 jaredgalanis

Any updates on this?

Even with a key on Suspense I'm not seeing the loading template when updating query params and pushing into the router.

I've actually realized that while this may not be expected behavior, I don't want to reload the entire fallback on query param changes. I'd rather use useTransition to provide a more subtle loading indicator per the React docs https://react.dev/reference/react/Suspense#preventing-unwanted-fallbacks.

jaredgalanis avatar Oct 28 '23 13:10 jaredgalanis

Any updates on this? Even with a key on Suspense I'm not seeing the loading template when updating query params and pushing into the router.

I've actually realized that while this may not be expected behavior, I don't want to reload the entire fallback on query param changes. I'd rather use useTransition to provide a more subtle loading indicator per the React docs https://react.dev/reference/react/Suspense#preventing-unwanted-fallbacks.

Yes, this is something I have come to realize too, I now see the loading skeleton as the state for "no data yet", but if I'm updating the UI with new data (maybe due to some search or filters being changed in the URL), I prefer the user to still be able to see the previous results, with some more inline and subtle indication that the data is changing.

Here's an example in my app:

https://wiseoldman.net/competitions

You'll see that as you first visit the page, there's a loading skeleton, but if you type in any search or select a filter, there's a subtle inline loading indicator that uses useTransition.

I found myself doing weird things like adding a key prop to the Suspense boundary, it was usually an indication that I was fighting the mental model of the app router because I thought that all loading states are equal. I do not believe they are, anymore.

psikoi avatar Oct 30 '23 12:10 psikoi

I'm doing a filter based on searchParams and I need to trigger the reload when the params change. Looking in the comments here it seems that in previous versions of Next it worked, I'm using version 14.0.3 and changing the params do not trigger the loading.tsx page.

tonimercadante avatar Dec 01 '23 11:12 tonimercadante

I'm doing a filter based on searchParams and I need to trigger the reload when the params change. Looking in the comments here it seems that in previous versions of Next it worked, I'm using version 14.0.3 and changing the params do not trigger the loading.tsx page.

Same problem here

peiris avatar Dec 12 '23 21:12 peiris

I'm doing a filter based on searchParams and I need to trigger the reload when the params change. Looking in the comments here it seems that in previous versions of Next it worked, I'm using version 14.0.3 and changing the params do not trigger the loading.tsx page.

Same problem here :((

nghyane avatar Dec 20 '23 13:12 nghyane

I experimented a little and found that using a regular anchor tag "a" instead of Link trigger loading.tsx every time.

I guess all assets are reloaded when using tag "a"

judgemavi avatar Dec 29 '23 00:12 judgemavi

+1

Edit by maintainers: Comment was automatically minimized because it was considered unhelpful. (If you think this was by mistake, let us know). Please only comment if it adds context to the issue. If you want to express that you have the same problem, use the upvote 👍 on the issue description or subscribe to the issue for updates. Thanks!

SkyGopnik avatar Jan 11 '24 20:01 SkyGopnik

@psikoi is right that this is not a bug, but that the right solution is to use useTransition to show a more subtle localized loading state while query params are changing.

MrLoh avatar Jan 31 '24 13:01 MrLoh

Hm, I even tried to generate page URLs like /report/page/X and this doesn't help. So seems like issue is not only with search params

abaksha-sc avatar Feb 20 '24 15:02 abaksha-sc

{ searchParams }: Props

Did you ever get this resolved? I'm also on Next 14 with the same issues

keennay avatar Mar 14 '24 19:03 keennay

https://www.youtube.com/watch?v=UNEXbGJCTw8

In this video he demonstrates several techniques, the best for me is the one in minute 17

LuizFernando991 avatar Apr 12 '24 12:04 LuizFernando991

Just in case this seems useful to anyone. My usecase included doing router.push calls to add navigate to the same page but with new search params. I wanted to trigger a loading indicator while the server replaced the old data with the new data. I tried managing my own suspense boundary with the key prop like @psikoi suggested, but I needed more granular control.

After reading @MrLoh 's comment, I tried to follow his suggestion. My solution ended being something like this

Instead of doing:

router.push(urlWithNewParams)

I did the following:

import { useTransition } from "react"

// Inside the component

const [isPending, startTransition] = useTransition()

const navigate = useCallback((newPath: string) => {
    startTransition(() => {
        router.push(newPath)
    })
})

Then, wherever I was calling router.push, I replaced with navigate. Finally, I was able to use the isPending value to display a loading spinner in the JSX

return (
    <>
        {isPending && <Loader2 className="animate-spin" />
    </>
)

This allowed me to show some indication to the users that there is more data coming in, without removing the old data from the page. Hope this helps someone!

adpadillar avatar Aug 03 '24 04:08 adpadillar

I've been toiling with this issue for the past days trying to find a solution that works. First about the video that @LuizFernando991 linked; I think that only works when you're manually using suspense boundaries within a page component. It doesn't seem to work with the suspense boundary between a layout and a page component. I was able to get it to work a little bit, but this was by dynamically rendering the entire layout, and this was also unreliable.

I'm pretty sure if my understanding of Next.Js is correct, the reason the old page doesn't get removed right away is because of useTransition. According to the react docs: "Suspense-enabled routers are expected to wrap the navigation updates into Transitions by default."

from: https://react.dev/reference/react/Suspense#resetting-suspense-boundaries-on-navigation

That part of that page is very relevant for the issue we are facing, and indeed the intended solution is to use the searchParams as a key for the component to tell React that although the path is the same, the content is different, and so it should fall back to the suspense boundary. The fact that this doesn't seem to work in Next is pretty disappointing.

haraldev01 avatar Aug 16 '24 15:08 haraldev01

Facing the same issue. At present, fixing it as described in https://github.com/vercel/next.js/issues/49297#issuecomment-1568557317. Hoping to see a fix soon.

manacy-keyvalue avatar Aug 28 '24 09:08 manacy-keyvalue

+1

Edit by maintainer bot: Comment was automatically minimized because it was considered unhelpful. (If you think this was by mistake, let us know). Please only comment if it adds context to the issue. If you want to express that you have the same problem, use the upvote 👍 on the issue description or subscribe to the issue for updates. Thanks!

romanticsoul avatar Apr 30 '25 11:04 romanticsoul