query icon indicating copy to clipboard operation
query copied to clipboard

null is passed as pageParam with PersistQueryClientProvider

Open mrlika opened this issue 3 years ago • 9 comments

Describe the bug

Our code uses react-native, useInfiniteQuery with PersistQueryClientProvider:

const queryClient = new QueryClient({
  defaultOptions: {
    queries: {
      staleTime: 5 * 60 * 1000, // 5 min
      cacheTime: 1000 * 60 * 60 * 24, // 24 hours
    },
  },
});

export function CustomQueryClientProvider({ children }: { children: ReactNode }) {
  return (
    <PersistQueryClientProvider
      client={queryClient}
      persistOptions={{ persister: asyncStoragePersister }}
      onSuccess={() => {
        // resume mutations after initial restore from the storage was successful
        queryClient.resumePausedMutations();
      }}
    >
      {children}
    </PersistQueryClientProvider>
  );
}

type MovesQueryKeyType = [string, GetMovesFilter | undefined];
type MovesQueryOptions = UseInfiniteQueryOptions<MovesData, unknown, MovesData, MovesData, MovesQueryKeyType>;
type MovesQueryFunctionContext = QueryFunctionContext<MovesQueryKeyType, number>;

export const useMovesQuery = (filter?: GetMovesFilter, options?: MovesQueryOptions) =>
  useInfiniteQuery(
    [MOVE_QUERY_KEY, filter],
    ({ signal, pageParam }: MovesQueryFunctionContext) => getMoves(filter, pageParam, signal),
    {
      ...options,
      getNextPageParam: (_, pages) => pages.length + 1,
    }
  );
  
const { isLoading, data: movesData } = useMovesQuery(undefined, {
    staleTime: 0,
  });

According to the type system pageParam passed to getMoves is a number | undefined. But when I re-run my app null is passed as pageParam

Your minimal, reproducible example

in description

Steps to reproduce

  1. Open app, load first page of the infinite query
  2. Reopen app to allow PersistQueryClientProvider to restore persisted cache

Expected behavior

null should not be passed as pageParam according to the types

How often does this bug happen?

Every time

Screenshots or Videos

No response

Platform

Expo Go on Android

react-query version

4.10.1

TypeScript version

latest

Additional context

No response

mrlika avatar Oct 11 '22 22:10 mrlika

Your minimal, reproducible example in description

please put that code in codesandbox or somewhere were we can run the example, thanks.

default serialization uses JSON.stringify so I can't imagine how an undefined value can become null after hydration. Can't really see anything like that in our codebase :/

TkDodo avatar Oct 12 '22 09:10 TkDodo

hi, @TkDodo i also got the same problem. Please see this repo for more details. for this information occurs when the application is reloaded. to achieve it, do it like this

scroll to last page -> pull to refresh -> reload app (type R in terminal) -> scroll to last page -> pull to refresh. in this last step we will see an error message indicating pageParam contains null

dodicandra avatar Oct 19 '22 07:10 dodicandra

The demo is in JS, but if add types with TypeScript it breaks the types. page should be number | undefined and never null

https://codesandbox.io/s/nifty-frost-8v1kjf?file=/src/App.js

https://user-images.githubusercontent.com/1469266/196660170-582c36f1-58fa-485f-8930-ac42c2425aa0.mov

mrlika avatar Oct 19 '22 09:10 mrlika

Right before saving pageParam is undefined (that is correct):

Screenshot 2022-10-21 at 12 57 19

But in Local Storage after saving it is null:

Screenshot 2022-10-21 at 12 59 42

mrlika avatar Oct 21 '22 10:10 mrlika

The issue is that undefined is not a valid JSON value:

Result of

JSON.stringify({ pageParams: [undefined] })

is "{\"pageParams\":[null]}".

The same result gives

JSON.stringify({ pageParams: [null] })

mrlika avatar Oct 21 '22 10:10 mrlika

@mrlika you're right about that - I think something similar happens when you try prefetch an infinite query on the server and then try to send it to the client via nextJs, because nextJs also just uses JSON.stringify.

I find it kinda weird that undefined is serialized as null. One thing you can do for now is to pass a different serialize function to the persisters:

https://github.com/TanStack/query/blob/7741a2f723f37bdb3ce79cb96ed2394b121b21d3/packages/query-async-storage-persister/src/index.ts#L48-L49

I think having a serializer that omits undefined values might do the trick.

We could also have a look at how we dehydrate queries and maybe leave out the pageParams is they are undefined:

https://github.com/TanStack/query/blob/357ec041a6fcc4a550f3df02c12ecc7bcdefbc05/packages/query-core/src/hydration.ts#L60-L66

Not sure if that would break something though

TkDodo avatar Oct 22 '22 10:10 TkDodo

The problem is that pageParams will likely have multiple entries, so it's:

pageParams: [undefined, 5, 10, 15]

there is no real way to serialize this to json and keep the information that there was an entry.

superjson supports serializing undefined, so you might just want to use this as a drop-in replacement for JSON.stringify / JSON.parse

TkDodo avatar Oct 22 '22 10:10 TkDodo

@TkDodo, the issue is that this undefined is passed by React Query into my code. And then default serializer of React Query saves it incorrectly and null is passed back instead of undefined breaking the types.

The solution is to manually make the type PageParams | null if persist feature is used.

mrlika avatar Oct 23 '22 21:10 mrlika

We've discussed this and there doesn't seem to be a good way that wouldn't break some edge cases where people rely on null being rightfully returned from getNextPageParam.

We have a plan that disallows null being returned from there, then we can treat null like undefined and side-step that issue. We would make sure internally that you'd always get undefined passed into the queryFn even if null sneaks into the pageParams.

But, it will have to wait a bit for the next major release.

TkDodo avatar Oct 28 '22 13:10 TkDodo

PR:

  • #5004

TkDodo avatar Feb 20 '23 17:02 TkDodo