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

null is passed as pageParam with PersistQueryClientProvider #4309

Closed
mrlika opened this issue Oct 11, 2022 · 10 comments · Fixed by #5004
Closed

null is passed as pageParam with PersistQueryClientProvider #4309

mrlika opened this issue Oct 11, 2022 · 10 comments · Fixed by #5004
Milestone

Comments

@mrlika
Copy link

mrlika commented Oct 11, 2022

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

@TkDodo
Copy link
Collaborator

TkDodo commented Oct 12, 2022

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 :/

@dodicandra
Copy link

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

@mrlika
Copy link
Author

mrlika commented Oct 19, 2022

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

Screen.Recording.2022-10-19.at.12.55.37-1.mov

@mrlika
Copy link
Author

mrlika commented Oct 21, 2022

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
Copy link
Author

mrlika commented Oct 21, 2022

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] })

@TkDodo TkDodo removed the needs-info label Oct 22, 2022
@TkDodo
Copy link
Collaborator

TkDodo commented Oct 22, 2022

@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:

serialize = JSON.stringify,
deserialize = JSON.parse,

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:

function dehydrateQuery(query: Query): DehydratedQuery {
return {
state: query.state,
queryKey: query.queryKey,
queryHash: query.queryHash,
}
}

Not sure if that would break something though

@TkDodo
Copy link
Collaborator

TkDodo commented Oct 22, 2022

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

@mrlika
Copy link
Author

mrlika commented Oct 23, 2022

@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.

@TkDodo
Copy link
Collaborator

TkDodo commented Oct 28, 2022

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 TkDodo added this to the v5 milestone Dec 23, 2022
@TkDodo
Copy link
Collaborator

TkDodo commented Feb 20, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants