Fix/activity feed re-fetching behavior

[mrkarimoff]

This PR intends to fix the double flash of content behavior in the activity feed table

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
fresco-sandbox	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 30, 2024 1:11pm

[jthrilly]

This seems like the wrong approach. We are using nuqs for search params management, and have a hook handling the setting/reading of the current search params state, and passing it to the query function. Why can't we control this from within that hook? What about the clearOnDefault option?

[mrkarimoff]

This seems like the wrong approach. We are using nuqs for search params management, and have a hook handling the setting/reading of the current search params state, and passing it to the query function. Why can't we control this from within that hook? What about the clearOnDefault option?

Okay, the reason I'm using searchParams which comes from the custom useTableStateFromSearchParams hook in the queryFn is that it has the default values, so when the search query gets pushed to the URL (I mean in the first render the default values get pushed), useQuery doesn't fetch again because of the hash(searchParams). I hope my explanation makes sense I tried my best to describe it

[mrkarimoff]

Why can't we control this from within that hook?

Okay, if you mean moving the logic where I'm creating new URLSearchParams() into the custom useTableStateFromSearchParams hook and exporting the params from there, that I can do, but since we are going to be using URL search params for only the fetch request in the query function, I thought it would better to define it there.

I'm going to write the code here instead. If you approve of it I'll add these changes:

// useTableStateFromSearchParams.ts
'use client';
import { useQueryStates } from 'nuqs';
import { searchParamsParsers } from './SearchParams';

/**
 * This hook implements the table state items required by the DataTable.
 *
 * Ultimately, we could abstract this further, and implement a generic
 * useSearchParamsTableState hook so that the way the state is stored is an
 * implementation detail. This would allow us to store table state in novel
 * ways, such as in localStorage, in the URL, or even in a database.
 *
 */
export const useTableStateFromSearchParams = () => {
  const [{ page, perPage, sort, sortField, filterParams }, setSearchParams] =
    useQueryStates(searchParamsParsers);

  const params = new URLSearchParams();
  params.set('page', page.toString());
  params.set('perPage', perPage.toString());
  params.set('sort', sort);
  params.set('sortField', sortField);
  params.set('filterParams', JSON.stringify(filterParams));

  return {
    searchParams: {
      page,
      perPage,
      sort,
      sortField,
      filterParams,
    },
    setSearchParams,
    params,
  };
};

and

// ActivityFeed.tsx
'use client';

import type { Events } from '@prisma/client';
import { useQuery } from '@tanstack/react-query';
import { DataTableSkeleton } from '~/components/data-table/data-table-skeleton';
import ActivityFeedTable from './ActivityFeedTable';
import { useTableStateFromSearchParams } from './useTableStateFromSearchParams';

export const ActivityFeed = () => {
  const { params } = useTableStateFromSearchParams();

  const { isPending, data } = useQuery({
    queryKey: ['activityFeed', params.toString()],
    queryFn: async () => {
      const response = await fetch(`/api/activity-feed?${params.toString()}`, {
        next: { tags: ['activityFeed'] },
      });
      return response.json() as Promise<{
        events: Events[];
        pageCount: number;
      }>;
    },
  });

  if (isPending || !data) {
    return <DataTableSkeleton columnCount={3} filterableColumnCount={1} />;
  }

  return <ActivityFeedTable tableData={data} />;
};

I tested this and it works without the double flash behavior ✔️

[mrkarimoff]

What about the clearOnDefault option?

I haven't heard about this option before, but I did a little research and couldn't find this option in the list here in tanstack docs: https://tanstack.com/query/latest/docs/framework/react/reference/useQuery

[jthrilly]

No, clearOnDefault is an option in nuqs.

As for your other question: no, I don't mean simply moving where you create search params. I don't want us to have two different approaches to setting and updating searchParams at all. If the issue is that the refetch is triggered when nuqs adds default values, we need to either prevent that (clearOnDefault), or handle it more gracefully in the query function. so as to not actually cause a refetch.

[mrkarimoff]

No, clearOnDefault is an option in nuqs.

As for your other question: no, I don't mean simply moving where you create search params. I don't want us to have two different approaches to setting and updating searchParams at all. If the issue is that the refetch is triggered when nuqs adds default values, we need to either prevent that (clearOnDefault), or handle it more gracefully in the query function. so as to not actually cause a refetch.

Yeah, I tried clearOnDefault and it's working, thank you