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

Add API to allow permissive query keys #375

Closed
paul-sachs opened this issue Jun 6, 2024 · 8 comments
Closed

Add API to allow permissive query keys #375

paul-sachs opened this issue Jun 6, 2024 · 8 comments

Comments

@paul-sachs
Copy link
Collaborator

As of version 1.4.1, query keys are always generated with their default values populated. While this works really well for preventing duplicate queries, it prevents some use cases of createConnectQueryKey, primarily when used along with queryClient.invalidateQueries. See slack thread.

We should expose a standardized way of creating permissive keys that allow more generalized matching where we want to match multiple queries.

@errorhandler
Copy link

Is there any plans to add this in the short term? We've been holding off updating and I'm evaluating if we should make our own little function for the main use case we have: invalidate all instances of a particular RPC.

@timostamm
Copy link
Member

Hey Joe, we'll update the function in v2, and we're actively working on v2, so it shouldn't be too long 🙂

The main reason to cut a v2 is to support Protobuf-ES v2, so we don't plan on changing the connect-query API too much, but this is a good chance to improve createConnectQueryKey.

It should be feasible to support creating a key for an entire service, for an individual RPC, and for an RPC with a specific request payload.

BTW, in the interim, you can create a key to invalidate all requests for an RPC with:

const key = createConnectQueryKey(rpc).slice(0, 2);

This was referenced Sep 23, 2024
@timostamm
Copy link
Member

#416 adds a function to create message keys. It returns plain, stable, JSON serializable objects. If we use it to create query keys, it should solve this issue, and we can remove defaultOptions.queries.queryKeyHashFn, and set queryKeyHashFn: JSON.stringify per-query as a small optimization.

Can we make other improvements to the query key factories? An overload to invalidate all queries per service?

declare function createConnectQueryKey(
  method: MethodUnaryDescriptor,
  input?: DisableQuery | MessageInitShape | undefined,
): ConnectQueryKey;
declare function createConnectQueryKey(
  service: MethodUnaryDescriptor,
  input?: DisableQuery | MessageInitShape | undefined,
): ConnectQueryKey;

@paul-sachs
Copy link
Collaborator Author

@timostamm I think a better alignment strategy would actually be to create our own dedicated QueryClient which has some type safe methods that get to the core of a bunch of issues that we've been circling around for a while. With a custom QueryClient we can answer a bunch of things in one place:

  • Invalidation
  • Setting data
  • Default options (even if we don't need queryKeyHashFn anymore)
  • etc

It also follows the pattern set for connect-query in v1, which was to wrap react-query instead of just providing utility methods on top.

What do you think?

@timostamm
Copy link
Member

Query keys are a core primitive, and I think we should make sure that they work as well as possible (also see #418) before we add more features.

a bunch of issues that we've been circling around for a while

I only see #390. Are there other issues related to QueryClient?

@paul-sachs
Copy link
Collaborator Author

I was thinking about #390 and #391, but it's also something that's been floating around for a while. We have been using a custom QueryClient internally for some time now and it works quite well, cleans up some verbose usages for updating and invalidating query data.

I can get behind adding these in steps, using the new method createMessageKey to be used within createConnectQueryKey. Not even sure an overload makes much sense if we use createMessageKey based on how it handles undefined and empty object, it might just not matter but it would need some testing to validate.

@paul-sachs
Copy link
Collaborator Author

Internally, we created a helper method called createPermissiveConnectQueryKey which can be used for filters, but I'd have to dig in and see if that even makes sense in v2 anymore.

@timostamm
Copy link
Member

This is solved by #441 in the upcoming version 2. We'll get a pre-release out as soon as possible.

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

No branches or pull requests

3 participants