useStableQueryArgs chokes on BigInt

[rkofman]

When serializing a query object that includes a BigInt, rktQuery blows up with:

TypeError: Do not know how to serialize a BigInt

This seems to be due to this code in buildQueryHooks:

useStableQueryArgs(skip ? skipToken : arg, defaultSerializeQueryArgs, context.endpointDefinitions[name], name);

Which uses defaultSerializeQueryArgs instead of user-supplied overrides (i.e. serializeQueryArgs) -- where users could override the behavior and make bigints safe to serialize.

[riqts]

Worth noting that currently the buildQueryHooks code intentionally overrides the endpoint definitions serializeQueryArgs so that refetching and cache lifecycles proceed as intended when a queryArg changes.
so the hook recognises a change and triggers a refetch even when the user provided key remains stable.

My thoughts on how to approach it:

1. Add handling for BigInt inside defaultSerializeQueryArgs

   • This might be the obvious answer I am not sure if there's a clear reason why BigInt was not handled in the first place or not
   • Alternatively there might be an obvious reason why its not lol

2. Add an option to bypass defaultSerializeQueryArgs

   • Kind of a risk as users may not understand they are taking on the responsibility of managing refetch and unique keys themselves
   • really simple to implement :D

[EskiMojo14]

This might be the obvious answer I am not sure if there's a clear reason why BigInt was not handled in the first place or not
Alternatively there might be an obvious reason why its not lol

because JSON.stringify doesn't handle it and we haven't added any special behaviour to it

the only thing defaultSerializeQueryArgs does differently with JSON.stringify is ensuring keys are in a predictable order, so { foo: 1, bar: 2 } serializes to the same string as { bar: 2, foo: 1 }

[rkofman]

serializeQueryArgs is documented as:

Accepts a custom function if you have a need to change the creation of cache keys for any reason.

To me, it is surprising that changing how cache keys are created doesn't affect refetching and cache lifecycles. The example documenting the per-endpoint version reinforces this:

// Example: an endpoint with an API client passed in as an argument,
// but only the item ID should be used as the cache key

The API client isn't likely to change and as a user, I don't expect it to trigger new requests if it does.

If I understand correctly (and I very well might not), the reason to not just use the user-provided serializeQueryArgs is to allow e.g. infinite-loading paginated endpoints. The storage cache-key remains the same, but when the page param changes, the query still needs to fire and fetch new data (whose merging is then perhaps managed somewhat manually with merge).

In my mind, the ideal API would be to use the user-provided serializeQueryArgs override for both storage and lifecycle. Users would still be able to use forceRefetch to force fetching in cases where the serialized query args are not triggering a refetch (e.g. the page parameter changed).

Alternately, if two separate cache key implementations really are useful; it might be best to allow users to override either one. Conceptually, I would think of them as: serializeQueryArgsForFetching and serializeQueryArgsForStorage.

[riqts]

Worth noting that currently the buildQueryHooks code intentionally overrides the endpoint definitions serializeQueryArgs so that refetching and cache lifecycles proceed as intended when a queryArg changes.

My apologies, I may have misrepresented the intention of the change here. I believe the reason defaultSerializeQueryArgs is used instead of the user provided serializeQueryArgs is to ensure the hooks dependancy retriggers when the queryArgs change.

Regardless, I do agree mostly with what you have said, it does seem inherently at odds to allow a user supplied serializeQueryArgs and then run the queryArgs through the defaultSerializeQueryArgs. It does seem the reason it exists is to support infiniteQuery scenarios, as you suggested. #2816

I am exploring further whether I can make any reasonable trigger from the useStableQueryArgs using a stable cache key in this area:

export function useStableQueryArgs<T>(
  queryArgs: T,
  serialize: SerializeQueryArgs<any>,
  endpointDefinition: EndpointDefinition<any, any, any, any>,
  endpointName: string,
) {
  const incoming = useMemo(
    () => ({
      queryArgs,
      serialized:
        typeof queryArgs == 'object'
          ? serialize({ queryArgs, endpointDefinition, endpointName })
          : queryArgs,
    }),
    [queryArgs, serialize, endpointDefinition, endpointName],
  )
  const cache = useRef(incoming)
  useEffect(() => {
    if (cache.current.serialized !== incoming.serialized) {
      cache.current = incoming
    }
  }, [incoming])

  return cache.current.serialized === incoming.serialized
    ? cache.current.queryArgs
    : queryArgs
}

But I do think that potentially the alternative is to provide an additional option for the user to handle the refetching or arg changes

[rkofman]

Since the serialization for StableQueryArgs doesn't need to be reversible, would it be useful to just implement a mapping of bigint values to some serialized form directly in StableQueryArgs (e.g. a string value directly, or an object with a __bigint__ key that maps to a string)? That way it just.. silently has the correct behavior w/o any user impact.

[rkofman]

Anything I can do to facilitate a code review / merging of my PR above?

[phryneas]

Unfortunately, right now we're just all pretty preoccupied - we'll get around to the PR eventually, but I'm sorry I can't guarantee a time frame on it.
That said, the PR contains a PR build in the CodeSandbox preview that you can add to your package.json in the meantime - I hope that makes it already usable for you.

[rkofman]

I appreciate the pointer to the CodeSandbox preview. It's definitely a better workaround than I had landed on -- and something I totally overlooked as a possibility. Will follow-up on other comments in the PR!