connectrpc / connect-query-es

TypeScript-first expansion pack for TanStack Query that gives you Protobuf superpowers.
https://connectrpc.com/docs/web/query/getting-started
Apache License 2.0
239 stars 17 forks source link

Availability of createUseQueryOptions and createUseInfiniteQueryOptions in v1 #296

Closed nakker1218 closed 10 months ago

nakker1218 commented 11 months ago

Is the absence of createUseQueryOptions and createUseInfiniteQueryOptions in v1 intentional? These methods appear to be extremely important when React context is not available.

https://github.com/connectrpc/connect-query-es/blob/main/packages/connect-query/src/index.ts#L19-L24

The README mentions createUseQueryOptions:

What if I have a custom Transport? If the Transport attached to React Context via the TransportProvider isn't working for you, then you can override transport at every level. For example, you can pass a custom transport directly to the lowest-level API like useQuery or createUseQueryOptions.

Does this only work with React? Connect-Query does require React, but the core (createUseQueryOptions) is not React specific so splitting off a connect-solid-query is possible.

(README Link: https://github.com/connectrpc/connect-query-es?tab=readme-ov-file#does-this-only-work-with-react)

Is there an alternative method now, or was their removal unintentional? If it's an unintentional, I'm willing to help with a PR.

paul-sachs commented 11 months ago

This was an intentional change to reduce the API surface and create a more focused API. That said, I would be open to exposing these APIs again given a specific use case that can't be served otherwise.

paul-sachs commented 11 months ago

We did expose a helper, callUnaryMethod which can often be used similarly, just without being specific to infiniteQuery or query

paul-sachs commented 10 months ago

I guess I also forgot to mention createConnectQueryKey(), which provides the queryKey.

nakker1218 commented 10 months ago

Thank you for your response. I understand now that it was an intentional change. First, we will try using callUnaryMethod in our product and see if there are any issues. For now, I will close this issue and if anything comes up, we will create a new one!

lsmurray commented 9 months ago

I think it would be nice if these were exposed. They are useful for building custom hooks but also for some APIs such as queryClient.ensureQueryData

    await queryClient.ensureQueryData(
      createUseQueryOptions(
        rpc,
        {
           input
        },
        {
          transport,
        }
      )
    );

While I could implement this with the unary helpers it ends up being the same code over and over again and the code is basically a copy paste of the internal API.

paul-sachs commented 9 months ago

In our codebase, this kind of pattern arose too, but we ended up going down the path of a custom QueryClient, which provided typesafe variants of the APIs, so invalidateQueries became invalidateConnectQueries with appropriate args. Ditto with setConnectQueryData. It's still a little early within our codebase to publish it, but I personally think it provides a better DX and could even eliminate the need (but keep an option for) the transport.

Obviously one does not negate the other but in the spirit of reducing the number of ways to do things, the custom client feels more directed and adds more value than just removing helper code.

If that would be of interest to you, I can open a tracking issue for that work and we can get a little feedback from the ecosystem in a slightly more visible way.

lsmurray commented 9 months ago

That sounds slightly better than what I implemented but similar. Would be happy to provide feedback when you're ready to publish anything.

advdv commented 8 months ago

I'm in the situation that I want to call "context.queryClient.ensureQueryData" (in Tanstack Router's loader as described here https://tanstack.com/router/latest/docs/framework/react/guide/external-data-loading#a-more-realistic-example-using-tanstack-query) But I'm a bit confused as to what the recommendation is right now?

Do I extend the query client? But I'm not sure how to do this, is there an example? I've tried to copy the createUseQueryOptions from the codebase but it is quite a lot it seems with all the required internal types, or maybe 'm doing it wrong as well?

Also, if i just do something like this:

import { createFileRoute } from "@tanstack/react-router";
import { say } from "@buf/connectrpc_eliza.connectrpc_query-es/connectrpc/eliza/v1/eliza-ElizaService_connectquery";
import {
  callUnaryMethod,
  createConnectQueryKey,
} from "@connectrpc/connect-query";
import { queryOptions, useSuspenseQuery } from "@tanstack/react-query";
import { connectTransport } from "@/api";

// query options
const sayQueryOptions = queryOptions({
  // eslint-disable-next-line @tanstack/query/exhaustive-deps
  queryKey: createConnectQueryKey(say, { sentence: "hello" }),
  queryFn: () =>
    callUnaryMethod(
      say,
      { sentence: "hello" },
      { transport: connectTransport },
    ),
});

// route
export const Route = createFileRoute("/about/")({
  component: About,
  loader: ({ context }) => context.queryClient.ensureQueryData(sayQueryOptions),
});

// page component
function About() {
  const { data } = useSuspenseQuery(sayQueryOptions);

  return <div className="p-2">Hello: {data?.sentence}</div>;
}

the https://tanstack.com/query/latest/docs/eslint/exhaustive-deps listing rule complains. Also giving me the feeling I'm doing something wrong.

Imo it's probably pretty common to use Tanstack query in router data "loaders" where the react context is not available.

paul-sachs commented 8 months ago

So the way we are creating our own custom query client is like so:

import { QueryClient as TSQueryClient, InvalidateOptions,
  SetDataOptions, } from "@tanstack/react-query";
import {
  callUnaryMethod,
  createConnectQueryKey,
  defaultOptions,
  DisableQuery,
  disableQuery,
  TransportProvider,
  useInfiniteQuery,
  useMutation,
  createProtobufSafeUpdater,
  useTransport,
  ConnectQueryKey
} from "@connectrpc/connect-query";
import { CallOptions, ConnectError, Transport } from "@connectrpc/connect";
import {
  AnyMessage,
  Message,
  MethodInfoUnary,
  PartialMessage,
  ServiceType,
} from "@bufbuild/protobuf";

export class QueryClient extends TSQueryClient {
  invalidateConnectQueries<I extends Message<I>, O extends Message<O>>(
    methodSig: MethodSignature<I, O>,
    input?: PartialMessage<I>,
    options?: InvalidateOptions
  ) {
    return this.invalidateQueries(
      {
        queryKey: createConnectQueryKey(methodSig, input),
      },
      options
    );
  }

  setConnectQueryData<I extends Message<I>, O extends Message<O>>(
    methodSig: MethodSignature<I, O>,
    updater: ConnectUpdater<O>,
    input?: typeof disableQuery | PartialMessage<I> | undefined,
    options?: SetDataOptions | undefined
  ) {
    return this.setQueryData(
      createConnectQueryKey(methodSig, input),
      createProtobufSafeUpdater(methodSig, updater),
      options
    );
  }
}

As for the eslint rule @tanstack/query/exhaustive-deps, it's a valid point. Currently we don't use the transport as part of the key, which means that if the transport changes then related data doesn't auto fetch again. Not actually sure how we'd want to solve that one, since the transport itself can't really be serialized into a string (it's a set of functions) and thus cannot be a part of the query key.

advdv commented 8 months ago

Ah right, the query client is just a class. That make sense, thank you

For the cache key not including the transport. Am I correct that it only means that the cache key might overlap if a project uses different transports with the same method signatures in the same project?

paul-sachs commented 8 months ago

For the cache key not including the transport. Am I correct that it only means that the cache key might overlap if a project uses different transports with the same method signatures in the same project?

Yes, that is correct. I'm not sure how realistic that scenario is, but it is possible.

paul-sachs commented 8 months ago

Fixed in https://github.com/connectrpc/connect-query-es/releases/tag/v1.3.0