vercel / next.js

The React Framework
https://nextjs.org
MIT License
126.99k stars 26.99k forks source link

Updating search params does not trigger suspense fallback or loading.tsx #53543

Open dclark27 opened 1 year ago

dclark27 commented 1 year ago

Verify canary release

Provide environment information

Operating System:
      Platform: linux
      Arch: x64
      Version: #22 SMP Tue Jan 10 18:39:00 UTC 2023
    Binaries:
      Node: 18.14.2
      npm: 9.5.0
      Yarn: 1.22.19
      pnpm: N/A
    Relevant Packages:
      next: 13.4.12
      eslint-config-next: 13.4.12
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 5.0.4
    Next.js Config:
      output: N/A

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

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

Link to the code that reproduces this issue or a replay of the bug

https://codesandbox.io/p/sandbox/muddy-glade-hn895s

To Reproduce

  1. Create a component which uses search parameters in fetching data
  2. Use a link to change the query param and load new data based on the search param
  3. Observe that loading.tsx fallback does not invoke and the page hangs while new data is loaded

Describe the Bug

Updating search params does not trigger suspense fallback or loading.tsx

Expected Behavior

Fetching new data based on new search params trigger suspense fallbacks or loading.tsx

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

Fredkiss3 commented 1 year ago

Hello, i don't know if this is an expected behavior or not, but i've stumbled on this behvior a little while ago and traced it back to React ( link : https://react.dev/reference/react/Suspense#resetting-suspense-boundaries-on-navigation ).

The thing is that for Suspense fallbacks (and loading.tsx which is just another Suspense) to show, it needs to have a different key setup ( <Suspense key={YOUR_KEY} />), it seems like next do not include the searchParams in the key of a route (for loading.tsx), so navigating to the same route do not retrigger the suspense fallback.

My solution was just to include the searchParams into the key of the Suspense fallback and it worked. Something like this :

export default function Page(props: {
  searchParams?: Record<string, string | undefined>;
}) {
  const keyString = `search=${props.searchParams?.search}`; //  <-- Construct key from searchParams 
  return (
    <section>
      <SearchInput />

      {props.searchParams?.search && (
        <React.Suspense
          // pass key to Suspense
          key={keyString} 
          fallback={
            <ul>
              <ProductCardSkeleton />
              <ProductCardSkeleton />
              <ProductCardSkeleton />
              <ProductCardSkeleton />
            </ul>
          }
        >
           // <ProductList> is an async React component that do the fetching 
          <ProductList name={props.searchParams?.search} />
        </React.Suspense>
      )}
    </section>
  );

This won't work for loading.tsx file though as that file do not take searchParams as props.

dclark27 commented 1 year ago

Thanks for the response @Fredkiss3 ! I'm sure it will help someone.

Unfortunately for my scenario that won't trigger the suspense boundary because I am loading new data via the a react server component. I would think the loading.tsx file would hit the fallback since that is the suspense boundary of my page.tsx.

Fredkiss3 commented 1 year ago

@dclark27

The key thing seems to be how it is supposed to work : https://twitter.com/sebmarkbage/status/1688622875702345728?s=46

Normally if you want to trigger suspense boundaries, you have to fetch your data in a subcomponent not the page directly, this is fairly easy as you can pass props from the page to another component.

dclark27 commented 1 year ago

@Fredkiss3

Thanks for the tweet thread, I hopped in the replies. Since NextJS is handling the boundaries, there should be a way to re-trigger the suspense boundary, whether that's giving loading.tsx knowledge of the params, or having search params be included in the keys.

I'm not sure what @sebmarkbage means that search params are generally used differently.

If I'm not using them correctly in my bug, I can refactor to use params for my pagination. But having something like app/page/[pageNumber]/page.tsx feels overkill for paging through a table (especially when the result set can vary in length).

edit:

I've updated the sandbox to include a refactored version of pagination using [pageNumber].

Fredkiss3 commented 1 year ago

One thing @dclark27 CodeSandbox does not support streaming, so suspense won't work as expected.

razvanbretoiu commented 1 year ago

I've encountered the same issue. If the API has a cold start when performing a search, I would like to show the "loading" state from loading.tsc file till the current request is finished.

MelancholYA commented 1 year ago

@Fredkiss3 did you test it in production too ? because it worked for me in dev mode but in production it didn't

Jordaneisenburger commented 1 year ago

I've got the same issue Suspense isn't being triggered for searchParams at all.

@dclark27 did you find a solution for this problem ?

anthonyiu commented 11 months ago

Got the same problem. I tried @Fredkiss3 's solution by passing a key to but it didn't work, not even in dev stage.

SarahLightBourne commented 11 months ago

Having the same problem. In a statically generated component, I have a Suspense boundary with a param-based key over a RSC that renders data based on query parameters from page.tsx. It also has noStore() to make the RSC dynamic. Fallback does render when query parameters change in development mode but not in production. Don't really know what to do from this point.

anthonyiu commented 11 months ago

Hi all, I tried a workaround and it works for me. Please try to convert your page.tsx to a client component using "use client" , get searchParams by useSearchParams() instead of the page props and wrap the server component that fetch data in <Suspense> as follows.

Hope it helps. Thanks.

page.tsx

"use client";

import { Suspense } from "react";
import BlogSkeleton from "../../../components/ui/BlogSkeleton";
import AllBlogPosts from "./AllBlogPosts";
import { useSearchParams } from "next/navigation";
import BlogLayout from "../../../components/section/BlogLayout";

const Blog = () => {

  const searchParams = useSearchParams();
  const searchParamsValue = searchParams.has("type")
    ? searchParams.get("type")
    : "all";

  const keyString = searchParamsValue ? searchParamsValue : "all";

  return (
    <BlogLayout>
      <Suspense key={`type=${keyString}`} fallback={<BlogSkeleton />}>
        <AllBlogPosts searchParamsValue={keyString} />
      </Suspense>
    </BlogLayout>
  );
};
export default Blog;
arklanq commented 11 months ago

Hi all, I tried a workaround and it works for me. Please try to convert your page.tsx to a client component using "use client" , get searchParams by useSearchParams() instead of the page props and wrap the server component that fetch data in <Suspense> as follows.

Hope it helps. Thanks.

page.tsx

"use client";

import { Suspense } from "react";
import BlogSkeleton from "../../../components/ui/BlogSkeleton";
import AllBlogPosts from "./AllBlogPosts";
import { useSearchParams } from "next/navigation";
import BlogLayout from "../../../components/section/BlogLayout";

const Blog = () => {

  const searchParams = useSearchParams();
  const searchParamsValue = searchParams.has("type")
    ? searchParams.get("type")
    : "all";

  const keyString = searchParamsValue ? searchParamsValue : "all";

  return (
    <BlogLayout>
      <Suspense key={`type=${keyString}`} fallback={<BlogSkeleton />}>
        <AllBlogPosts searchParamsValue={keyString} />
      </Suspense>
    </BlogLayout>
  );
};
export default Blog;

Unfortunately, this solution wastes all the benefits of using server components.

anthonyiu commented 11 months ago

Hi all, I tried a workaround and it works for me. Please try to convert your page.tsx to a client component using "use client" , get searchParams by useSearchParams() instead of the page props and wrap the server component that fetch data in <Suspense> as follows. Hope it helps. Thanks. page.tsx

"use client";

import { Suspense } from "react";
import BlogSkeleton from "../../../components/ui/BlogSkeleton";
import AllBlogPosts from "./AllBlogPosts";
import { useSearchParams } from "next/navigation";
import BlogLayout from "../../../components/section/BlogLayout";

const Blog = () => {

  const searchParams = useSearchParams();
  const searchParamsValue = searchParams.has("type")
    ? searchParams.get("type")
    : "all";

  const keyString = searchParamsValue ? searchParamsValue : "all";

  return (
    <BlogLayout>
      <Suspense key={`type=${keyString}`} fallback={<BlogSkeleton />}>
        <AllBlogPosts searchParamsValue={keyString} />
      </Suspense>
    </BlogLayout>
  );
};
export default Blog;

Unfortunately, this solution wastes all the benefits of using server components.

I somewhat agree with you. However, given that my app is kind of light-weighted and it's acceptable for me to shift the rendering to client side.

SarahLightBourne commented 10 months ago

@anthonyiu if you specify the whole page as client component, then how can you fetch data in AllBlogPosts as it's no longer a server component? Or do you mean you fetch data manually using API?

abhishek2393 commented 9 months ago

hey did anybody found the solution?

siduck commented 9 months ago

hey did anybody found the solution?

use key in suspense component and the key should be unique, for my case, the key was api's url with the searchparams in string format

christopher-caldwell commented 9 months ago

Adding the key in Suspense worked for me, but then turned the entire route dynamic. I was hoping to use partial prerendering, but that only works if I remove they key from Suspense, which then causes the fallback to not trigger lol.

const Page = ({ searchParams }) => {
  return (
    <>
      <Suspense key={JSON.stringify(searchParams)}>
        <DynamicContent searchParams={searchParams} />
      </Suspense>
      <StaticContent />
    </>
  )
}

This works visually as expected, but causes the whole route to be dynamic

Edit: This only worked locally, and did not work when deployed.

ubirajaramneto commented 9 months ago

For people that are still not able to get this to work correctly, here is what worked for me.

Some context before we move on. The react documentation states the following:

Suspense lets you display a fallback until its children have finished loading.

So with that in mind, make sure that the following is true:

1 - The actual data loading is happening in the child, and not the same component where the Suspense component lives. 2 - Make sure that you assign the key to the Suspense component, and not the child component itself.

NOTES:

Although I am unsure of the consequences of adding a key to a Suspense component, this seemed to work well and solved the issue I was having. Maybe this is an anti pattern for Suspense specifically? Time will tell.

Both of the components shown below are server components. I am unsure if applying the same strategy would work if the child component was a client component, but from the react documentation it states the following:

Only Suspense-enabled data sources will activate the Suspense component. They include:

Data fetching with Suspense-enabled frameworks like Relay and Next.js Lazy-loading component code with lazy Reading the value of a Promise with use

So it should work for client components, if the conditions above are met.

Here is an example:

// page.tsx
// note that this is a server component

import { Suspense } from "react";
import { ListingGrid } from "@/app/_listing/ListingGrid";
import ListingGridFilter from "@/app/_listing/ListingGridFilter";
import { ListingSearchParams } from "@/app/lib/listing/data";
import ListingGridSkeleton from "@/app/_listing/ListingGridSkeleton";

export default async function Home({
  searchParams,
}: {
  searchParams: ListingSearchParams;
}) {
  const beds = searchParams?.bedrooms ?? "0";
  const baths = searchParams?.bathrooms ?? "0";
  const park = searchParams?.parking ?? "0";
  const price = searchParams?.price ?? "0";

  return (
    <main className="flex -h-screen flex-col items-center justify-between p-24">
      <ListingGridFilter />
      <Suspense
        key={beds + baths + park + price}
        fallback={<ListingGridSkeleton />}
      >
        <ListingGrid searchParams={searchParams} /> // <--- Data loading is happening here
      </Suspense>
    </main>
  );
}
// ListingGrid.tsx
// note that this is also a server component

import {
  ListingData,
  ListingSearchParams,
  ListingType,
} from "@/app/lib/listing/data";
import { ListingCard } from "./ListingCard";
import { unstable_noStore } from "next/cache";

export async function ListingGrid({
  searchParams,
}: {
  searchParams: ListingSearchParams;
}) {
   // This is just to force requests to be fetched every time
   // so you can see the components loading.
  unstable_noStore();
  const listing: Array<ListingType> = await ListingData(searchParams);

  if (!listing.length) {
    return <p className="text-xl">No listing matches your criteria</p>;
  }

  return (
    <div className="w-3/4 grid grid-cols-1 2xl:grid-cols-4 xl:grid-cols-3 lg:grid-cols-2 gap-y-12 gap-x-4">
      {listing.map((listingItem) => (
        <ListingCard key={listingItem.Id} listing={listingItem} />
      ))}
    </div>
  );
}
christopher-caldwell commented 9 months ago

I made an in depth reproduction of my use case here.

Repo: https://github.com/christopher-caldwell/ppr-loading-suspense-demo Live URL: https://ppr-loading-suspense-demo.vercel.app

Adding a key to suspense works locally, but NOT hosted on Vercel.

I haven't tried unstable_noStore, so I will give that a go. The docs say:

Partial Prerendering does not yet apply to client-side navigations. We are actively working on this.

So I cannot really tell if this behavior is intentional or not.

christopher-caldwell commented 9 months ago

@ubirajaramneto Adding unstable_noStore did not change the result for me. It works local, but not hosted. Maybe my particular issue is with Vercel, and not Next itself.

With Suspense key: https://ppr-loading-suspense-demo.vercel.app/?page=1 Without Suspense key: https://ppr-loading-suspense-demo.vercel.app/without-suspense-key?page=1

I mostly want to know if this behavior is considered intentional for where the dev team is in the development process of PPR. I understand that features will trickle in, but is this supposed to be happening? At least for now.

Edit: yarn build and then yarn start on my local machine, and this works as expected. I think my issue might be with Vercel in their hosting.

ubirajaramneto commented 9 months ago

@christopher-caldwell I have uploaded the whole project in my github for you to see, also just to make sure, I have deployed the application to vercel and it is working the same way as it is working on localhost.

Demo: https://nextjs-listings-app.vercel.app/ Repo: https://github.com/ubirajaramneto/nextjs-listings-app

After reviewing your code, many things popped in my head as what the culprit would be, but I am not confident enough to take a bet.

But if I was forced to guess, I would try the following:

1 - remove the wait promise that you wrapped your api call around 2 - make sure that inside your suspense component, there is only one component (maybe react is not seeing the promise inside Suspense)

Try to read through my example and try applying some of the patterns you see there and see if anything works out.

christopher-caldwell commented 9 months ago

@ubirajaramneto Thanks for your suggestions. I appreciate you taking a look.

I have implemented them, but didn't seem to change anything: https://ppr-loading-suspense-demo-git-ubira-e4da36-christopher-caldwell.vercel.app/ and https://github.com/christopher-caldwell/ppr-loading-suspense-demo/blob/ubirajara-suggestions/app/page.tsx

Something to note, your project does not have the ppr experimental flag. I understand that I might be up a creek due to it being experimental, but wanted to point that out.

For the wait, it is just to simulate the network taking longer than expected. It doesn't wrap the call, wait is a function that is awaited inside the API call function. I removed it anyway just to be sure.

I put in a support ticket with Vercel, as this works when I run it on my computer with yarn start. We shall see what happens.

ubirajaramneto commented 9 months ago

Sorry @christopher-caldwell , my solution was not necessarily aiming at your problem, since I am not doing any pre-rendering in the example I provided. This example I posted was to point the general public on how to handle the suspense boundary not triggering, which was the problem I first faced and wanted to share my solution here so it can serve as a reference.

And regarding your await call, yes you are right, I might have expressed myself poorly on that instance.

If you like you can contact me directly and I'll be glad to throw some insight into your problem, just so we can keep this issue clean of parallel discussions.

Hope you can find a solution for this problem soon!

christopher-caldwell commented 9 months ago

Sorry @christopher-caldwell , my solution was not necessarily aiming at your problem, since I am not doing any pre-rendering in the example I provided. This example I posted was to point the general public on how to handle the suspense boundary not triggering, which was the problem I first faced and wanted to share my solution here so it can serve as a reference.

And regarding your await call, yes you are right, I might have expressed myself poorly on that instance.

If you like you can contact me directly and I'll be glad to throw some insight into your problem, just so we can keep this issue clean of parallel discussions.

Hope you can find a solution for this problem soon!

Interestingly, your solution worked perfectly for me locally. So I reached to Vercel, because they say that if your code is working locally, and on their platform, you should submit a support ticket.

The only blurb about PPR is that it doesn't support client side navigation, which is a bit ambiguous to me. So I'm not sure if my issue is because of PPR (which is fine, it's experimental) or Vercel is not supporting it or whatever the case is.

christopher-caldwell commented 9 months ago

If anyone else runs into this issue (specifically with PPR), using regular html anchor tags worked for me. You don't get all the benefits of the Next <Link/>, but it shows the loading fallback until the team can implement client side navigation.

I also heard back from Vercel. Even if it works locally, they say to take it up with Next since it's an experimental feature. It's not their problem, according to them (they were nice, I don't mean to imply they weren't).

vishaltyagi227 commented 7 months ago

face the same issue, working fine on local build npm run build & npm run start but I am building the standalone build by setting the output = standalone in the next config file. and after deploying to the Google Cloud. an application does not work as expected and a route that uses the Suspense gives 500

if any one solve the issue, please share it can be helpful

JClackett commented 5 months ago

Any updates from the next.js team from this? I would have assumed adding a loading.tsx file would mean that if the page is fetching more data on navigation change (search params changes etc) that the loading.tsx would be re-rendered?

AChangXD commented 5 months ago

This really needs to be addressed, can't use server-side data fetching for a data-intensive chart now :(

JClackett commented 5 months ago

@AChangXD you can just put the data fetching into an async component that you then wrap in Suspense in your page.tsx file. You can then add the fallback to the suspense instead of using loading.tsx and it works. NOTE: if you're using search params to navigate you do have to put a key on the Suspense that changes, so that it actually triggers the fallback though for example:

  key={`${searchParams.query}${searchParams.otherParam}`}
AChangXD commented 5 months ago

@AChangXD you can just put the data fetching into an async component that you then wrap in Suspense in your page.tsx file. You can then add the fallback to the suspense instead of using loading.tsx and it works. NOTE: if you're using search params to navigate you do have to put a key on the Suspense that changes, so that it actually triggers the fallback though for example:


  key={`${searchParams.query}${searchParams.otherParam}`}

Yeah I didn't realize the key needs to be dynamic, working now😃

remitone commented 5 months ago

@AChangXD you can just put the data fetching into an async component that you then wrap in Suspense in your page.tsx file. You can then add the fallback to the suspense instead of using loading.tsx and it works. NOTE: if you're using search params to navigate you do have to put a key on the Suspense that changes, so that it actually triggers the fallback though for example:

  key={`${searchParams.query}${searchParams.otherParam}`}

I have similar problem. Do you have an example how to do this? When you say you can just put the data fetching into an async component, Currently my data fetched in the Server component, moving to a seperate component means what the data should I return?

This is how my current component looks

export default async function Transactions({
  searchParams,
}: {
  searchParams?: {
    fromDate?: string;
    toDate?: string;
    transferType?: string;
    status?: string;
    beneficiary?: string;
    order?: string;
  };
}) {
  const t = await getTranslations();
  const fromDate = searchParams?.fromDate || null;
  const toDate = searchParams?.toDate || null;
  const transferType = searchParams?.transferType || null;
  const status = searchParams?.status || '';
  const beneficiary = searchParams?.beneficiary || null;
  const order = searchParams?.order || 'DESC';

  let filters: TransactionListFilters = {
    order: order as 'ASC' | 'DESC',
  };
  if (fromDate) {
    filters['from_date'] = fromDate;
  }
  if (toDate) {
    filters['to_date'] = toDate;
  }
  if (transferType) {
    filters['trans_type'] = transferType as TransType;
  }
  if (status) {
    filters['status'] = status as Status;
  }
  if (beneficiary) {
    filters['beneficiary_id'] = beneficiary;
  }

  // Data fetching 
  const transactions = await transactionService.getTransactionList(filters);

  // data fetching
  const beneficiaries = await beneficiaryService.getBeneficiaryList();

  return (
    <Box>
      <Container>
        <TransactionList
          transactions={transactions?.transactions}
          appliedFilters={Object.keys(searchParams!).length ?? 0}
          beneficiaries={beneficiaries?.beneficiaries}
        />
      </Container>
    </Box>
  );
}
JClackett commented 5 months ago

@remitone that should be fine, just wrap that component in suspense where ever youre rendering it, and add they key prop

luwes commented 4 months ago

for me in Next v14.2.3 the page render was not even triggering going from having a ?search=.. to without. this was a workaround that worked:

  const handleSearch = (term: string) => {
    const params = new URLSearchParams(searchParams);
    if (term) {
      params.set('search', term);
    } else {
      // This is a workaround for a bug where the page without a search query
      // would not trigger a RSC update.
      // params.delete('search');
      params.set('search', term);
    }
    replace(`${pathname}?${params.toString()}`, { scroll: false });
  };
1Ghasthunter1 commented 4 months ago

Are there any fixes in site? Many threads about this that have been open for 1+ years.

ThomasBurgess2000 commented 4 months ago

Also hoping for a real solution

hidecchi commented 2 months ago

I use startTransition like that.

 const [isPending, startTransition] = useTransition();
    ....
   startTransition(() => {
      router.replace(`${pathname}?${params.toString()}`);
    });

If isPending is true ,I show Loading component. Although this is client side management, this woks.

wottpal commented 2 months ago

Adding a key to suspense works locally, but NOT hosted on Vercel.

@christopher-caldwell Unfortunately, this is still the case with Next.js 15 Canary and experimental PPR. – Did you ever found a workaround/solution?

cc @leerob

leerob commented 2 months ago

If the behavior you are looking for is this:

  1. You have a form which updates search params
  2. You want to prefetch and client-side transition to the loading state
  3. The Suspense boundary should be keyed on the search params

Then we now have an example working on canary with next/form (new):

wottpal commented 2 months ago

Hey @leerob,

first: Wow, what a fast & helpful reply. You're an absolute OG. 🫶

Using <Form action=…> also for data querying via search params wasn't something that came to my mind. It's still kind of an exotic paradigm for me, but nonetheless, I gave it a try.

In my case it's a simple pagination I want to get to work, I rebuilt the navigation buttons like that (I gave them each their separate <Form> element with a hidden input field – But I'm happy to know if a solution with only one form comes to your mind):

// Get the `page` param (type-safely via nuqs)
const [page] = useQueryState('page', parseAsInteger.withDefault(1))

return (
  <PaginationFormAction page={page} newPage={Math.max(page - 1, 1)}>
    <ChevronLeft className="h-4 w-4" />
  </PaginationFormAction>

  <PaginationFormAction page={page} newPage={Math.min(page + 1, totalPages)}>
    <ChevronRight className="h-4 w-4" />
  </PaginationFormAction>
)
interface PaginationFormActionProps extends HTMLAttributes<HTMLFormElement> {
  page: number
  newPage: number
}
const PaginationFormAction = ({ children, page, newPage }: PaginationFormActionProps) => {
  return (
    <Form action="" replace={true} scroll={false}>
      <input type="hidden" name="page" value={newPage} />
      <Button variant="outline" className="h-8 w-8 p-0" disabled={page === newPage}>
        {children}
      </Button>
    </Form>
  )
}

The asynchronous server components that perform the data fetching are wrapped in a Suspense key-ed with the search params (as suggested above):

<Suspense
  key={JSON.stringify(props.searchParams)} // Note: Using `props.searchParams` here as I pass down `searchParams` into a nested server component and make use of PPR
  fallback={<Skeleton />}
>
  <Items />
  <Pagination />
</Suspense>

And voilà: It work's as it should. 🎉 But only locally (both via next dev & next build), not on Vercel. 🚨

The live version deployed to Vercel (same node version) just doesn't show the Suspense fallback at all. The site is frozen/blocked until the fresh data is awaited.

It basically yields to the exact same problem I, and some others above like @christopher-caldwell, have had before just with a different approach (using Form) :/

Have you tried deploying the commerce example? Is Vercel not yet ready for PPR (even though it's in experimentation-state for 1 year)?

leerob commented 2 months ago

The above code is deployed here https://demo.vercel.store and working on Vercel.

https://github.com/user-attachments/assets/9add7894-ebce-4fb0-8479-8ed65769fae2

wottpal commented 2 months ago

The above code is deployed here https://demo.vercel.store and working on Vercel.

Hmm, unfortunately I can't try it myself as I don't have a Shopify Storefront and the whole setup seems a bit too cumbersome for just debugging this. Nonetheless, I'll try to find differences in the implementations (mine shared above still looks right to me).

What I can see right away is that the example, even though it runs on next@15.0.0-canary.113, it doesn't has PPR enabled (neither ppr: 'incremental' in config nor export const experimental_ppr = true on the page). – Is Suspense & PPR on route changes supposed to be working on Vercel as of now?

leerob commented 2 months ago

Yes, Suspense and PPR (although experimental) are working on Vercel 👍

nachogutierrez commented 2 months ago

I'm confused by the behavior Suspense has on Server components and don't know how much of it is working as intended. Some people here suggest that there are workarounds, but I haven't found any for my specific problem. I created https://github.com/nachogutierrez/nextjs-suspense-bug-example to explain what I mean. I use ValueSetter to represent client components that update search params in the URL, and ContentLoader to represent an async server component that fetches data and then renders something (note: to simulate latency, ContentLoader always takes 2 seconds to render). Lastly there's only one page which renders two value setters for search params 'A' and 'B', and 1 or 2 content loaders depending on which case it is (see below).

I created branches case-N for N={1,2,3,4,5} to show a few things I found intersting. Each case is defined by how many content loaders (and thus how many Suspense boundaries) there are and what keys are used in each. There are only two possible keys: keyA which is whatever value was set in the 'A' search param and similar for keyB. In some cases there are no keys. Here are my findings:

branch Suspense boundaries Keys notes
case-1 1 no key No updates to either keyA or keyB result in the fallback being triggered and search params in the URL are updated after 2 seconds. I guess this is expected because there's no key set.
case-2 1 keyA Updates to keyA result in the loader's fallback being triggered, and the search parameter in the URL is updated immediately. Updates to keyB don't result in fallback being triggered and the search parameter in the URL is updated after 2 seconds. This behavior was expected because keyB is not used in the Suspense boundary.
case-3 2 keyA / keyB No updates to either keyA or keyB result in either fallback being triggered. This is the case I'm most interested in because it reflects what I do in my app (different subset of search params are relevant for different Suspense boundaries). I expected the first loader to trigger its fallback upon changes to keyA and the second loader to trigger its fallback upon changes to keyB. In both cases I expected the updated search params to appear in the URL instantly, but it takes 2 seconds to do so.
case-4 2 keyA / keyA This is REALLY strange behavior, each subsequent update of keyA results in cloned elements in the DOM. The list of content loaders gets bigger and bigger, and the fallback is triggered for the last two loaders. This makes absolutely no sense, so I assume this is a bug.
case-5 2 keyA / keyA The difference here is that the Suspense components are not siblings in the DOM, they have different parent containers. For some reason this fixes the really strange behavior found in case-4. In this case updates to keyA trigger the fallback for the first content loader but not for the second, I was expecting both to trigger their fallback since they have the same key. Updates to keyB don't trigger any fallback and it takes 2 seconds to see the updated search params in the URL.

@leerob I gave the new Form component in canary a try but I don't think it solves the problem I have with case-3. I created branch case-3.2 with a different implementation of ValueSetter that uses a Form. Unfortunately the navigation via Form triggers the fallback for all content loaders, not just the one for which the key was updated. Moreover, form submission seems to erase any search param from the URL that isn't controlled by that Form. This is a problem for me because in my real app I can't put all client components that update the search params under the same Form, they could be in different parts of the DOM. Is there any workaround for case-3?

raky291 commented 2 months ago

If the behavior you are looking for is this:

  1. You have a form which updates search params
  2. You want to prefetch and client-side transition to the loading state
  3. The Suspense boundary should be keyed on the search params

Then we now have an example working on canary with next/form (new):

I tested the canary version with the new Form component, but I still have the same results that using useRouter() API, I'm testing it with 3G connection and it takes 2s for the server to respond and during that time no loading status is shown (using the suspense component).

In my case I think the issue is because during those 2s the routes are not changing and the component ChildrenWrapper can't force the re-rendering, I'm not sure why it takes so much time. I know it's because of the 3G connection but 2 seconds is a lot, right?

lubieowoce commented 1 month ago

Moreover, form submission seems to erase any search param from the URL that isn't controlled by that Form

@nachogutierrez yeah this restriction is unfortunate, but it's how native <form> works, and we want <Form> to progressively enhance, so we have to mirror that. this avoids strange bugs where submitting a form before/after hydration would result in different search params

also note that <Form action="/somewhere?foo=bar"> is not allowed for the same reason -- native <form> will ignore any search params from action.

the only solution for preserving search params (that i'm aware of) is to render an <input type="hidden"> for each param you'd like to keep around. it's a bit clunky, but it'll also work before hydration, which is nice

This is a problem for me because in my real app I can't put all client components that update the search params under the same Form, they could be in different parts of the DOM

it's not always convenient, but <input form="myForm"> might help in some of these cases -- it lets you connect inputs to a form located elsewhere in the DOM https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input#form

csmartinsfct commented 1 month ago

The above code is deployed here https://demo.vercel.store and working on Vercel.

CleanShot.2024-09-04.at.09.08.57.mp4

Does not work on vercel at this moment. Typing in the search bar does nothing, doesn't update the URL either.

leerob commented 1 month ago

@csmartinsfct you need to press enter – we could probably change this to be onChange.